Skip to content

Commit 347d192

Browse files
committed
feat(bug-detectors): replace RCE substring check with global canary
The old detector checked whether the target string appeared in eval/Function source text. This false-positived on safely quoted occurrences (e.g. Handlebars' lookupProperty(d, "jaz_zer")). Install a canary function on globalThis that fires only when attacker-controlled code actually executes. The before-hooks now provide fuzzing guidance only (guideTowardsContainment). The canary is propagated to the Jest vm.Context sandbox, following the same pattern as the prototype-pollution detector.
1 parent 5076a58 commit 347d192

File tree

4 files changed

+296
-144
lines changed

4 files changed

+296
-144
lines changed

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

Lines changed: 54 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -14,33 +14,56 @@
1414
* limitations under the License.
1515
*/
1616

17+
import type { Context } from "vm";
18+
1719
import {
20+
getJazzerJsGlobal,
1821
guideTowardsContainment,
1922
reportAndThrowFinding,
2023
} from "@jazzer.js/core";
21-
import { callSiteId, registerBeforeHook } from "@jazzer.js/hooking";
24+
import { registerBeforeHook } from "@jazzer.js/hooking";
25+
26+
const CANARY_NAME = "jaz_zer";
27+
28+
// Global canary that fires when eval'd/Function'd code actually executes the
29+
// identifier, not when it merely appears in source text (no false positives on
30+
// quoted occurrences). Uses a getter so bare reads like `+ (jaz_zer) +` from
31+
// template-engine expression injection also trigger it.
32+
const canaryDescriptor: PropertyDescriptor = {
33+
get() {
34+
reportAndThrowFinding(
35+
"Remote Code Execution\n" +
36+
" attacker-controlled code accessed globalThis.jaz_zer",
37+
);
38+
},
39+
enumerable: false,
40+
configurable: false,
41+
};
42+
43+
Object.defineProperty(globalThis, CANARY_NAME, canaryDescriptor);
2244

23-
const targetString = "jaz_zer";
45+
// In Jest, fuzz targets run inside a vm.Context sandbox with its own global
46+
// scope. The canary must be visible there too, or code compiled by the
47+
// sandbox's Function constructor won't resolve it.
48+
const vmContext = getJazzerJsGlobal<Context>("vmContext");
49+
if (vmContext) {
50+
Object.defineProperty(vmContext, CANARY_NAME, canaryDescriptor);
51+
}
52+
53+
// Guidance: before-hooks steer the fuzzer toward getting the canary name into
54+
// eval/Function bodies. Detection itself is handled entirely by the canary.
2455

2556
registerBeforeHook(
2657
"eval",
2758
"",
2859
false,
2960
function beforeEvalHook(_thisPtr: unknown, params: string[], hookId: number) {
3061
const code = params[0];
31-
// This check will prevent runtime TypeErrors should the user decide to call Function with
32-
// non-string arguments.
33-
// noinspection SuspiciousTypeOfGuard
34-
if (typeof code === "string" && code.includes(targetString)) {
35-
reportAndThrowFinding(
36-
"Remote Code Execution\n" + ` using eval:\n '${code}'`,
37-
);
62+
// eval with non-string arguments is a no-op (returns the argument as-is),
63+
// so guidance is only meaningful for actual strings.
64+
if (typeof code === "string") {
65+
guideTowardsContainment(code, CANARY_NAME, hookId);
3866
}
39-
40-
// Since we do not hook eval using the hooking framework, we have to recompute the
41-
// call site ID on every call to eval. This shouldn't be an issue, because eval is
42-
// considered evil and should not be called too often, or even better -- not at all!
43-
guideTowardsContainment(code, targetString, hookId);
4467
},
4568
);
4669

@@ -53,19 +76,24 @@ registerBeforeHook(
5376
params: string[],
5477
hookId: number,
5578
) {
56-
if (params.length > 0) {
57-
const functionBody = params[params.length - 1];
79+
if (params.length === 0) return;
5880

59-
// noinspection SuspiciousTypeOfGuard
60-
if (typeof functionBody === "string") {
61-
if (functionBody.includes(targetString)) {
62-
reportAndThrowFinding(
63-
"Remote Code Execution\n" +
64-
` using Function:\n '${functionBody}'`,
65-
);
66-
}
67-
guideTowardsContainment(functionBody, targetString, hookId);
68-
}
81+
// The Function constructor coerces every argument to string via ToString().
82+
// Template engines (e.g. Handlebars) pass non-string objects like SourceNode
83+
// whose toString() yields executable code. Coerce here to match V8's
84+
// behavior so guidance works for those cases too.
85+
const lastParam = params[params.length - 1];
86+
if (lastParam == null) return;
87+
88+
let bodyStr: string;
89+
try {
90+
bodyStr = String(lastParam);
91+
} catch {
92+
// toString() would also throw inside the Function constructor, so
93+
// no code will be compiled, no RCE risk, no guidance needed.
94+
return;
6995
}
96+
97+
guideTowardsContainment(bodyStr, CANARY_NAME, hookId);
7098
},
7199
);

0 commit comments

Comments
 (0)