feat(bug-detectors): replace RCE substring check with global canary

docs/bug-detectors.md

@@ -100,9 +100,11 @@ using Jest in `.jazzerjsrc.json`:
 
 ## Remote Code Execution
 
-Hooks the `eval` and `Function` functions and reports a finding if the fuzzer
-was able to pass a special string to `eval` and to the function body of
-`Function`.
+Installs a canary getter on `globalThis` and hooks the `eval` and `Function`
+functions. The before-hooks guide the fuzzer toward injecting the canary
+identifier into code strings. A finding is reported when dynamically compiled
+code accesses the canary property, avoiding false positives from the identifier
+merely appearing inside string literals.
 
 _Disable with:_ `--disableBugDetectors=remote-code-execution` in CLI mode; or
 when using Jest in `.jazzerjsrc.json`:

packages/bug-detectors/internal/remote-code-execution.ts

@@ -14,33 +14,70 @@
  * limitations under the License.
  */
 
+import type { Context } from "vm";
+
 import {
+	getJazzerJsGlobal,
 	guideTowardsContainment,
 	reportAndThrowFinding,
 } from "@jazzer.js/core";
-import { callSiteId, registerBeforeHook } from "@jazzer.js/hooking";
+import { registerBeforeHook } from "@jazzer.js/hooking";
+
+const CANARY_NAME = "jaz_zer";
+
+function createCanaryDescriptor(canaryName: string): PropertyDescriptor {
+	return {
+		get() {
+			reportAndThrowFinding(
+				"Remote Code Execution\n" +
+					`    attacker-controlled code accessed globalThis.${canaryName}`,
+			);
+		},
+		enumerable: false,
+		configurable: false,
+	};
+}
+
+function installCanaryIfMissing(
+	target: object,
+	canaryName: string,
+	descriptor: PropertyDescriptor,
+): void {
+	if (Object.getOwnPropertyDescriptor(target, canaryName)) {
+		return;
+	}
+	Object.defineProperty(target, canaryName, descriptor);
+}
+
+// The canary should be present in both globals used by Jazzer.js:
+// - globalThis in CLI mode
+// - vmContext in Jest mode
+const canaryDescriptor = createCanaryDescriptor(CANARY_NAME);
+installCanaryIfMissing(globalThis, CANARY_NAME, canaryDescriptor);
+
+const vmContext = getJazzerJsGlobal<Context>("vmContext");
+if (vmContext) {
+	installCanaryIfMissing(vmContext, CANARY_NAME, canaryDescriptor);
+}
 
-const targetString = "jaz_zer";
+// Guidance: before-hooks steer the fuzzer toward getting the canary name into
+// eval/Function bodies. A finding is reported when compiled code reads it.
 
 registerBeforeHook(
 	"eval",
 	"",
 	false,
-	function beforeEvalHook(_thisPtr: unknown, params: string[], hookId: number) {
+	function beforeEvalHook(
+		_thisPtr: unknown,
+		params: unknown[],
+		hookId: number,
+	) {
 		const code = params[0];
-		// This check will prevent runtime TypeErrors should the user decide to call Function with
-		// non-string arguments.
-		// noinspection SuspiciousTypeOfGuard
-		if (typeof code === "string" && code.includes(targetString)) {
-			reportAndThrowFinding(
-				"Remote Code Execution\n" + `    using eval:\n        '${code}'`,
-			);
+		// eval with non-string arguments is a no-op (returns the argument as-is),
+		// so guidance is only meaningful for actual strings.
+		if (typeof code === "string") {
+			guideTowardsContainment(code, CANARY_NAME, hookId);
 		}
-
-		// Since we do not hook eval using the hooking framework, we have to recompute the
-		// call site ID on every call to eval. This shouldn't be an issue, because eval is
-		// considered evil and should not be called too often, or even better -- not at all!
-		guideTowardsContainment(code, targetString, hookId);
 	},
 );
 
@@ -50,22 +87,27 @@ registerBeforeHook(
 	false,
 	function beforeFunctionHook(
 		_thisPtr: unknown,
-		params: string[],
+		params: unknown[],
 		hookId: number,
 	) {
-		if (params.length > 0) {
-			const functionBody = params[params.length - 1];
+		if (params.length === 0) return;
 
-			// noinspection SuspiciousTypeOfGuard
-			if (typeof functionBody === "string") {
-				if (functionBody.includes(targetString)) {
-					reportAndThrowFinding(
-						"Remote Code Execution\n" +
-							`    using Function:\n        '${functionBody}'`,
-					);
-				}
-				guideTowardsContainment(functionBody, targetString, hookId);
-			}
+		// The Function constructor coerces every argument to string via ToString().
+		// Template engines (e.g. Handlebars) pass non-string objects like SourceNode
+		// whose toString() yields executable code. Coerce here to match V8's
+		// behavior so guidance works for those cases too.
+		const functionBody = params[params.length - 1];
+		if (functionBody == null) return;
+
+		let functionBodySource: string;
+		try {
+			functionBodySource = String(functionBody);
+		} catch {
+			// toString() would also throw inside the Function constructor, so
+			// no code will be compiled, no RCE risk, no guidance needed.
+			return;
 		}
+
+		guideTowardsContainment(functionBodySource, CANARY_NAME, hookId);
 	},
 );