Hi all,

Le 17/06/2013 16:24, Willy Tarreau a écrit :
Hi all,

Here's an important update for 1.4 and 1.5, please read it, it contains
an important security fix.

When a config makes use of hdr_ip(x-forwarded-for,-1) or any such thing
involving a negative occurrence count, the header is still parsed in the
order it appears, and an array of up to MAX_HDR_HISTORY entries is created.
When more entries are used, the entries simply wrap and continue this way.

A problem happens when the incoming header field count exactly divides
MAX_HDR_HISTORY, because the computation removes the number of requested
occurrences from the count, but does not care about the risk of wrapping
with a negative number. Thus we can dereference the array with a negative
number and randomly crash the process.

The bug is located in http_get_hdr() in haproxy 1.5, and get_ip_from_hdr2()
in haproxy 1.4. It affects configurations making use of one of the following
functions with a negative <value> occurence number :

    - hdr_ip(<name>, <value>)  (in 1.4)
    - hdr_*(<name>, <value>)   (in 1.5)

It also affects "source" statements involving "hdr_ip(<name>)" since that
statement implicitly uses -1 for <value> :

    - source 0.0.0.0 usesrc hdr_ip(<name>)

A workaround consists in rejecting dangerous requests early using
hdr_cnt(<name>), which is available both in 1.4 and 1.5 :

    block if { hdr_cnt(<name>) ge 10 }

This bug has been present since the introduction of the negative offset
count in 1.4.4 via commit bce70882. It has been reported by David Torgerson
who offered some debugging traces showing where the crash happened, thus
making it significantly easier to find the bug!

CVE-2013-2175 was assigned to this bug.

Some monthes ago, I developed a patch to add IP geolocation support to haproxy :
https://github.com/cbonte/haproxy-patches/wiki/Geolocation

I didn't take time to publish it on the mailing list, so I'm not sure it is used by others, but in case it is, I strongly recommend to upgrade to haproxy 1.4.24 or 1.5-dev19, as it also uses http_get_hdr() when matching an IP against an HTTP header.

For the short story, after discussing with Willy, we decided not to include it directly in haproxy. This will let us introduce some improvements in the configuration file syntax after 1.5 is released.


--
Cyril Bonté

Reply via email to