Skip to content

Commit daf019e

Browse files
author
Ralph Küpper
committed
fix(json): parse deep nesting with a heap-backed stack
1 parent 1ee158d commit daf019e

6 files changed

Lines changed: 382 additions & 71 deletions

File tree

crates/perry-runtime/src/json/mod.rs

Lines changed: 75 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -824,7 +824,7 @@ mod tests {
824824
}
825825
}
826826

827-
/// #7792 — deeply nested input must throw, not take the process out.
827+
/// #7792 / #7817 — deeply nested input must not take the process out.
828828
///
829829
/// Both parsers that read the document recurse once per nesting level, so
830830
/// a deep enough document exhausted the stack: SIGSEGV, exit 139, no
@@ -833,7 +833,17 @@ mod tests {
833833
/// document is unusual.
834834
mod nesting_depth {
835835
use super::*;
836-
use crate::json::parser::{nesting_depth_exceeds, MAX_NESTING_DEPTH};
836+
use crate::json::parser::{
837+
nesting_depth_exceeds, MAX_ITERATIVE_NESTING_DEPTH, MAX_RECURSIVE_NESTING_DEPTH,
838+
};
839+
840+
fn nested_arrays(depth: usize, leaf: u8) -> Vec<u8> {
841+
let mut input = Vec::with_capacity(depth * 2 + 1);
842+
input.extend(std::iter::repeat_n(b'[', depth));
843+
input.push(leaf);
844+
input.extend(std::iter::repeat_n(b']', depth));
845+
input
846+
}
837847

838848
#[test]
839849
fn the_scan_counts_only_structural_brackets() {
@@ -855,28 +865,79 @@ mod tests {
855865
assert!(!nesting_depth_exceeds(b"", 0));
856866
}
857867

858-
/// The limit is the point of the change, so pin the boundary itself:
859-
/// one level under passes, one level over is refused.
868+
/// Pin the parser handoff boundary: both sides produce a value.
860869
#[test]
861-
fn parse_refuses_input_past_the_limit_and_accepts_input_under_it() {
862-
let ok_depth = MAX_NESTING_DEPTH - 1;
863-
let mut ok = vec![b'['; ok_depth];
864-
ok.extend(std::iter::repeat(b']').take(ok_depth));
870+
fn parse_switches_to_the_iterative_path_past_the_recursive_threshold() {
871+
let ok_depth = MAX_RECURSIVE_NESTING_DEPTH - 1;
872+
let ok = nested_arrays(ok_depth, b'0');
865873
let text = js_string_from_bytes(ok.as_ptr(), ok.len() as u32);
866874
assert!(
867875
unsafe { js_json_parse_result(text) }.is_ok(),
868-
"input inside the limit must still parse"
876+
"input below the handoff must parse"
869877
);
870878

871-
let deep_depth = MAX_NESTING_DEPTH + 1;
872-
let mut deep = vec![b'['; deep_depth];
873-
deep.extend(std::iter::repeat(b']').take(deep_depth));
879+
let deep_depth = MAX_RECURSIVE_NESTING_DEPTH + 1;
880+
let deep = nested_arrays(deep_depth, b'0');
874881
let text = js_string_from_bytes(deep.as_ptr(), deep.len() as u32);
875882
assert!(
876-
unsafe { js_json_parse_result(text) }.is_err(),
877-
"input past the limit must be refused rather than descended into"
883+
unsafe { js_json_parse_result(text) }.is_ok(),
884+
"input above the handoff must parse through the heap-stack path"
885+
);
886+
}
887+
888+
#[test]
889+
fn parses_three_hundred_thousand_levels_on_a_small_worker_stack() {
890+
const DEPTH: usize = 300_000;
891+
std::thread::Builder::new()
892+
.name("json-deep-worker".into())
893+
.stack_size(2 * 1024 * 1024)
894+
.spawn(|| {
895+
let input = nested_arrays(DEPTH, b'7');
896+
let text = js_string_from_bytes(input.as_ptr(), input.len() as u32);
897+
let mut value = unsafe { js_json_parse_result(text) }
898+
.expect("deep JSON must parse on a worker-sized stack");
899+
900+
for level in 0..DEPTH {
901+
assert!(value.is_pointer(), "level {level} must be an array");
902+
let array = (value.bits() & POINTER_MASK) as *const crate::ArrayHeader;
903+
assert_eq!(unsafe { (*array).length }, 1, "level {level}");
904+
value = crate::array::js_array_get(array, 0);
905+
}
906+
assert_eq!(f64::from_bits(value.bits()), 7.0);
907+
})
908+
.expect("worker thread starts")
909+
.join()
910+
.expect("worker parse does not panic");
911+
}
912+
913+
#[test]
914+
fn rejects_nesting_beyond_the_iterative_resource_budget() {
915+
let input = nested_arrays(MAX_ITERATIVE_NESTING_DEPTH + 1, b'0');
916+
let text = js_string_from_bytes(input.as_ptr(), input.len() as u32);
917+
let error = unsafe { js_json_parse_result(text) }
918+
.expect_err("the iterative path must keep a finite resource budget");
919+
let error = (error.to_bits() & POINTER_MASK) as *const crate::error::ErrorHeader;
920+
assert_eq!(
921+
unsafe { (*error).error_kind },
922+
crate::error::ERROR_KIND_RANGE_ERROR
878923
);
879924
}
925+
926+
#[test]
927+
fn iterative_path_still_rejects_malformed_json() {
928+
let depth = MAX_RECURSIVE_NESTING_DEPTH + 1;
929+
let mut trailing = nested_arrays(depth, b'0');
930+
trailing.push(b'x');
931+
let mut invalid_number = Vec::with_capacity(depth * 2 + 2);
932+
invalid_number.extend(std::iter::repeat_n(b'[', depth));
933+
invalid_number.extend_from_slice(b"01");
934+
invalid_number.extend(std::iter::repeat_n(b']', depth));
935+
936+
for input in [trailing, invalid_number] {
937+
let text = js_string_from_bytes(input.as_ptr(), input.len() as u32);
938+
assert!(unsafe { js_json_parse_result(text) }.is_err());
939+
}
940+
}
880941
}
881942

882943
#[test]

crates/perry-runtime/src/json/parse_api.rs

Lines changed: 81 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,6 @@ fn syntax_error_value(message: &str) -> f64 {
6969
f64::from_bits(JSValue::pointer(err as *const u8).bits())
7070
}
7171

72-
/// A catchable `RangeError`, for the one JSON failure that is about size
73-
/// rather than shape: input nested deeper than the parser can descend.
7472
fn range_error_value(message: &str) -> f64 {
7573
let msg_ptr = js_string_from_bytes(message.as_ptr(), message.len() as u32);
7674
let err = crate::error::js_rangeerror_new(msg_ptr);
@@ -85,21 +83,32 @@ fn throw_range_error(message: &str) -> ! {
8583
crate::exception::js_throw(range_error_value(message))
8684
}
8785

88-
/// The one depth check, called by every entry that is about to descend.
86+
/// Select the heap-stack parser before recursive validation or materialization
87+
/// gets close to the smallest worker-thread stack.
8988
///
9089
/// `js_json_parse` and `js_json_parse_result` are separate implementations of
9190
/// the same flow, and the typed-array path is a third. Sharing the decision is
9291
/// what keeps them from drifting — the first version of this fix guarded only
9392
/// one of the three and appeared to do nothing at all, because the entry point
9493
/// codegen actually calls was one of the other two.
95-
fn nesting_is_too_deep(bytes: &[u8]) -> bool {
96-
crate::json::parser::nesting_depth_exceeds(bytes, crate::json::parser::MAX_NESTING_DEPTH)
94+
fn requires_iterative_parse(bytes: &[u8]) -> bool {
95+
crate::json::parser::nesting_depth_exceeds(
96+
bytes,
97+
crate::json::parser::MAX_RECURSIVE_NESTING_DEPTH,
98+
)
9799
}
98100

99-
fn too_deep_message() -> String {
101+
fn exceeds_iterative_budget(bytes: &[u8]) -> bool {
102+
crate::json::parser::nesting_depth_exceeds(
103+
bytes,
104+
crate::json::parser::MAX_ITERATIVE_NESTING_DEPTH,
105+
)
106+
}
107+
108+
fn iterative_budget_message() -> String {
100109
format!(
101-
"JSON.parse: input nested deeper than {} levels",
102-
crate::json::parser::MAX_NESTING_DEPTH
110+
"JSON.parse: input exceeds the {}-level iterative nesting budget",
111+
crate::json::parser::MAX_ITERATIVE_NESTING_DEPTH
103112
)
104113
}
105114

@@ -118,6 +127,53 @@ fn is_json_null_literal(bytes: &[u8]) -> bool {
118127
&bytes[start..end] == b"null"
119128
}
120129

130+
/// Parse a deeply nested document through the flat tape representation. Tape
131+
/// construction validates syntax with an explicit heap stack; materialization
132+
/// likewise keeps pending containers on the heap. This path runs only beyond
133+
/// the recursive fast path's safe depth, so ordinary JSON keeps its existing
134+
/// allocation and shape-specialization behavior.
135+
unsafe fn try_parse_deep_iterative(
136+
text_ptr: *const StringHeader,
137+
len: usize,
138+
bytes: &[u8],
139+
) -> Option<JSValue> {
140+
let text_root = parse_root_push(JSValue::string_ptr(text_ptr as *mut StringHeader));
141+
let result = crate::json_tape::with_built_tape(bytes, |tape_entries| {
142+
crate::gc::gc_collect_pending_suppressed_parse();
143+
crate::gc::gc_check_trigger();
144+
crate::gc::gc_suppress();
145+
146+
let bytes = {
147+
let moved = parse_root_get(text_root);
148+
let hdr = moved.as_string_ptr();
149+
let data_ptr = (hdr as *const u8).add(std::mem::size_of::<StringHeader>());
150+
std::slice::from_raw_parts(data_ptr, len)
151+
};
152+
let result = crate::json_tape::materialize_iterative(tape_entries, bytes);
153+
if let Some(value) = result {
154+
parse_root_push(value);
155+
}
156+
157+
crate::gc::gc_unsuppress();
158+
crate::gc::gc_bump_malloc_trigger();
159+
crate::gc::gc_schedule_parse_boundary_collection_if_pressure();
160+
result
161+
})
162+
.flatten();
163+
parse_root_restore(text_root);
164+
165+
PARSE_KEY_CACHE.with(|cell| {
166+
let cache = cell.borrow();
167+
if cache.len() > 4096 {
168+
drop(cache);
169+
cell.borrow_mut().clear();
170+
clear_parse_key_ring();
171+
}
172+
});
173+
174+
result
175+
}
176+
121177
/// Non-throwing JSON parse entry for APIs that must reject a Promise rather than
122178
/// synchronously throwing through `JSON.parse`'s FFI boundary.
123179
///
@@ -136,12 +192,12 @@ pub unsafe fn js_json_parse_result(text_ptr: *const StringHeader) -> Result<JSVa
136192
return Err(syntax_error_value("Unexpected end of JSON input"));
137193
}
138194

139-
// #7792: depth first, BEFORE the validation pass below. That pass recurses
140-
// once per nesting level itself, so a check placed after it would run after
141-
// the crash it exists to prevent. The scan is one linear pass over bytes we
142-
// are about to read anyway.
143-
if nesting_is_too_deep(bytes) {
144-
return Err(range_error_value(&too_deep_message()));
195+
if requires_iterative_parse(bytes) {
196+
if exceeds_iterative_budget(bytes) {
197+
return Err(range_error_value(&iterative_budget_message()));
198+
}
199+
return try_parse_deep_iterative(text_ptr, len, bytes)
200+
.ok_or_else(|| syntax_error_value("JSON parse error: malformed deep document"));
145201
}
146202

147203
// Validate without constructing a second full JSON tree. The Perry parser
@@ -236,11 +292,14 @@ pub unsafe extern "C" fn js_json_parse(text_ptr: *const StringHeader) -> JSValue
236292
if len == 0 {
237293
throw_syntax_error("Unexpected end of JSON input");
238294
}
239-
// #7792: depth first, ahead of the validation pass, for the same reason as
240-
// the `_result` twin above. This is the entry codegen emits, so a guard
241-
// that covered only the twin covered nothing a compiled program can reach.
242-
if nesting_is_too_deep(bytes) {
243-
throw_range_error(&too_deep_message());
295+
if requires_iterative_parse(bytes) {
296+
if exceeds_iterative_budget(bytes) {
297+
throw_range_error(&iterative_budget_message());
298+
}
299+
return match try_parse_deep_iterative(text_ptr, len, bytes) {
300+
Some(value) => value,
301+
None => throw_syntax_error("JSON parse error: malformed deep document"),
302+
};
244303
}
245304
// Keep serde_json's strict syntax validation, but discard tokens as they
246305
// are read instead of allocating an intermediate `serde_json::Value`
@@ -559,10 +618,9 @@ pub unsafe extern "C" fn js_json_parse_typed_array(
559618
let data_ptr = (text_ptr as *const u8).add(std::mem::size_of::<StringHeader>());
560619
let bytes = std::slice::from_raw_parts(data_ptr, len);
561620

562-
// #7792: this path builds its own parser, so it needs its own guard. Hand
563-
// deep input to the generic entry rather than repeating the error here, so
564-
// both report it identically.
565-
if nesting_is_too_deep(bytes) {
621+
// Deep input uses the generic entry's heap-stack fallback. The shape fast
622+
// path is deliberately retained for ordinary payloads only.
623+
if requires_iterative_parse(bytes) {
566624
return js_json_parse(text_ptr);
567625
}
568626

crates/perry-runtime/src/json/parser.rs

Lines changed: 12 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -56,30 +56,20 @@ pub(crate) struct ObjectShapeHint {
5656
pub(crate) field_count: u32,
5757
}
5858

59-
/// The deepest `[`/`{` nesting `JSON.parse` will accept.
59+
/// The deepest `[`/`{` nesting handled by the recursive fast path.
6060
///
61-
/// Both parsers that see the input recurse once per level — the `serde_json`
62-
/// validation pass and Perry's own value parser — so a deep enough document
63-
/// exhausts the stack and takes the whole process out with SIGSEGV, no
64-
/// diagnostic and no output, on input that is very often attacker-supplied
65-
/// (#7792). Measured on a default 8 MB main-thread stack the crash lands
66-
/// between 20,000 and 40,000 levels — but that is the most generous stack in
67-
/// the process, and it is the wrong one to size against. Perry parses JSON on
68-
/// `perry/thread` workers and tokio workers too, and a 2 MiB thread stack
69-
/// overflows well before 10,000 levels: a first attempt at this limit picked
70-
/// 10,000 off the main-thread measurement, and the unit test below promptly
71-
/// crashed the test harness at 9,999.
61+
/// Both the `serde_json` validation pass and Perry's direct value parser recurse
62+
/// once per container, so their cutoff is sized for Perry's smallest worker
63+
/// stack rather than the main thread's larger stack (#7792).
7264
///
73-
/// So the limit is sized for the SMALLEST stack in the process, not the
74-
/// largest, and 1,000 is the same depth Python's parser has settled on. Real
75-
/// documents do not come close: JSON nested past a hundred levels is already
76-
/// unusual, and past a thousand is a machine talking to itself.
77-
///
78-
/// This is a deliberate parity gap. Node parses far deeper than this because
79-
/// V8's parser is iterative and does not consume stack per level; matching it
80-
/// means making this parser iterative too, which is the follow-up. Until then
81-
/// a catchable error beats a SIGSEGV on untrusted input.
82-
pub(crate) const MAX_NESTING_DEPTH: usize = 1_000;
65+
/// Deeper documents switch to the flat-tape parser and iterative materializer,
66+
/// so this is a native-stack safety threshold rather than an input limit.
67+
pub(crate) const MAX_RECURSIVE_NESTING_DEPTH: usize = 1_000;
68+
69+
/// Heap-stack safety ceiling. This remains well above Node-parity cases such as
70+
/// #7817's 300,000-level document, while bounding the tape, pending-frame stack,
71+
/// and runtime-container amplification for unusually deep input.
72+
pub(crate) const MAX_ITERATIVE_NESTING_DEPTH: usize = 500_000;
8373

8474
/// Does `bytes` nest deeper than `limit`?
8575
///

0 commit comments

Comments
 (0)