|
| 1 | +From 1e01ccaf983170bf6d94d9386f33affb92f1db78 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Willy Tarreau < [email protected]> |
| 3 | +Date: Mon, 29 Sep 2025 18:34:11 +0200 |
| 4 | +Subject: [PATCH] BUG/CRITICAL: mjson: fix possible DoS when parsing numbers |
| 5 | + |
| 6 | +Mjson comes with its own strtod() implementation for portability |
| 7 | +reasons and probably also because many generic strtod() versions as |
| 8 | +provided by operating systems do not focus on resource preservation |
| 9 | +and may call malloc(), which is not welcome in a parser. |
| 10 | + |
| 11 | +The strtod() implementation used here apparently originally comes from |
| 12 | +https://gist.github.com/mattn/1890186 and seems to have purposely |
| 13 | +omitted a few parts that were considered as not needed in this context |
| 14 | +(e.g. skipping white spaces, or setting errno). But when subject to the |
| 15 | +relevant test cases of the designated file above, the current function |
| 16 | +provides the same results. |
| 17 | + |
| 18 | +The aforementioned implementation uses pow() to calculate exponents, |
| 19 | +but mjson authors visibly preferred not to introduce a libm dependency |
| 20 | +and replaced it with an iterative loop in O(exp) time. The problem is |
| 21 | +that the exponent is not bounded and that this loop can take a huge |
| 22 | +amount of time. There's even an issue already opened on mjson about |
| 23 | +this: https://github.com/cesanta/mjson/issues/59. In the case of |
| 24 | +haproxy, fortunately, the watchdog will quickly stop a runaway process |
| 25 | +but this remains a possible denial of service. |
| 26 | + |
| 27 | +A first approach would consist in reintroducing pow() like in the |
| 28 | +original implementation, but if haproxy is built without Lua nor |
| 29 | +51Degrees, -lm is not used so this will not work everywhere. |
| 30 | + |
| 31 | +Anyway here we're dealing with integer exponents, so an easy alternate |
| 32 | +approach consists in simply using shifts and squares, to compute the |
| 33 | +exponent in O(log(exp)) time. Not only it doesn't introduce any new |
| 34 | +dependency, but it turns out to be even faster than the generic pow() |
| 35 | +(85k req/s per core vs 83.5k on the same machine). |
| 36 | + |
| 37 | +This must be backported as far as 2.4, where mjson was introduced. |
| 38 | + |
| 39 | +Many thanks to Oula Kivalo for reporting this issue. |
| 40 | + |
| 41 | +CVE-2025-11230 was assigned to this issue. |
| 42 | + |
| 43 | +(cherry picked from commit 06675db4bf234ed17e14305f1d59259d2fe78b06) |
| 44 | +Signed-off-by: Christopher Faulet < [email protected]> |
| 45 | +Signed-off-by: Azure Linux Security Servicing Account < [email protected]> |
| 46 | +Upstream-reference: https://github.com/haproxy/haproxy/commit/6fd1287526eae1b31329997a2df29c9fb564a8e8.patch |
| 47 | +--- |
| 48 | + src/mjson.c | 14 ++++++++++++-- |
| 49 | + 1 file changed, 12 insertions(+), 2 deletions(-) |
| 50 | + |
| 51 | +diff --git a/src/mjson.c b/src/mjson.c |
| 52 | +index 73b7a57..2a4106b 100644 |
| 53 | +--- a/src/mjson.c |
| 54 | ++++ b/src/mjson.c |
| 55 | +@@ -767,11 +767,13 @@ static double mystrtod(const char *str, char **end) { |
| 56 | + |
| 57 | + /* exponential part */ |
| 58 | + if ((*p == 'E') || (*p == 'e')) { |
| 59 | ++ double exp, f; |
| 60 | + int i, e = 0, neg = 0; |
| 61 | + p++; |
| 62 | + if (*p == '-') p++, neg++; |
| 63 | + if (*p == '+') p++; |
| 64 | + while (is_digit(*p)) e = e * 10 + *p++ - '0'; |
| 65 | ++ i = e; |
| 66 | + if (neg) e = -e; |
| 67 | + #if 0 |
| 68 | + if (d == 2.2250738585072011 && e == -308) { |
| 69 | +@@ -785,8 +787,16 @@ static double mystrtod(const char *str, char **end) { |
| 70 | + goto done; |
| 71 | + } |
| 72 | + #endif |
| 73 | +- for (i = 0; i < e; i++) d *= 10; |
| 74 | +- for (i = 0; i < -e; i++) d /= 10; |
| 75 | ++ /* calculate f = 10^i */ |
| 76 | ++ exp = 10; |
| 77 | ++ f = 1; |
| 78 | ++ while (i > 0) { |
| 79 | ++ if (i & 1) f *= exp; |
| 80 | ++ exp *= exp; |
| 81 | ++ i >>= 1; |
| 82 | ++ } |
| 83 | ++ if (e > 0) d *= f; |
| 84 | ++ else if (e < 0) d /= f; |
| 85 | + a = p; |
| 86 | + } else if (p > str && !is_digit(*(p - 1))) { |
| 87 | + a = str; |
| 88 | +-- |
| 89 | +2.45.4 |
| 90 | + |
0 commit comments