fix(json): bound json_encode depth so a cyclic value raises instead of segfaulting (#730)

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

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 |
 |------|-----------|-------------|

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);

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;

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 ""

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"