← Back to branch summary

~ubuntu-branches/ubuntu/saucy/haproxy/saucy

« back to all changes in this revision

Viewing changes to debian/patches/CVE-2013-1912.patch

  • Committer: Package Import Robot
  • Author(s): Vincent Bernat, Apollon Oikonomopoulos, Vincent Bernat, Prach Pongpanich
  • Date: 2013-05-06 20:02:14 UTC
  • mfrom: (1.1.13) (14.1.1 sid)
  • Revision ID: package-import@ubuntu.com-20130506200214-21m58va04oazl7ot
Tags: 1.4.23-1
http://bugs.debian.org/643650
http://bugs.debian.org/678953
http://bugs.debian.org/674447
http://bugs.debian.org/704611
http://bugs.debian.org/702893
http://bugs.debian.org/641762
http://bugs.debian.org/649085
http://bugs.debian.org/706890
[ Apollon Oikonomopoulos ]
* New upstream version (Closes: #643650, #678953)
   + This fixes CVE-2012-2942 (Closes: #674447)
   + This fixes CVE-2013-1912 (Closes: #704611)
* Ship vim addon as vim-haproxy (Closes: #702893)
* Check for the configuration file after sourcing /etc/default/haproxy
  (Closes: #641762)
* Use /dev/log for logging by default (Closes: #649085)

[ Vincent Bernat ]
* debian/control:
   + add Vcs-* fields
   + switch maintenance to Debian HAProxy team. (Closes: #706890)
   + drop dependency to quilt: 3.0 (quilt) format is in use.
* debian/rules:
   + don't explicitly call dh_installchangelog.
   + use dh_installdirs to install directories.
   + use dh_install to install error and configuration files.
   + switch to `linux2628` Makefile target for Linux.
* debian/postrm:
   + remove haproxy user and group on purge.
* Ship a more minimal haproxy.cfg file: no `listen` blocks but `global`
  and `defaults` block with appropriate configuration to use chroot and
  logging in the expected way.

[ Prach Pongpanich ]
* debian/copyright:
   + add missing copyright holders
   + update years of copyright
* debian/rules:
   + build with -Wl,--as-needed to get rid of unnecessary depends
* Remove useless files in debian/haproxy.{docs,examples}
* Update debian/watch file, thanks to Bart Martens

Show diffs side-by-side

added added

removed removed

Lines of Context:
debian/patches/CVE-2013-1912.patch
1
 
Backport of:
2
 
 
3
 
From: Willy Tarreau <w@1wt.eu>
4
 
Date: Fri, 29 Mar 2013 11:31:49 +0000 (+0100)
5
 
Subject: BUG/CRITICAL: using HTTP information in tcp-request content may crash the process
6
 
X-Git-Tag: v1.4.23~1
7
 
X-Git-Url: http://git.1wt.eu:81/web?p=haproxy-1.4.git;a=commitdiff_plain;h=dc80672211
8
 
 
9
 
BUG/CRITICAL: using HTTP information in tcp-request content may crash the process
10
 
 
11
 
During normal HTTP request processing, request buffers are realigned if
12
 
there are less than global.maxrewrite bytes available after them, in
13
 
order to leave enough room for rewriting headers after the request. This
14
 
is done in http_wait_for_request().
15
 
 
16
 
However, if some HTTP inspection happens during a "tcp-request content"
17
 
rule, this realignment is not performed. In theory this is not a problem
18
 
because empty buffers are always aligned and TCP inspection happens at
19
 
the beginning of a connection. But with HTTP keep-alive, it also happens
20
 
at the beginning of each subsequent request. So if a second request was
21
 
pipelined by the client before the first one had a chance to be forwarded,
22
 
the second request will not be realigned. Then, http_wait_for_request()
23
 
will not perform such a realignment either because the request was
24
 
already parsed and marked as such. The consequence of this, is that the
25
 
rewrite of a sufficient number of such pipelined, unaligned requests may
26
 
leave less room past the request been processed than the configured
27
 
reserve, which can lead to a buffer overflow if request processing appends
28
 
some data past the end of the buffer.
29
 
 
30
 
A number of conditions are required for the bug to be triggered :
31
 
  - HTTP keep-alive must be enabled ;
32
 
  - HTTP inspection in TCP rules must be used ;
33
 
  - some request appending rules are needed (reqadd, x-forwarded-for)
34
 
  - since empty buffers are always realigned, the client must pipeline
35
 
    enough requests so that the buffer always contains something till
36
 
    the point where there is no more room for rewriting.
37
 
 
38
 
While such a configuration is quite unlikely to be met (which is
39
 
confirmed by the bug's lifetime), a few people do use these features
40
 
together for very specific usages. And more importantly, writing such
41
 
a configuration and the request to attack it is trivial.
42
 
 
43
 
A quick workaround consists in forcing keep-alive off by adding
44
 
"option httpclose" or "option forceclose" in the frontend. Alternatively,
45
 
disabling HTTP-based TCP inspection rules enough if the application
46
 
supports it.
47
 
 
48
 
At first glance, this bug does not look like it could lead to remote code
49
 
execution, as the overflowing part is controlled by the configuration and
50
 
not by the user. But some deeper analysis should be performed to confirm
51
 
this. And anyway, corrupting the process' memory and crashing it is quite
52
 
trivial.
53
 
 
54
 
Special thanks go to Yves Lafon from the W3C who reported this bug and
55
 
deployed significant efforts to collect the relevant data needed to
56
 
understand it in less than one week.
57
 
 
58
 
CVE-2013-1912 was assigned to this issue.
59
 
 
60
 
Note that 1.4 is also affected so the fix must be backported.
61
 
(cherry picked from commit aae75e3279c6c9bd136413a72dafdcd4986bb89a)
62
 
---
63
 
 
64
 
Index: haproxy-1.4.18/src/proto_http.c
65
 
===================================================================
66
 
--- haproxy-1.4.18.orig/src/proto_http.c        2013-04-05 10:10:55.606062123 -0400
67
 
+++ haproxy-1.4.18/src/proto_http.c     2013-04-05 10:10:55.602062123 -0400
68
 
@@ -8097,6 +8097,14 @@
69
 
                return 1;
70
 
        }
71
 
 
72
 
+       /* If the buffer does not leave enough free space at the end,
73
 
+        * we must first realign it.
74
 
+        */
75
 
+       if (unlikely(req->lr > req->data &&
76
 
+           (req->r < req->lr || req->r > req->data + req->size - global.tune.maxrewrite)) &&
77
 
+           (req->l <= req->size - global.tune.maxrewrite))
78
 
+               http_buffer_heavy_realign(req, msg);
79
 
+
80
 
        /* Try to decode HTTP request */
81
 
        if (likely(req->lr < req->r))
82
 
                http_msg_analyzer(req, msg, &txn->hdr_idx);
83
 
@@ -8114,6 +8122,20 @@
84
 
        /* OK we got a valid HTTP request. We have some minor preparation to
85
 
         * perform so that further checks can rely on HTTP tests.
86
 
         */
87
 
+
88
 
+       /* If the request was parsed but was too large, we must absolutely
89
 
+        * return an error so that it is not processed. At the moment this
90
 
+        * cannot happen, but if the parsers are to change in the future,
91
 
+        * we want this check to be maintained.
92
 
+        */
93
 
+       if (unlikely(req->lr > req->data &&
94
 
+           (req->r < req->lr || req->l > req->size - global.tune.maxrewrite ||
95
 
+            req->r > req->data + req->size - global.tune.maxrewrite))) {
96
 
+               msg->msg_state = HTTP_MSG_ERROR;
97
 
+               test->flags |= ACL_TEST_F_SET_RES_PASS;
98
 
+               return 1;
99
 
+       }
100
 
+
101
 
        txn->meth = find_http_meth(msg->sol, msg->sl.rq.m_l);
102
 
        if (txn->meth == HTTP_METH_GET || txn->meth == HTTP_METH_HEAD)
103
 
                s->flags |= SN_REDIRECTABLE;