Skip to content

Commit f0734f5

Browse files
committed
security: cap persistent_zval nesting depth, add fuzz target
persistent_zval_persist/_to_request/_free (zval.h) recurse once per nesting level with no depth guard. A plain linear chain of nested single-element arrays crashes the process (SIGBUS, native stack overflow) around depth ~700 on a local debug build; sanitizer builds, with much larger per-frame redzones, would hit it shallower still. Not reachable today - the only caller is the FRANKENPHP_TEST-only roundtrip hook, and zval.h itself is only compiled in under that guard, pending the first real caller (background workers, per the comment at its include site) - but it's a live landmine for whenever that lands: a native stack overflow there kills the whole process, not just one request. persistent_zval_validate is the one gate every caller already runs before persist/free/to_request, so it's the only safe place to reject excess depth: rejecting there means persist never starts, so there's no partially-persisted tree to unwind on the error path. Picked 256 as the cap, the same order of magnitude as PHP's own defaults (json_decode()'s $depth, Xdebug's max_nesting_level). The new fuzz target's own builder script needed a fix too: growing every slot at every level makes the tree size width**depth, which blows past available memory (and hangs the fuzzer) well under the depth needed to threaten the stack; only the first slot per level now recurses, so total size is depth*width instead.
1 parent 7febf3d commit f0734f5

4 files changed

Lines changed: 102 additions & 6 deletions

File tree

frankenphp.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,8 +1056,9 @@ PHP_FUNCTION(frankenphp_test_persist_roundtrip) {
10561056

10571057
if (!persistent_zval_validate(input)) {
10581058
zend_throw_exception(spl_ce_LogicException,
1059-
"persistent_zval: value type not supported "
1060-
"(only scalars, arrays, and enums are allowed)",
1059+
"persistent_zval: value not supported (only "
1060+
"scalars, arrays, and enums are allowed, nested "
1061+
"no deeper than PERSISTENT_ZVAL_MAX_DEPTH)",
10611062
0);
10621063
RETURN_THROWS();
10631064
}

frankenphp_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1218,6 +1218,35 @@ func FuzzResponseHeaders(f *testing.F) {
12181218
})
12191219
}
12201220

1221+
// FuzzPersistZvalRoundtrip exercises zval.h's persistent_zval_persist/
1222+
// _to_request/_free recursive tree walk: FrankenPHP's own mechanism for
1223+
// carrying values across the request/persistent memory boundary (used by
1224+
// worker state), not php-src itself. Nesting depth and width are
1225+
// fuzzer-controlled, since unbounded native recursion (no depth guard) is
1226+
// the interesting bug class here, not the value shapes themselves.
1227+
func FuzzPersistZvalRoundtrip(f *testing.F) {
1228+
f.Add(0, 1)
1229+
f.Add(1, 1)
1230+
f.Add(10, 2)
1231+
f.Add(100, 1)
1232+
f.Add(1000, 1)
1233+
f.Add(-1, -1)
1234+
1235+
f.Fuzz(func(t *testing.T, depth, width int) {
1236+
runTest(t, func(handler func(http.ResponseWriter, *http.Request), _ *httptest.Server, _ int) {
1237+
req := httptest.NewRequest("GET", fmt.Sprintf("http://example.com/fuzz-persist-roundtrip.php?depth=%d&width=%d", depth, width), nil)
1238+
body, resp := testRequest(req, handler, t)
1239+
1240+
if body == "SKIP" {
1241+
t.Skip("FRANKENPHP_TEST not set; skipping persistent_zval roundtrip fuzzing")
1242+
}
1243+
1244+
assert.Equal(t, 200, resp.StatusCode)
1245+
assert.NotContains(t, body, "MISMATCH", "roundtrip changed the value for depth=%d width=%d", depth, width)
1246+
}, nil)
1247+
})
1248+
}
1249+
12211250
func TestSessionHandlerReset_worker(t *testing.T) {
12221251
runTest(t, func(_ func(http.ResponseWriter, *http.Request), ts *httptest.Server, i int) {
12231252
// Request 1: Set a custom session handler and start session
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<?php
2+
3+
// Exercises zval.h's persistent_zval_persist/_to_request/_free recursive
4+
// tree walk (FrankenPHP's own worker-state mechanism, not php-src) via the
5+
// FRANKENPHP_TEST-only frankenphp_test_persist_roundtrip hook. Nesting
6+
// depth and width come from the query string so fuzzing controls the shape.
7+
8+
$rt = 'frankenphp_test_persist_roundtrip';
9+
if (!function_exists($rt)) {
10+
echo 'SKIP';
11+
return;
12+
}
13+
14+
$depth = max(0, min((int) ($_GET['depth'] ?? 0), 5000));
15+
$width = max(1, min((int) ($_GET['width'] ?? 1), 4));
16+
17+
function frankenphp_fuzz_build_nested(int $depth, int $width): mixed
18+
{
19+
if ($depth <= 0) {
20+
return 'leaf';
21+
}
22+
23+
// Only the first slot recurses; the rest are cheap leaves. This keeps
24+
// the total node count linear in depth * width instead of width**depth
25+
// (a naive every-slot-recurses builder would blow past available
26+
// memory well before depth=50 at width=2), while still stressing
27+
// exactly the same nesting depth per recursive C call.
28+
$arr = ['leaf'];
29+
for ($i = 1; $i < $width; $i++) {
30+
$arr[$i] = 'leaf';
31+
}
32+
$arr[0] = frankenphp_fuzz_build_nested($depth - 1, $width);
33+
34+
return $arr;
35+
}
36+
37+
$value = frankenphp_fuzz_build_nested($depth, $width);
38+
39+
try {
40+
echo $rt($value) === $value ? 'OK' : 'MISMATCH';
41+
} catch (\Throwable $e) {
42+
echo 'THROWN:'.get_class($e);
43+
}

zval.h

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,13 @@
33
* Provides a small, self-contained toolkit for moving zval trees across
44
* thread boundaries. The supported shape is a whitelist: scalars, arrays,
55
* and enums. Everything else is rejected by persistent_zval_validate so
6-
* callers can fail fast before allocating.
6+
* callers can fail fast before allocating. Nesting is also capped there
7+
* (PERSISTENT_ZVAL_MAX_DEPTH): persist/free/to_request below recurse once
8+
* per nesting level with no guard of their own, so every caller MUST run
9+
* persistent_zval_validate first, on the full tree, before calling any of
10+
* them - it's the only thing standing between attacker-controlled nesting
11+
* depth and a native stack overflow (crashes the whole process, not just
12+
* one request).
713
*
814
* Fast paths:
915
* - Interned strings: shared memory, no copy.
@@ -16,6 +22,14 @@
1622

1723
#include <Zend/zend_enum.h>
1824

25+
/* Conservative on purpose: comfortably below the depth that overflows the
26+
* native stack even under sanitizer builds (larger per-frame redzones), far
27+
* above any depth a legitimate config/state value would ever need. Matches
28+
* the same order of magnitude as PHP's own default nesting caps (e.g.
29+
* json_decode()'s default $depth of 512, Xdebug's max_nesting_level of
30+
* 256). */
31+
#define PERSISTENT_ZVAL_MAX_DEPTH 256
32+
1933
/* Enum payload stored in persistent memory: the class name + case name
2034
* are kept as persistent zend_strings and the case object is re-resolved
2135
* via zend_lookup_class + zend_enum_get_case_cstr on each read. */
@@ -26,8 +40,13 @@ typedef struct {
2640

2741
/* Whitelist check: only scalars, arrays of allowed values, and enum
2842
* instances pass. Returns false for objects other than enums, resources,
29-
* closures, references, etc. */
30-
static bool persistent_zval_validate(zval *z) {
43+
* closures, references, etc. Also enforces PERSISTENT_ZVAL_MAX_DEPTH,
44+
* bailing out before recursing further once hit - see the file header for
45+
* why this is the only place that's safe to do so. */
46+
static bool persistent_zval_validate_depth(zval *z, int depth) {
47+
if (depth > PERSISTENT_ZVAL_MAX_DEPTH) {
48+
return false;
49+
}
3150
switch (Z_TYPE_P(z)) {
3251
case IS_NULL:
3352
case IS_FALSE:
@@ -47,7 +66,7 @@ static bool persistent_zval_validate(zval *z) {
4766
return true;
4867
zval *val;
4968
ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(z), val) {
50-
if (!persistent_zval_validate(val))
69+
if (!persistent_zval_validate_depth(val, depth + 1))
5170
return false;
5271
}
5372
ZEND_HASH_FOREACH_END();
@@ -58,6 +77,10 @@ static bool persistent_zval_validate(zval *z) {
5877
}
5978
}
6079

80+
static bool persistent_zval_validate(zval *z) {
81+
return persistent_zval_validate_depth(z, 0);
82+
}
83+
6184
/* Deep-copy a zval from request memory into persistent (pemalloc) memory.
6285
* Callers must have already passed persistent_zval_validate on src.
6386
*

0 commit comments

Comments
 (0)