Add recursion check to JSON.parse() reviver walk

src-input/duk_bi_json.c

@@ -955,15 +955,18 @@ DUK_LOCAL void duk__json_dec_value(duk_json_dec_ctx *js_ctx) {
 	DUK_UNREACHABLE();
 }
 
-/* Recursive value reviver, implements the Walk() algorithm.  No C recursion
- * check is done here because the initial parsing step will already ensure
- * there is a reasonable limit on C recursion depth and hence object depth.
+/* Recursive value reviver, implements the Walk() algorithm.  The parsing
+ * step ensures there is a reasonable depth limit to the input.  However,
+ * the reviver may create more depth by editing object or array entries, so
+ * we have both C recursion limit and native stack checks here.
  */
 DUK_LOCAL void duk__json_dec_reviver_walk(duk_json_dec_ctx *js_ctx) {
 	duk_hthread *thr = js_ctx->thr;
 	duk_hobject *h;
 	duk_uarridx_t i, arr_len;
 
+	duk__json_dec_objarr_entry(js_ctx);
+
 	DUK_DDD(DUK_DDDPRINT("walk: top=%ld, holder=%!T, name=%!T",
 	                     (long) duk_get_top(thr),
 	                     (duk_tval *) duk_get_tval(thr, -2),
@@ -1042,6 +1045,8 @@ DUK_LOCAL void duk__json_dec_reviver_walk(duk_json_dec_ctx *js_ctx) {
 	duk_insert(thr, -4);  /* -> [ ... reviver holder name val ] */
 	duk_call_method(thr, 2);  /* -> [ ... res ] */
 
+	duk__json_dec_objarr_exit(js_ctx);
+
 	DUK_DDD(DUK_DDDPRINT("walk: top=%ld, result=%!T",
 	                     (long) duk_get_top(thr), (duk_tval *) duk_get_tval(thr, -1)));
 }
@@ -2852,6 +2857,7 @@ void duk_bi_json_parse_helper(duk_hthread *thr,
 	DUK_ASSERT(*(js_ctx->p_end) == 0x00);
 
 	duk__json_dec_value(js_ctx);  /* -> [ ... value ] */
+	DUK_ASSERT(js_ctx->recursion_depth == 0);
 
 	/* Trailing whitespace has been eaten by duk__json_dec_value(), so if
 	 * we're not at end of input here, it's a SyntaxError.
@@ -2876,7 +2882,9 @@ void duk_bi_json_parse_helper(duk_hthread *thr,
 		                     (duk_tval *) duk_get_tval(thr, -2),
 		                     (duk_tval *) duk_get_tval(thr, -1)));
 
+		DUK_ASSERT(js_ctx->recursion_depth == 0);
 		duk__json_dec_reviver_walk(js_ctx);  /* [ ... val root "" ] -> [ ... val val' ] */
+		DUK_ASSERT(js_ctx->recursion_depth == 0);
 		duk_remove_m2(thr);             /* -> [ ... val' ] */
 	} else {
 		DUK_DDD(DUK_DDDPRINT("reviver does not exist or is not callable: %!T",

tests/ecmascript/test-bi-json-dec-reviver-array-rec.js

@@ -0,0 +1,20 @@
+// When we revive array index 0 (and 1), array index 2 is always replaced
+// with a new array.  When we walk that array, the same happens, without
+// limit.
+
+/*===
+RangeError
+done
+===*/
+
+function revive(key, value) {
+    this[2] = [ 0, 0, 0 ];
+    return value;
+}
+
+try {
+    print(JSON.parse('[1, 2, 3]', revive));
+} catch (e) {
+    print(e.name);
+}
+print('done');

tests/ecmascript/test-bi-json-dec-reviver-object-rec.js

@@ -0,0 +1,21 @@
+// When we revive object key .foo, the key .bar is always replaced with a
+// new object.  When we walk that object, the same happens, without limit.
+// Note that the .bar key must be present when the Walk() algorithm first
+// enumerates the target object; new keys are not considered.
+
+/*===
+RangeError
+done
+===*/
+
+function revive(key, value) {
+    this.bar = { foo: 123, bar: 234 };
+    return value;
+}
+
+try {
+    print(JSON.parse('{ "foo": 123, "bar": 234 }', revive));
+} catch (e) {
+    print(e.name);
+}
+print('done');

tests/ecmascript/test-bug-json-reviver-array-recursion.js

@@ -0,0 +1,19 @@
+/*
+ *  https://github.com/svaarala/duktape/issues/2338
+ */
+
+/*===
+RangeError
+done
+===*/
+
+function b() {
+    a = Array(3);
+    this[2] = a;
+}
+try {
+    JSON.parse("[1, 2, []]", b);
+} catch (e) {
+    print(e.name);
+}
+print('done');