fuzz: expand coverage to cgi.go parsing, response headers, and zval persistence

cgi.go

@@ -18,6 +18,7 @@ import (
 	"crypto/tls"
 	"net"
 	"net/http"
+	"path"
 	"path/filepath"
 	"strings"
 	"unicode/utf8"
@@ -338,17 +339,33 @@ func sanitizedPathJoin(root, reqPath string) string {
 		root = "."
 	}
 
-	path := filepath.Join(root, filepath.Clean("/"+reqPath))
+	// reqPath is an HTTP request path: nominally "/"-separated, regardless
+	// of host OS, but an attacker can smuggle literal "\" bytes in it too
+	// (e.g. via %5C). Normalize those to "/" before cleaning: filepath.Join
+	// below runs with the host's native separator semantics, and on
+	// Windows it treats "\" as a separator, so any ".." hidden behind a
+	// backslash must already be collapsed here or it survives path.Clean
+	// (POSIX-only, "\" is just an ordinary byte to it) and escapes root
+	// once filepath.Join resolves it.
+	//
+	// It must be cleaned with the "path" package (POSIX-only), not
+	// "path/filepath": on Windows, filepath.Clean does not treat a
+	// driveless "/"-rooted path as absolute, so a leading ".." isn't
+	// collapsed at the root the way it is on POSIX - it survives into the
+	// joined path instead, also escaping root.
+	cleanedReqPath := filepath.FromSlash(path.Clean("/" + strings.ReplaceAll(reqPath, `\`, "/")))
+
+	joined := filepath.Join(root, cleanedReqPath)
 
 	// filepath.Join also cleans the path, and cleaning strips
 	// the trailing slash, so we need to re-add it afterward.
 	// if the length is 1, then it's a path to the root,
 	// and that should return ".", so we don't append the separator.
 	if strings.HasSuffix(reqPath, "/") && len(reqPath) > 1 {
-		path += separator
+		joined += separator
 	}
 
-	return path
+	return joined
 }
 
 // splitRemoteAddr splits "host:port" leniently: a missing port is accepted.

cgi_test.go

@@ -1,6 +1,7 @@
 package frankenphp
 
 import (
+	"path/filepath"
 	"strings"
 	"testing"
 
@@ -330,3 +331,65 @@ func TestSplitPosSecurityRegressionUnicodeBypass(t *testing.T) {
 		assert.Equalf(t, -1, splitPos(p, split), "payload %q must not be detected as .php", p)
 	}
 }
+
+// FuzzSplitPos guards the byte/rune-boundary arithmetic behind the Unicode
+// case-folding bypasses above (GHSA-3g8v-8r37-cgjm, GHSA-v4h7-cj44-8fc8):
+// splitPos must never return an out-of-bounds position, whatever bytes are
+// thrown at it.
+func FuzzSplitPos(f *testing.F) {
+	f.Add("/path/to/script.php", ".php")
+	f.Add("/path/to/script.php/some/path", ".php")
+	f.Add("/ȺȺȺȺshell.php.txt.php", ".php")
+	f.Add("/shell﹒php", ".php")
+	f.Add("", "")
+
+	f.Fuzz(func(t *testing.T, path, splitMarker string) {
+		pos := splitPos(path, []string{splitMarker})
+		if pos < -1 || pos > len(path) {
+			t.Fatalf("splitPos(%q, %q) returned out-of-bounds position %d for a %d-byte path", path, splitMarker, pos, len(path))
+		}
+	})
+}
+
+// FuzzSanitizedPathJoin checks that the request path can never escape root,
+// however it's mangled.
+func FuzzSanitizedPathJoin(f *testing.F) {
+	f.Add("/var/www/html", "/index.php")
+	f.Add("/var/www/html", "../../etc/passwd")
+	f.Add("/var/www/html", "..\\..\\windows\\win.ini")
+	f.Add("", "/../../../etc/passwd")
+	f.Add("/var/www/html", "")
+
+	f.Fuzz(func(t *testing.T, root, reqPath string) {
+		result := sanitizedPathJoin(root, reqPath)
+
+		cleanRoot := root
+		if cleanRoot == "" {
+			cleanRoot = "."
+		}
+		rel, err := filepath.Rel(cleanRoot, result)
+		if err != nil {
+			// Different volumes on Windows and the like: not a traversal, just
+			// an unrelated path, but it must still not happen for a plain root.
+			return
+		}
+		if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) {
+			t.Fatalf("sanitizedPathJoin(%q, %q) = %q escapes root", root, reqPath, result)
+		}
+	})
+}
+
+// FuzzSplitRemoteAddr guards against panics: it's called from a cgo
+// callback, so a panic here would crash the process.
+func FuzzSplitRemoteAddr(f *testing.F) {
+	f.Add("1.2.3.4:5")
+	f.Add("[::1]:443")
+	f.Add("[fe80::1%eth0]:443")
+	f.Add("[")
+	f.Add("[:9000")
+	f.Add("")
+
+	f.Fuzz(func(t *testing.T, remoteAddr string) {
+		splitRemoteAddr(remoteAddr)
+	})
+}

frankenphp.c

@@ -1056,8 +1056,9 @@ PHP_FUNCTION(frankenphp_test_persist_roundtrip) {
 
   if (!persistent_zval_validate(input)) {
     zend_throw_exception(spl_ce_LogicException,
-                         "persistent_zval: value type not supported "
-                         "(only scalars, arrays, and enums are allowed)",
+                         "persistent_zval: value not supported (only "
+                         "scalars, arrays, and enums are allowed, nested "
+                         "no deeper than PERSISTENT_ZVAL_MAX_DEPTH)",
                          0);
     RETURN_THROWS();
   }

frankenphp_test.go

@@ -7,6 +7,8 @@ package frankenphp_test
 import (
 	"bytes"
 	"context"
+	"encoding/base64"
+	"encoding/json"
 	"errors"
 	"flag"
 	"fmt"
@@ -100,8 +102,10 @@ func runTest(t *testing.T, test func(func(http.ResponseWriter, *http.Request), *
 	wg.Add(opts.nbParallelRequests)
 	for i := 0; i < opts.nbParallelRequests; i++ {
 		go func(i int) {
+			// Deferred so a t.Skip/t.Fatalf from a non-main goroutine (which
+			// triggers runtime.Goexit) still decrements the WaitGroup.
+			defer wg.Done()
 			test(handler, ts, i)
-			wg.Done()
 		}(i)
 	}
 
@@ -1189,6 +1193,60 @@ func FuzzRequest(f *testing.F) {
 	})
 }
 
+// FuzzResponseHeaders exercises add_response_header (frankenphp.c), FrankenPHP's
+// own copy of the response header list into a PHP array. The header line is
+// base64-encoded so arbitrary bytes reach it unmangled by HTTP transport.
+func FuzzResponseHeaders(f *testing.F) {
+	f.Add("X-Foo: bar")
+	f.Add("X-Foo:bar")
+	f.Add("X-Foo :  bar  ")
+	f.Add(":no-name")
+	f.Add("no-colon-at-all")
+	f.Add("X-Foo: ")
+	f.Add("")
+	f.Add(strings.Repeat("X-Foo: bar", 1000))
+
+	f.Fuzz(func(t *testing.T, headerLine string) {
+		runTest(t, func(handler func(http.ResponseWriter, *http.Request), _ *httptest.Server, _ int) {
+			encoded := base64.StdEncoding.EncodeToString([]byte(headerLine))
+			req := httptest.NewRequest("GET", "http://example.com/fuzz-response-header.php?h="+url.QueryEscape(encoded), nil)
+			body, resp := testRequest(req, handler, t)
+
+			assert.Equal(t, 200, resp.StatusCode)
+			assert.True(t, json.Valid([]byte(body)), "frankenphp_response_headers() must always return valid JSON, got: %s", body)
+		}, nil)
+	})
+}
+
+// FuzzPersistZvalRoundtrip exercises zval.h's persistent_zval_persist/
+// _to_request/_free recursive tree walk: FrankenPHP's own mechanism for
+// carrying values across the request/persistent memory boundary (used by
+// worker state), not php-src itself. Nesting depth and width are
+// fuzzer-controlled, since unbounded native recursion (no depth guard) is
+// the interesting bug class here, not the value shapes themselves.
+func FuzzPersistZvalRoundtrip(f *testing.F) {
+	f.Add(0, 1)
+	f.Add(1, 1)
+	f.Add(10, 2)
+	f.Add(100, 1)
+	f.Add(1000, 1)
+	f.Add(-1, -1)
+
+	f.Fuzz(func(t *testing.T, depth, width int) {
+		runTest(t, func(handler func(http.ResponseWriter, *http.Request), _ *httptest.Server, _ int) {
+			req := httptest.NewRequest("GET", fmt.Sprintf("http://example.com/fuzz-persist-roundtrip.php?depth=%d&width=%d", depth, width), nil)
+			body, resp := testRequest(req, handler, t)
+
+			if body == "SKIP" {
+				t.Skip("FRANKENPHP_TEST not set; skipping persistent_zval roundtrip fuzzing")
+			}
+
+			assert.Equal(t, 200, resp.StatusCode)
+			assert.NotContains(t, body, "MISMATCH", "roundtrip changed the value for depth=%d width=%d", depth, width)
+		}, nil)
+	})
+}
+
 func TestSessionHandlerReset_worker(t *testing.T) {
 	runTest(t, func(_ func(http.ResponseWriter, *http.Request), ts *httptest.Server, i int) {
 		// Request 1: Set a custom session handler and start session

testdata/fuzz-persist-roundtrip.php

@@ -0,0 +1,43 @@
+<?php
+
+// Exercises zval.h's persistent_zval_persist/_to_request/_free recursive
+// tree walk (FrankenPHP's own worker-state mechanism, not php-src) via the
+// FRANKENPHP_TEST-only frankenphp_test_persist_roundtrip hook. Nesting
+// depth and width come from the query string so fuzzing controls the shape.
+
+$rt = 'frankenphp_test_persist_roundtrip';
+if (!function_exists($rt)) {
+    echo 'SKIP';
+    return;
+}
+
+$depth = max(0, min((int) ($_GET['depth'] ?? 0), 5000));
+$width = max(1, min((int) ($_GET['width'] ?? 1), 4));
+
+function frankenphp_fuzz_build_nested(int $depth, int $width): mixed
+{
+    if ($depth <= 0) {
+        return 'leaf';
+    }
+
+    // Only the first slot recurses; the rest are cheap leaves. This keeps
+    // the total node count linear in depth * width instead of width**depth
+    // (a naive every-slot-recurses builder would blow past available
+    // memory well before depth=50 at width=2), while still stressing
+    // exactly the same nesting depth per recursive C call.
+    $arr = ['leaf'];
+    for ($i = 1; $i < $width; $i++) {
+        $arr[$i] = 'leaf';
+    }
+    $arr[0] = frankenphp_fuzz_build_nested($depth - 1, $width);
+
+    return $arr;
+}
+
+$value = frankenphp_fuzz_build_nested($depth, $width);
+
+try {
+    echo $rt($value) === $value ? 'OK' : 'MISMATCH';
+} catch (\Throwable $e) {
+    echo 'THROWN:'.get_class($e);
+}

testdata/fuzz-response-header.php

@@ -0,0 +1,17 @@
+<?php
+
+// Exercises add_response_header (frankenphp.c) through FrankenPHP's own
+// frankenphp_response_headers(), not php-src's header() validation itself.
+// The header line is base64-encoded in the query string so arbitrary bytes
+// reach header() unmangled by HTTP transport.
+
+$raw = base64_decode($_GET['h'] ?? '', true);
+if ($raw !== false && $raw !== '') {
+    // Silence header()'s own warnings (e.g. embedded CR/LF); we only care
+    // about what add_response_header does with whatever it accepts.
+    @header($raw);
+}
+
+// Header values may legitimately contain non-UTF-8 bytes; substitute
+// instead of letting json_encode() fail (and echo nothing) on those.
+echo json_encode(frankenphp_response_headers(), JSON_INVALID_UTF8_SUBSTITUTE);

zval.h

@@ -3,7 +3,13 @@
  * Provides a small, self-contained toolkit for moving zval trees across
  * thread boundaries. The supported shape is a whitelist: scalars, arrays,
  * and enums. Everything else is rejected by persistent_zval_validate so
- * callers can fail fast before allocating.
+ * callers can fail fast before allocating. Nesting is also capped there
+ * (PERSISTENT_ZVAL_MAX_DEPTH): persist/free/to_request below recurse once
+ * per nesting level with no guard of their own, so every caller MUST run
+ * persistent_zval_validate first, on the full tree, before calling any of
+ * them - it's the only thing standing between attacker-controlled nesting
+ * depth and a native stack overflow (crashes the whole process, not just
+ * one request).
  *
  * Fast paths:
  *   - Interned strings: shared memory, no copy.
@@ -16,6 +22,14 @@
 
 #include <Zend/zend_enum.h>
 
+/* Conservative on purpose: comfortably below the depth that overflows the
+ * native stack even under sanitizer builds (larger per-frame redzones), far
+ * above any depth a legitimate config/state value would ever need. Matches
+ * the same order of magnitude as PHP's own default nesting caps (e.g.
+ * json_decode()'s default $depth of 512, Xdebug's max_nesting_level of
+ * 256). */
+#define PERSISTENT_ZVAL_MAX_DEPTH 256
+
 /* Enum payload stored in persistent memory: the class name + case name
  * are kept as persistent zend_strings and the case object is re-resolved
  * via zend_lookup_class + zend_enum_get_case_cstr on each read. */
@@ -26,8 +40,13 @@ typedef struct {
 
 /* Whitelist check: only scalars, arrays of allowed values, and enum
  * instances pass. Returns false for objects other than enums, resources,
- * closures, references, etc. */
-static bool persistent_zval_validate(zval *z) {
+ * closures, references, etc. Also enforces PERSISTENT_ZVAL_MAX_DEPTH,
+ * bailing out before recursing further once hit - see the file header for
+ * why this is the only place that's safe to do so. */
+static bool persistent_zval_validate_depth(zval *z, int depth) {
+  if (depth > PERSISTENT_ZVAL_MAX_DEPTH) {
+    return false;
+  }
   switch (Z_TYPE_P(z)) {
   case IS_NULL:
   case IS_FALSE:
@@ -47,7 +66,7 @@ static bool persistent_zval_validate(zval *z) {
       return true;
     zval *val;
     ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(z), val) {
-      if (!persistent_zval_validate(val))
+      if (!persistent_zval_validate_depth(val, depth + 1))
         return false;
     }
     ZEND_HASH_FOREACH_END();
@@ -58,6 +77,10 @@ static bool persistent_zval_validate(zval *z) {
   }
 }
 
+static bool persistent_zval_validate(zval *z) {
+  return persistent_zval_validate_depth(z, 0);
+}
+
 /* Deep-copy a zval from request memory into persistent (pemalloc) memory.
  * Callers must have already passed persistent_zval_validate on src.
  *