bug detectors: builtin hooks use proper callSiteIDs for hookIDs

- hooks in the path traversal bug detector that need several hookIds
now use the callSiteId function

Author: oetr

examples/bug-detectors/path-traversal/fuzz.js

@@ -19,9 +19,6 @@ const JSZip = require("jszip");
 // eslint-disable-next-line @typescript-eslint/no-var-requires
 const path = require("path");
 
-// eslint-disable-next-line @typescript-eslint/no-var-requires
-const { FuzzedDataProvider } = require("@jazzer.js/core");
-
 /**
  * This demonstrates the path traversal bug detector on a vulnerable version of jszip.
  * @param { Buffer } data

packages/bug-detectors/internal/path-traversal.ts

@@ -16,7 +16,7 @@
 
 import { reportFinding } from "../findings";
 import { guideTowardsContainment } from "@jazzer.js/fuzzer";
-import { registerBeforeHook } from "@jazzer.js/hooking";
+import { callSiteId, registerBeforeHook } from "@jazzer.js/hooking";
 
 /**
  * Importing this file adds "before-hooks" for all functions in the built-in `fs`, `fs/promises`, and `path` module and guides
@@ -170,29 +170,32 @@ const functionsWithTwoTargets = [
 
 for (const module of functionsWithTwoTargets) {
 	for (const functionName of module.functionNames) {
-		const beforeHook = (
-			thisPtr: unknown,
-			params: unknown[],
-			hookId: number
-		) => {
-			if (params === undefined || params.length < 2) {
-				return;
-			}
-			// The first two arguments are paths.
-			const firstArgument = params[0] as string;
-			const secondArgument = params[1] as string;
-			if (firstArgument.includes(goal) || secondArgument.includes(goal)) {
-				reportFinding(
-					`Path Traversal in ${functionName}(): called with '${firstArgument}'` +
-						` and '${secondArgument}'`
-				);
-			}
-			guideTowardsContainment(firstArgument, goal, hookId);
-			// We don't want to confuse the fuzzer with the same hookId (used as a program counter (PC)),
-			// so we increment it.
-			guideTowardsContainment(secondArgument, goal, hookId + 1);
+		const makeBeforeHook = (extraHookId: number) => {
+			return (thisPtr: unknown, params: unknown[], hookId: number) => {
+				if (params === undefined || params.length < 2) {
+					return;
+				}
+				// The first two arguments are paths.
+				const firstArgument = params[0] as string;
+				const secondArgument = params[1] as string;
+				if (firstArgument.includes(goal) || secondArgument.includes(goal)) {
+					reportFinding(
+						`Path Traversal in ${functionName}(): called with '${firstArgument}'` +
+							` and '${secondArgument}'`
+					);
+				}
+				guideTowardsContainment(firstArgument, goal, hookId);
+				// We don't want to confuse the fuzzer guidance with the same hookId for both function arguments.
+				// Therefore, we use an extra hookId for the second argument.
+				guideTowardsContainment(secondArgument, goal, extraHookId);
+			};
 		};
 
-		registerBeforeHook(functionName, module.moduleName, false, beforeHook);
+		registerBeforeHook(
+			functionName,
+			module.moduleName,
+			false,
+			makeBeforeHook(callSiteId(functionName, module.moduleName, "secondId"))
+		);
 	}
 }

packages/hooking/manager.ts

@@ -161,41 +161,41 @@ export class HookManager {
 		const hook = this._hooks[id];
 		switch (hook.type) {
 			case HookType.Before:
-				(hook.hookFunction as BeforeHookFn)(thisPtr, params, this.callSiteId());
+				(hook.hookFunction as BeforeHookFn)(thisPtr, params, callSiteId());
 				break;
 			case HookType.Replace:
 				return (hook.hookFunction as ReplaceHookFn)(
 					thisPtr,
 					params,
-					this.callSiteId(),
+					callSiteId(),
 					// eslint-disable-next-line @typescript-eslint/ban-types
 					resultOrOriginalFunction as Function
 				);
 			case HookType.After:
 				(hook.hookFunction as AfterHookFn)(
 					thisPtr,
 					params,
-					this.callSiteId(),
+					callSiteId(),
 					resultOrOriginalFunction
 				);
 		}
 	}
+}
 
-	private callSiteId(): number {
-		const stackTrace = new Error().stack;
-		if (!stackTrace || stackTrace.length === 0) {
-			return 0;
-		}
-		let hash = 0,
-			i,
-			chr;
-		for (i = 0; i < stackTrace.length; i++) {
-			chr = stackTrace.charCodeAt(i);
-			hash = (hash << 5) - hash + chr;
-			hash |= 0; // Convert to 32bit integer
-		}
-		return hash;
+export function callSiteId(...additionalArguments: unknown[]): number {
+	const stackTrace = additionalArguments?.join(",") + new Error().stack;
+	if (!stackTrace || stackTrace.length === 0) {
+		return 0;
+	}
+	let hash = 0,
+		i,
+		chr;
+	for (i = 0; i < stackTrace.length; i++) {
+		chr = stackTrace.charCodeAt(i);
+		hash = (hash << 5) - hash + chr;
+		hash |= 0; // Convert to 32bit integer
 	}
+	return hash;
 }
 
 export const hookManager = new HookManager();
@@ -234,7 +234,7 @@ export function registerAfterHook(
 export async function hookBuiltInFunction(hook: Hook): Promise<void> {
 	const { default: module } = await import(hook.pkg);
 	const originalFn = module[hook.target];
-	const id = hookManager.hookIndex(hook);
+	const id = callSiteId(hookManager.hookIndex(hook), hook.pkg, hook.target);
 	if (hook.type == HookType.Before) {
 		module[hook.target] = (...args: unknown[]) => {
 			(hook.hookFunction as BeforeHookFn)(null, args, id);