Willy Tarreau (w) reports:
Hi all,
a reproducible crash on latest haproxy snapshots was recently reported,
and could finally be tracked down to a serious bug affecting all versions
since 1.4.4.
The bug is in http_get_hdr() in haproxy 1.5, or get_ip_from_hdr2() in
haproxy 1.4. If a configuration makes use of one of the following functions :
- hdr_ip(<name>, <value>) (in 1.4)
- hdr_*(<name>, <value>) (in 1.5)
with a negative <value>, then the configuration risks to crash when the
request contains exactly MAX_HDR_HISTORY values for the header <name>.
Note: "source 0.0.0.0 usesrc hdr_ip(<name>)" uses -1 by default for <value>
and is vulnerable as well !
The quick workaround I can suggest before patching is to reject dangerous
requests early using hdr_cnt(<name>), which is available both in 1.4 and 1.5 :
block if { hdr_cnt(<name>) ge 10 }
Thus, I'm going to issue 1.5-dev19 and 1.4.24 with the following patch applied.
1.5-dev has not received significant updates recently and is of very little
risk when migrating from dev17 or dev18.
The new version will probably be finished during the week-end with an announce
on Monday morning european time. It leaves enough time to finish testing the
last snapshots.
If anyone has any concern with the fix, please discuss it on the list so that
we can find a quick solution together.
I'll request a CVE ID for this bug after the release. If any of the
subscribers has a list of spare CVE IDs, feel free to propose one before
the release, that way I'll update the commit message to report it.
Best regards,
Willy
-
From 20d1c2cb1c6fcfe1f74a79f15573223701903834 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w>
Date: Wed, 12 Jun 2013 22:27:44 +0200
Subject: BUG/CRITICAL: fix a possible crash when using negative header
occurrences
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.
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!
This fix must absolutely be backported to 1.4.
Comment 11Murray McAllister
2013-07-30 09:38:20 UTC
Acknowledgements:
Red Hat would like to thank HAProxy upstream for reporting this issue. Upstream acknowledges David Torgerson as the original reporter.
Willy Tarreau (w) reports: Hi all, a reproducible crash on latest haproxy snapshots was recently reported, and could finally be tracked down to a serious bug affecting all versions since 1.4.4. The bug is in http_get_hdr() in haproxy 1.5, or get_ip_from_hdr2() in haproxy 1.4. If a configuration makes use of one of the following functions : - hdr_ip(<name>, <value>) (in 1.4) - hdr_*(<name>, <value>) (in 1.5) with a negative <value>, then the configuration risks to crash when the request contains exactly MAX_HDR_HISTORY values for the header <name>. Note: "source 0.0.0.0 usesrc hdr_ip(<name>)" uses -1 by default for <value> and is vulnerable as well ! The quick workaround I can suggest before patching is to reject dangerous requests early using hdr_cnt(<name>), which is available both in 1.4 and 1.5 : block if { hdr_cnt(<name>) ge 10 } Thus, I'm going to issue 1.5-dev19 and 1.4.24 with the following patch applied. 1.5-dev has not received significant updates recently and is of very little risk when migrating from dev17 or dev18. The new version will probably be finished during the week-end with an announce on Monday morning european time. It leaves enough time to finish testing the last snapshots. If anyone has any concern with the fix, please discuss it on the list so that we can find a quick solution together. I'll request a CVE ID for this bug after the release. If any of the subscribers has a list of spare CVE IDs, feel free to propose one before the release, that way I'll update the commit message to report it. Best regards, Willy - From 20d1c2cb1c6fcfe1f74a79f15573223701903834 Mon Sep 17 00:00:00 2001 From: Willy Tarreau <w> Date: Wed, 12 Jun 2013 22:27:44 +0200 Subject: BUG/CRITICAL: fix a possible crash when using negative header occurrences 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. 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! This fix must absolutely be backported to 1.4.