From 165a072b6e737d4e406f0e281b78eedda3a46522 Mon Sep 17 00:00:00 2001 From: InauguralPhysicist Date: Tue, 28 Jul 2026 17:43:19 -0500 Subject: [PATCH] fix(json): bound json_encode depth so a cyclic value raises instead of segfaulting (#730) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The decoder has been bounded at JSON_MAX_DEPTH since #495; the encoder walked the Value graph with no counter and no cycle check. A Value graph CAN contain cycles — the cycle collector exists because it can — and building one takes two lines (dict_set of [d, "self", d]) or happens by accident (append of [a, a]). The walk then recursed until the C stack was gone. The crash was uncatchable: try/catch does nothing against a SIGSEGV. print already handled cycles, which is what made json_encode the surprise. Both directions now share one constant, hoisted above the encoder, so a document that decodes always re-encodes. Exceeding it raises a catchable EK_VALUE error rather than emitting truncated JSON — a silent truncation here would be the exact silent-wrong-answer class this audit is closing. Depth rides the C stack as a parameter rather than a thread-local counter like the decoder's g_json_depth. The decoder can use a counter safely because it has no early return between its ++ and --; this encoder signals failure by returning -1 through every frame, which is precisely the shape where a counter gets skewed by a missed decrement — and a skewed counter fails the NEXT call, not the one that broke it. A parameter cannot leak. Reachability beyond json_encode itself: json_path, and shared_set — an HTTP handler storing a self-referential value took the whole server down. eigs_json_encode now returns NULL on refusal and both ext_http.c callers check it; they previously strlen'd the result unconditionally, so returning NULL without touching them would have traded a stack overflow for a NULL deref. shared_incr needed a mutex unlock on its new early return. Tests extend test_json_depth.eigs (3 -> 9 checks), the file that already owns the decoder half: cyclic dict, cyclic list, over-deep non-cyclic, plus a CONTROL that a 151-deep value still encodes and decodes back — without it a bound of 1 would pass every raise-check — and a round-trip proving the two limits agree. Pre-fix the section exits 139 (SIGSEGV); post-fix 0. Measured with -fstack-usage: 80 B/frame, so 200 levels is ~16 KiB above baseline, against the decoder's 96 B/frame at the same limit. Release 3268/3268, asan-http 3363/3363 leak tally 0, embed_stack_soak (64 KiB) PASS. Closes #730 Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 18 ++++++++++ docs/BUILTINS.md | 13 ++++--- src/builtins.c | 73 +++++++++++++++++++++++++++++++------- src/ext_http.c | 8 +++++ tests/run_all_tests.sh | 8 ++--- tests/test_json_depth.eigs | 56 +++++++++++++++++++++++++++++ 6 files changed, 155 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b580f250..0565fe97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,24 @@ All notable changes to EigenScript are documented here. ### Security +- **`json_encode` had no depth bound, so a cyclic value segfaulted the process + (#730).** The decoder has been bounded at `JSON_MAX_DEPTH` since #495; the + encoder walked the Value graph with no counter and no cycle check. A Value + graph *can* contain cycles — the cycle collector exists because it can — and + building one takes two lines (`dict_set of [d, "self", d]`, or + `append of [a, a]` by accident). The walk then recursed until the C stack was + gone. The crash was **uncatchable**: `try`/`catch` does nothing against a + SIGSEGV. `print` already handled cycles, which made `json_encode` the + surprise. Reachable from `json_path` and, more seriously, from + `shared_set` — an HTTP handler storing a self-referential value took the + whole server down. Both directions now share one limit, so a document that + decodes always re-encodes; exceeding it raises a catchable `EK_VALUE` error + rather than emitting truncated JSON, which would have been the + silent-wrong-answer class this audit was closing. Depth rides the C stack as + a parameter rather than a thread-local counter, so it cannot leak across a + bail-out. `eigs_json_encode` now returns NULL on refusal and its two + `ext_http.c` callers check it instead of `strlen`-ing the result. + - **Content-Length was matched by substring, so a header value could frame the body (#715).** The read loop located the header with a single `strcasestr(reqbuf, "Content-Length:")` over the whole header block, so the diff --git a/docs/BUILTINS.md b/docs/BUILTINS.md index 0414ad8d..90caf929 100644 --- a/docs/BUILTINS.md +++ b/docs/BUILTINS.md @@ -214,8 +214,8 @@ sandbox allowlist can name them) but every call raises `value`: | Name | Signature | Description | |------|-----------|-------------| -| `json_encode` | `json_encode of value` | Serialize value to JSON string | -| `json_decode` | `json_decode of s` | Parse JSON string to value | +| `json_encode` | `json_encode of value` | Serialize value to JSON string. Raises on a value nested deeper than 200 levels — which includes any **cyclic** value (`dict_set of [d, "self", d]`, `append of [a, a]`), since a cycle has no depth. Catchable. | +| `json_decode` | `json_decode of s` | Parse JSON string to value. Raises past the same 200-level limit, so a document that decodes always re-encodes. | | `json_build` | `json_build of [k1, v1, k2, v2, ...]` | Build JSON object from key-value pairs | | `json_raw` | `json_raw of s` | Wrap raw JSON string (skip encoding) | | `json_path` | `json_path of [json_str, "dot.path"]` | Extract nested value by dot-notation path | @@ -558,9 +558,12 @@ shared store. JSON-serialized map living on the `EigsHttpServer`, mutex-guarded. Values cross worker boundaries by being encoded on write and re-parsed on read into a value owned by the caller's state. Function values -can't be stored (encoded as `null` per `json_encode`). Total bytes are -bounded by `EIGS_HTTP_SHARED_MAX_BYTES` (default 64 MiB); over-cap -writes return `null` without mutating. +can't be stored (encoded as `null` per `json_encode`). A cyclic or +over-deep value can't be stored either — `shared_set` rejects it and +`json_encode` raises, rather than the crash that used to take the whole +server down with it. Total bytes are bounded by +`EIGS_HTTP_SHARED_MAX_BYTES` (default 64 MiB); over-cap writes return +`null` without mutating. | Name | Signature | Description | |------|-----------|-------------| diff --git a/src/builtins.c b/src/builtins.c index a852c5ae..c3d67e59 100644 --- a/src/builtins.c +++ b/src/builtins.c @@ -818,11 +818,37 @@ Value* builtin_type(Value *arg) { return make_str("none"); } -static void eigs_json_encode_value(Value *v, strbuf *out) { +/* Bound JSON nesting depth: each array/object descent is a C recursion, so + * untrusted input like "[[[[...]]]]" would otherwise exhaust the C stack and + * crash (SIGSEGV). 200 is far beyond any legitimate document. Shared by the + * decoder (which has enforced it since #495) and the encoder (#730). */ +#define JSON_MAX_DEPTH 200 + +/* The encoder walks a Value graph, and a Value graph can contain cycles — the + * cycle collector exists precisely because it can. `d is {}` then + * `dict_set of [d, "self", d]` is two lines, and `append of [a, a]` builds one + * by accident. Without a bound the walk recurses until the C stack is gone: a + * SIGSEGV, which `try`/`catch` cannot catch because it is not a runtime error. + * + * The decoder has been bounded at JSON_MAX_DEPTH since #495; the encoder is the + * asymmetry. Same limit here so the two directions agree: a document that + * decodes must re-encode. + * + * Depth rides the C stack rather than a thread-local counter — it cannot leak + * across a bail-out and it is reentrant for free. Returns 0, or -1 once the + * limit is hit; every caller propagates so the walk unwinds instead of + * finishing a truncated document. The entry points turn the -1 into a catchable + * rt_error rather than emitting `null`: a silently truncated document here + * would be the exact silent-wrong-answer class the 0.33.0 audit was closing. */ +#define JSON_ENCODE_MAX_DEPTH JSON_MAX_DEPTH + +static int eigs_json_encode_value(Value *v, strbuf *out, int depth) { if (!v || v->type == VAL_NULL || v->type == VAL_FN || v->type == VAL_BUILTIN) { strbuf_append(out, "null"); - return; + return 0; } + if ((v->type == VAL_LIST || v->type == VAL_DICT) && depth >= JSON_ENCODE_MAX_DEPTH) + return -1; switch (v->type) { case VAL_NUM: { double n = v->data.num; @@ -860,7 +886,8 @@ static void eigs_json_encode_value(Value *v, strbuf *out) { strbuf_append_char(out, '['); for (int i = 0; i < v->data.list.count; i++) { if (i > 0) strbuf_append_char(out, ','); - eigs_json_encode_value(v->data.list.items[i], out); + if (eigs_json_encode_value(v->data.list.items[i], out, depth + 1) != 0) + return -1; } strbuf_append_char(out, ']'); break; @@ -887,7 +914,8 @@ static void eigs_json_encode_value(Value *v, strbuf *out) { } strbuf_append_char(out, '"'); strbuf_append_char(out, ':'); - eigs_json_encode_value(v->data.dict.vals[i], out); + if (eigs_json_encode_value(v->data.dict.vals[i], out, depth + 1) != 0) + return -1; } strbuf_append_char(out, '}'); break; @@ -896,21 +924,40 @@ static void eigs_json_encode_value(Value *v, strbuf *out) { strbuf_append(out, "null"); break; } + return 0; +} + +/* Shared by every entry point so the message is written once. */ +static void json_encode_depth_error(const char *who) { + rt_error(EK_VALUE, 0, + "%s: value nests deeper than %d levels " + "(a self-referential value will always exceed this)", + who, JSON_ENCODE_MAX_DEPTH); } Value* builtin_json_encode(Value *arg) { strbuf out; strbuf_init(&out); - eigs_json_encode_value(arg, &out); + if (eigs_json_encode_value(arg, &out, 0) != 0) { + strbuf_free(&out); + json_encode_depth_error("json_encode"); + return make_null(); + } Value *result = make_str(out.data); strbuf_free(&out); return result; } +/* Returns NULL when the value is too deep to encode, having already raised. + * Callers must check — the C-string entry point has no other way to say no. */ char* eigs_json_encode(Value *v) { strbuf out; strbuf_init(&out); - eigs_json_encode_value(v, &out); + if (eigs_json_encode_value(v, &out, 0) != 0) { + strbuf_free(&out); + json_encode_depth_error("json_encode"); + return NULL; + } char *result = xstrdup(out.data); strbuf_free(&out); return result; @@ -1047,11 +1094,8 @@ static Value* eigs_json_parse_number(const char *s, int *pos) { return make_num(d); } -/* Bound JSON nesting depth: each array/object descent is a C recursion, so - * untrusted input like "[[[[...]]]]" would otherwise exhaust the C stack and - * crash (SIGSEGV). 200 is far beyond any legitimate document. */ -#define JSON_MAX_DEPTH 200 -/* g_json_depth lives on EigsThread (Phase 8); bridge macro from eigenscript.h. */ +/* JSON_MAX_DEPTH is defined above the encoder, which shares it (#730). + * g_json_depth lives on EigsThread (Phase 8); bridge macro from eigenscript.h. */ static Value* eigs_json_parse_array(const char *s, int *pos) { (*pos)++; @@ -2789,7 +2833,12 @@ Value* builtin_json_path(Value *arg) { /* For complex types, json_encode them */ strbuf out; strbuf_init(&out); - eigs_json_encode_value(current, &out); + if (eigs_json_encode_value(current, &out, 0) != 0) { + strbuf_free(&out); + val_decref(root); + json_encode_depth_error("json_path"); + return make_null(); + } Value *r = make_str(out.data); strbuf_free(&out); val_decref(root); diff --git a/src/ext_http.c b/src/ext_http.c index d3d8c3e4..78d46bcc 100644 --- a/src/ext_http.c +++ b/src/ext_http.c @@ -529,7 +529,12 @@ Value* builtin_shared_set(Value *arg) { Server *s = eigs_http_active; if (!s) return make_null(); + /* NULL when the value is cyclic or deeper than JSON_MAX_DEPTH (#730). A + * handler storing a self-referential value used to segfault the server + * here; reject the store instead. eigs_json_encode has already raised, so + * the route sees a catchable error rather than a silent no-op. */ char *json = eigs_json_encode(val); + if (!json) return make_null(); long new_json_len = (long)strlen(json); long key_len = (long)strlen(key_v->data.str); long cap = shared_max_bytes(); @@ -601,6 +606,9 @@ Value* builtin_shared_incr(Value *arg) { Value *new_v = make_num(new_val); char *new_json = eigs_json_encode(new_v); val_decref(new_v); + /* A number can't exceed the depth bound, but don't leave a bare strlen on + * a documented-nullable return. */ + if (!new_json) { pthread_mutex_unlock(&s->shared_mu); return make_null(); } long new_json_len = (long)strlen(new_json); long key_len = (long)strlen(key_v->data.str); long old_bytes = (idx >= 0) ? (long)strlen(s->shared[idx].json) : 0; diff --git a/tests/run_all_tests.sh b/tests/run_all_tests.sh index 66bce527..9ab70b9a 100755 --- a/tests/run_all_tests.sh +++ b/tests/run_all_tests.sh @@ -583,13 +583,13 @@ else fi echo "" -echo "[JSON Depth / DoS guard] (3 checks)" +echo "[JSON Depth / DoS guard] (9 checks)" JD_OUTPUT=$(./eigenscript ../tests/test_json_depth.eigs 2>&1) -TOTAL=$((TOTAL + 3)) +TOTAL=$((TOTAL + 9)) if echo "$JD_OUTPUT" | grep -q "All tests passed"; then - echo " PASS: deep-JSON guard + nested parsing"; PASS=$((PASS + 3)) + echo " PASS: deep-JSON guard (decode + encode) + nested parsing"; PASS=$((PASS + 9)) else - echo " FAIL: json-depth (possible crash/regression)"; FAIL=$((FAIL + 3)) + echo " FAIL: json-depth (possible crash/regression)"; FAIL=$((FAIL + 9)) echo "$JD_OUTPUT" | grep -iE "ASSERT|error" | head -5 fi echo "" diff --git a/tests/test_json_depth.eigs b/tests/test_json_depth.eigs index ec884451..1d895d43 100644 --- a/tests/test_json_depth.eigs +++ b/tests/test_json_depth.eigs @@ -31,4 +31,60 @@ assert of [(len of inner) == 2, "second-level array parses"] obj is json_decode of "{\"a\": {\"b\": 42}}" assert of [obj.a.b == 42, "nested object parses"] +# ---- The ENCODER half (#730) ---- +# The decoder was bounded by #495; json_encode was not, so encoding a cyclic +# or over-deep value recursed until the C stack was gone. That is a SIGSEGV, +# which try/catch cannot catch — the process just died. A value cycle takes +# two lines to build and `print` already handles one, so json_encode was the +# surprise. Reaching the line after each catch is the assertion. + +# A self-referential DICT. +cyc_raised is 0 +d is {"x": 1} +dict_set of [d, "self", d] +try: + s is json_encode of d +catch e: + cyc_raised is 1 +assert of [cyc_raised == 1, "cyclic dict raises rather than crashing"] + +# A self-referential LIST — `append of [a, a]` builds one by accident. +list_raised is 0 +a is [1] +append of [a, a] +try: + s2 is json_encode of a +catch e: + list_raised is 1 +assert of [list_raised == 1, "cyclic list raises rather than crashing"] + +# Non-cyclic but past the 200 limit. +deep_enc_raised is 0 +nest is [1] +i2 is 0 +loop while i2 < 300: + nest is ([nest]) + i2 is i2 + 1 +try: + s3 is json_encode of nest +catch e: + deep_enc_raised is 1 +assert of [deep_enc_raised == 1, "over-deep value raises rather than crashing"] + +# CONTROL: a legitimately deep value UNDER the limit must still encode. Without +# this, a bound of 1 would pass every check above. +ok_nest is [1] +i3 is 0 +loop while i3 < 150: + ok_nest is ([ok_nest]) + i3 is i3 + 1 +encoded is json_encode of ok_nest +# 151 levels deep (the seed plus 150 wrappers): 151 '[' + "1" + 151 ']'. +assert of [(len of encoded) == 303, "151-deep value still encodes"] +assert of [(len of (json_decode of encoded)) == 1, "and decodes back"] + +# The two directions agree: what decodes must re-encode. +round is json_encode of (json_decode of "[1,[2,[3]]]") +assert of [round == "[1,[2,[3]]]", "decode/encode round-trips at shared depth"] + print of "All tests passed"