Skip to content

Heap out-of-bounds read in mystrtod (counted-buffer over-read on a truncated number) #77

Description

@eobi

Summary

mjson(const char *s, int len, ...) is a length-bounded API and honors len throughout its main loop, except the number path, which calls mystrtod() with only a pointer and no length. mystrtod walks forward relying on a NUL terminator. When the input buffer is not NUL-terminated (which the counted-length API permits) and ends inside or at the start of a number, mystrtod reads past s + len. CWE-125, AddressSanitizer-confirmed.

This is reachable in Mongoose, which parses JSON directly from network buffers (mg_str = pointer + length, not guaranteed NUL-terminated); a remote peer can send JSON ending in a bare -, 1e, etc.

Affected: current master (commit 696969c). Distinct from CVE-2023-30421, which is an algorithmic-complexity DoS in the same function (a different bug class); this is a memory-safety over-read.

Root cause
Call site, mjson.c:114:

} else if (c == '-' || ((c >= '0' && c <= '9'))) {
const char *end = NULL;
mystrtod(&s[i], &end); // no length passed
...
mystrtod (around mjson.c:741) reads *p for the sign and is_digit(*p), and loops while (*p && is_digit(*p)), bounded only by an assumed NUL. ASan reports a heap-buffer-overflow READ of size 1 at mjson.c:753 via mjson (mjson.c:114).

Reproducer (self-contained, no fuzzer)

/* cc -fsanitize=address -O0 poc.c mjson.c -o poc ; ./poc /
#include <stdint.h>
#include <stdlib.h>
#include <string.h>
#include "mjson.h"
static int cb(int e,const char
b,int o,int l,voidu){
(void)e;(void)b;(void)o;(void)l;(void)u; return 0; }
int main(void){
const char in[] = {'-'}; /
NOT NUL-terminated; exactly 1 byte /
char buf = (char)malloc(sizeof in); /
exact length so the over-read faults */
memcpy(buf, in, sizeof in);
mjson(buf, (int)sizeof in, cb, 0);
free(buf);
return 0;
}
Output: AddressSanitizer: heap-buffer-overflow READ of size 1 ... in mystrtod. Trigger: any counted, non-NUL-terminated buffer ending in a truncated number; minimal input is the single byte 0x2D (-). Also -., 1e, 0e-, [-, {"a":-.

Impact
Out-of-bounds read only: a crash (DoS) or adjacent-heap disclosure. No out-of-bounds write, no code execution. Remotely reachable in Mongoose-based services that parse untrusted, non-NUL-terminated JSON.

Suggested fix
Respect the counted length in the number path (never read past s + len). Either give mystrtod an end/length bound tested before every *p dereference, or bound the number span at the call site and parse a NUL-terminated copy:

} else if (c == '-' || (c >= '0' && c <= '9')) {
const char *end = NULL; char numbuf[64]; int nb = 0, j = i;
while (j < len && nb < 63 && (s[j]=='-'||s[j]=='+'||s[j]=='.'||
s[j]=='e'||s[j]=='E'||(s[j]>='0'&&s[j]<='9'))) numbuf[nb++] = s[j++];
numbuf[nb] = 0;
mystrtod(numbuf, &end);
if (end != NULL) i += (int)(end - numbuf - 1);
tok = MJSON_TOK_NUMBER;
}

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions