Skip to content

Commit 29592cb

Browse files
committed
BUG/MINOR: mjson: make mystrtod() length-aware to prevent out-of-bounds reads
mystrtod() was not length-aware and relied on null-termination or a non-numeric character to stop. The fix adds a length parameter as a strict upper bound for all pointer accesses. The practical impact in haproxy is essentially null: all callers embed the JSON payload inside a large haproxy buffer, so the speculative read past the last digit lands on memory that is still within the same allocation. ASAN cannot detect it in a normal haproxy run for the same reason — the overread never escapes the enclosing buffer. Triggering a detectable fault requires placing the JSON payload at the exact end of an allocation. Note: the 'path' buffer was using a null-terminated string so the result of strlen is passed to it, this part was not at risk. Thanks to Kamil Frankowicz for the original bug report. This patch must be backported to all maintained versions.
1 parent 8dae4f7 commit 29592cb

File tree

1 file changed

+21
-22
lines changed

1 file changed

+21
-22
lines changed

src/mjson.c

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424

2525
#include <import/mjson.h>
2626

27-
static double mystrtod(const char *str, char **end);
27+
static double mystrtod(const char *str, int len, char **end);
2828

2929
static int mjson_esc(int c, int esc) {
3030
const char *p, *esc1 = "\b\f\n\r\t\\\"", *esc2 = "bfnrt\\\"";
@@ -101,7 +101,7 @@ int mjson(const char *s, int len, mjson_cb_t cb, void *ud) {
101101
tok = MJSON_TOK_FALSE;
102102
} else if (c == '-' || ((c >= '0' && c <= '9'))) {
103103
char *end = NULL;
104-
mystrtod(&s[i], &end);
104+
mystrtod(&s[i], len - i, &end);
105105
if (end != NULL) i += (int) (end - &s[i] - 1);
106106
tok = MJSON_TOK_NUMBER;
107107
} else if (c == '"') {
@@ -212,7 +212,7 @@ static int mjson_get_cb(int tok, const char *s, int off, int len, void *ud) {
212212
} else if (tok == '[') {
213213
if (data->d1 == data->d2 && data->path[data->pos] == '[') {
214214
data->i1 = 0;
215-
data->i2 = (int) mystrtod(&data->path[data->pos + 1], NULL);
215+
data->i2 = (int) mystrtod(&data->path[data->pos + 1], strlen(&data->path[data->pos + 1]), NULL);
216216
if (data->i1 == data->i2) {
217217
data->d2++;
218218
data->pos += 3;
@@ -272,7 +272,7 @@ int mjson_get_number(const char *s, int len, const char *path, double *v) {
272272
const char *p;
273273
int tok, n;
274274
if ((tok = mjson_find(s, len, path, &p, &n)) == MJSON_TOK_NUMBER) {
275-
if (v != NULL) *v = mystrtod(p, NULL);
275+
if (v != NULL) *v = mystrtod(p, n, NULL);
276276
}
277277
return tok == MJSON_TOK_NUMBER ? 1 : 0;
278278
}
@@ -343,57 +343,56 @@ static int is_digit(int c) {
343343
}
344344

345345
/* NOTE: strtod() implementation by Yasuhiro Matsumoto. */
346-
static double mystrtod(const char *str, char **end) {
346+
static double mystrtod(const char *str, int len, char **end) {
347347
double d = 0.0;
348348
int sign = 1, __attribute__((unused)) n = 0;
349349
const char *p = str, *a = str;
350+
const char *end_p = str + len;
350351

351352
/* decimal part */
352-
if (*p == '-') {
353+
if (p < end_p && *p == '-') {
353354
sign = -1;
354355
++p;
355-
} else if (*p == '+') {
356+
} else if (p < end_p && *p == '+') {
356357
++p;
357358
}
358-
if (is_digit(*p)) {
359+
if (p < end_p && is_digit(*p)) {
359360
d = (double) (*p++ - '0');
360-
while (*p && is_digit(*p)) {
361+
while (p < end_p && is_digit(*p)) {
361362
d = d * 10.0 + (double) (*p - '0');
362363
++p;
363364
++n;
364365
}
365366
a = p;
366-
} else if (*p != '.') {
367+
} else if (p >= end_p || *p != '.') {
367368
goto done;
368369
}
369370
d *= sign;
370371

371372
/* fraction part */
372-
if (*p == '.') {
373+
if (p < end_p && *p == '.') {
373374
double f = 0.0;
374375
double base = 0.1;
375376
++p;
376377

377-
if (is_digit(*p)) {
378-
while (*p && is_digit(*p)) {
379-
f += base * (*p - '0');
380-
base /= 10.0;
381-
++p;
382-
++n;
383-
}
378+
while (p < end_p && is_digit(*p)) {
379+
f += base * (*p - '0');
380+
base /= 10.0;
381+
++p;
382+
++n;
384383
}
385384
d += f * sign;
386385
a = p;
387386
}
388387

389388
/* exponential part */
390-
if ((*p == 'E') || (*p == 'e')) {
389+
if (p < end_p && ((*p == 'E') || (*p == 'e'))) {
391390
double exp, f;
392391
int i, e = 0, neg = 0;
393392
p++;
394-
if (*p == '-') p++, neg++;
395-
if (*p == '+') p++;
396-
while (is_digit(*p)) e = e * 10 + *p++ - '0';
393+
if (p < end_p && *p == '-') p++, neg++;
394+
if (p < end_p && *p == '+') p++;
395+
while (p < end_p && is_digit(*p)) e = e * 10 + *p++ - '0';
397396
i = e;
398397
if (neg) e = -e;
399398
#if 0

0 commit comments

Comments
 (0)