Skip to content

Commit cf75218

Browse files
authored
feat: implement new default isMonkeyPatch probe with monkey-patch warning (#485)
1 parent 79d20c4 commit cf75218

File tree

9 files changed

+268
-14
lines changed

9 files changed

+268
-14
lines changed

.changeset/sparkly-pugs-doubt.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@nodesecure/js-x-ray": minor
3+
---
4+
5+
Implement a new monkey-patch warning/probe

README.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,8 @@ type WarningName = "parsing-error"
134134
| "shady-link"
135135
| "synchronous-io"
136136
| "log-usage"
137-
| "serialize-environment";
137+
| "serialize-environment"
138+
| "monkey-patch";
138139

139140
declare const warnings: Record<WarningName, {
140141
i18n: string;
@@ -175,7 +176,7 @@ This section describes all the possible warnings returned by JSXRay. Click on th
175176
| [data-exfiltration](./docs/data-exfiltration.md) || the code potentially attemps to transfer sensitive data wihtout authorization from a computer or network to an external location. |
176177
| [log-usage](./docs/log-usage.md) || The code contains a log call. |
177178
| [sql-injection](./docs/sql-injection.md) || The code contains a SQL injection vulnerability |
178-
179+
| [monkey-patch](./docs/monkey-patch.md) || The code alters built-in JavaScript prototype properties |
179180

180181
## Workspaces
181182

docs/monkey-patch.md

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
# Monkey patch
2+
3+
| Code | Severity | i18n | Experimental |
4+
| --- | --- | --- | :-: |
5+
| monkey-patch | `Warning` | `sast_warnings.monkey_patch` ||
6+
7+
## Introduction
8+
9+
Monkey-patching involves modifying native language objects (prototypes, global functions) at runtime to alter their behavior. While it can serve legitimate purposes like polyfills or extending APIs, it introduces significant security risks: breaking invariants, global side effects, flow hijacking (hooking), stealthy persistence, and concealing malicious activities.
10+
11+
JS-X-Ray raises a `monkey-patch` warning when it detects writes to native prototypes. The signal is intentionally broad to facilitate review: while some legitimate uses exist, any invasive modification deserves inspection.
12+
13+
## Examples
14+
15+
```js
16+
Array.prototype.map = function() {
17+
// alters global map() behavior
18+
};
19+
20+
Object.defineProperty(String.prototype, "replace", {
21+
configurable: true,
22+
enumerable: false,
23+
writable: true,
24+
value: function replacer(search, replaceWith) {
25+
// systematic interception of all replace() calls
26+
}
27+
});
28+
```

workspaces/js-x-ray/src/ProbeRunner.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import isSyncIO from "./probes/isSyncIO.ts";
2626
import isUnsafeCallee from "./probes/isUnsafeCallee.ts";
2727
import isUnsafeCommand from "./probes/isUnsafeCommand.ts";
2828
import isWeakCrypto from "./probes/isWeakCrypto.ts";
29+
import isMonkeyPatch from "./probes/isMonkeyPatch.ts";
2930

3031
import type { SourceFile } from "./SourceFile.ts";
3132
import type { OptionalWarningName } from "./warnings.ts";
@@ -100,7 +101,8 @@ export class ProbeRunner {
100101
isUnsafeCommand,
101102
isSerializeEnv,
102103
dataExfiltration,
103-
sqlInjection
104+
sqlInjection,
105+
isMonkeyPatch
104106
];
105107

106108
static Optionals: Record<OptionalWarningName, Probe> = {
Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
// Import Third-party Dependencies
2+
import type { ESTree } from "meriyah";
3+
import {
4+
getCallExpressionIdentifier,
5+
getMemberExpressionIdentifier
6+
} from "@nodesecure/estree-ast-utils";
7+
8+
// Import Internal Dependencies
9+
import type {
10+
ProbeMainContext,
11+
ProbeContext
12+
} from "../ProbeRunner.ts";
13+
import { generateWarning } from "../warnings.ts";
14+
15+
// CONSTANTS
16+
export const JS_TYPES = new Set([
17+
"AggregateError",
18+
"Array",
19+
"ArrayBuffer",
20+
"BigInt",
21+
"BigInt64Array",
22+
"BigUint64Array",
23+
"Boolean",
24+
"DataView",
25+
"Date",
26+
"Error",
27+
"EvalError",
28+
"FinalizationRegistry",
29+
"Float32Array",
30+
"Float64Array",
31+
"Function",
32+
"Int16Array",
33+
"Int32Array",
34+
"Int8Array",
35+
"Map",
36+
"Number",
37+
"Object",
38+
"Promise",
39+
"Proxy",
40+
"RangeError",
41+
"ReferenceError",
42+
"Reflect",
43+
"RegExp",
44+
"Set",
45+
"SharedArrayBuffer",
46+
"String",
47+
"Symbol",
48+
"SyntaxError",
49+
"TypeError",
50+
"Uint16Array",
51+
"Uint32Array",
52+
"Uint8Array",
53+
"Uint8ClampedArray",
54+
"URIError",
55+
"WeakMap",
56+
"WeakRef",
57+
"WeakSet"
58+
]);
59+
60+
/**
61+
* @description Search for monkey patching of built-in prototypes.
62+
* @example
63+
* Array.prototype.map = function() {};
64+
*/
65+
function validateNodeAssignment(
66+
node: ESTree.Node,
67+
ctx: ProbeContext
68+
): [boolean, any?] {
69+
if (
70+
node.type !== "AssignmentExpression" ||
71+
node.left.type !== "MemberExpression"
72+
) {
73+
return [false];
74+
}
75+
76+
return validateMemberExpression(node.left, ctx);
77+
}
78+
79+
function validateDefineProperty(
80+
node: ESTree.Node,
81+
ctx: ProbeContext
82+
): [boolean, any?] {
83+
if (node.type !== "CallExpression") {
84+
return [false];
85+
}
86+
const id = getCallExpressionIdentifier(node);
87+
88+
if (
89+
(id !== "Object.defineProperty" && id !== "Reflect.defineProperty")
90+
) {
91+
return [false];
92+
}
93+
94+
const firstArg = node.arguments.at(0);
95+
if (firstArg?.type !== "MemberExpression") {
96+
return [false];
97+
}
98+
99+
return validateMemberExpression(firstArg, ctx);
100+
}
101+
102+
function validateMemberExpression(
103+
node: ESTree.MemberExpression,
104+
ctx: ProbeContext
105+
): [boolean, any?] {
106+
const iter = getMemberExpressionIdentifier(node, {
107+
externalIdentifierLookup: (name: string) => ctx.sourceFile.tracer.literalIdentifiers.get(name) ?? null
108+
});
109+
110+
const jsTypeName = iter.next().value;
111+
if (typeof jsTypeName !== "string" || !JS_TYPES.has(jsTypeName)) {
112+
return [false];
113+
}
114+
115+
return [
116+
iter.next().value === "prototype",
117+
`${jsTypeName}.prototype`
118+
];
119+
}
120+
121+
function main(
122+
node: ESTree.Node,
123+
options: ProbeMainContext
124+
) {
125+
const { sourceFile, data: prototypeName } = options;
126+
127+
sourceFile.warnings.push(
128+
generateWarning("monkey-patch", { value: prototypeName, location: node.loc })
129+
);
130+
}
131+
132+
export default {
133+
name: "isMonkeyPatch",
134+
validateNode: [
135+
validateNodeAssignment,
136+
validateDefineProperty
137+
],
138+
main
139+
};

workspaces/js-x-ray/src/warnings.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ import {
1010
} from "./utils/toArrayLocation.ts";
1111

1212
export type OptionalWarningName =
13-
| "synchronous-io" | "log-usage";
13+
| "synchronous-io"
14+
| "log-usage";
1415

1516
export type WarningName =
1617
| "parsing-error"
@@ -28,6 +29,7 @@ export type WarningName =
2829
| "serialize-environment"
2930
| "data-exfiltration"
3031
| "sql-injection"
32+
| "monkey-patch"
3133
| OptionalWarningName;
3234

3335
export interface Warning<T = WarningName> {
@@ -126,6 +128,11 @@ export const warnings = Object.freeze({
126128
i18n: "sast_warnings.sql_injection",
127129
severity: "Warning",
128130
experimental: false
131+
},
132+
"monkey-patch": {
133+
i18n: "sast_warnings.monkey_patch",
134+
severity: "Warning",
135+
experimental: false
129136
}
130137
}) satisfies Record<WarningName, Pick<Warning, "experimental" | "i18n" | "severity">>;
131138

workspaces/js-x-ray/test/probes/isLiteral.spec.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -296,9 +296,9 @@ describe("email collection", () => {
296296

297297
const emails = Array.from(emailSet);
298298
assert.strictEqual(emails.length, 3);
299-
assert.ok(emails.some(e => e.value === "user@example.com"));
300-
assert.ok(emails.some(e => e.value === "name.surname@domain.co.uk"));
301-
assert.ok(emails.some(e => e.value === "test123@test-domain.org"));
299+
assert.ok(emails.some((e) => e.value === "user@example.com"));
300+
assert.ok(emails.some((e) => e.value === "name.surname@domain.co.uk"));
301+
assert.ok(emails.some((e) => e.value === "test123@test-domain.org"));
302302
});
303303

304304
test("should not collect invalid email formats", () => {
@@ -311,7 +311,7 @@ describe("email collection", () => {
311311
`;
312312
const ast = parseScript(str);
313313
const emailSet = new CollectableSet("email");
314-
const sastAnalysis = getSastAnalysis(isLiteral, { collectables: [emailSet] })
314+
getSastAnalysis(isLiteral, { collectables: [emailSet] })
315315
.execute(ast.body);
316316

317317
const emails = Array.from(emailSet);
@@ -327,21 +327,21 @@ describe("email collection", () => {
327327
`;
328328
const ast = parseScript(str);
329329
const emailSet = new CollectableSet("email");
330-
const sastAnalysis = getSastAnalysis(isLiteral, { collectables: [emailSet] })
330+
getSastAnalysis(isLiteral, { collectables: [emailSet] })
331331
.execute(ast.body);
332332

333333
const emails = Array.from(emailSet);
334334
assert.strictEqual(emails.length, 3);
335-
assert.ok(emails.some(e => e.value === "admin@site.com"));
336-
assert.ok(emails.some(e => e.value === "support@help.io"));
337-
assert.ok(emails.some(e => e.value === "info@company.net"));
335+
assert.ok(emails.some((e) => e.value === "admin@site.com"));
336+
assert.ok(emails.some((e) => e.value === "support@help.io"));
337+
assert.ok(emails.some((e) => e.value === "info@company.net"));
338338
});
339339

340340
test("should track email locations correctly", () => {
341341
const str = `const email = "test@example.com";`;
342342
const ast = parseScript(str);
343343
const emailSet = new CollectableSet("email");
344-
const sastAnalysis = getSastAnalysis(isLiteral, { collectables: [emailSet], location: "test.js" })
344+
getSastAnalysis(isLiteral, { collectables: [emailSet], location: "test.js" })
345345
.execute(ast.body);
346346

347347
const emails = Array.from(emailSet);
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
2+
// Import Node.js Dependencies
3+
import { describe, test } from "node:test";
4+
5+
// Import Internal Dependencies
6+
import isMonkeyPatch, { JS_TYPES } from "../../src/probes/isMonkeyPatch.ts";
7+
import { getSastAnalysis, parseScript } from "../utils/index.ts";
8+
9+
describe("isMonkeyPatch", () => {
10+
test("should detect monkey patching via direct prototype assignment", (t) => {
11+
t.plan(JS_TYPES.size * 2);
12+
13+
for (const jsType of JS_TYPES) {
14+
const computed = Math.random() > 0.5 ? `["prototype"]` : ".prototype";
15+
const str = `${jsType}${computed}.any = function() {};`;
16+
17+
const ast = parseScript(str);
18+
const sastAnalysis = getSastAnalysis(isMonkeyPatch);
19+
sastAnalysis.execute(ast);
20+
21+
const outputWarnings = sastAnalysis.warnings();
22+
23+
t.assert.equal(outputWarnings.length, 1);
24+
t.assert.partialDeepStrictEqual(outputWarnings[0], {
25+
kind: "monkey-patch",
26+
value: `${jsType}.prototype`
27+
});
28+
}
29+
});
30+
31+
test("should detect monkey patching via Object.defineProperty", (t) => {
32+
const str = `Object.defineProperty(Array.prototype, "map", {
33+
configurable: true,
34+
enumerable: false,
35+
writable: true,
36+
value: function mapper(fn, thisArg) {}
37+
});`;
38+
39+
const ast = parseScript(str);
40+
const sastAnalysis = getSastAnalysis(isMonkeyPatch);
41+
sastAnalysis.execute(ast);
42+
43+
const outputWarnings = sastAnalysis.warnings();
44+
45+
t.assert.equal(outputWarnings.length, 1);
46+
t.assert.partialDeepStrictEqual(outputWarnings[0], {
47+
kind: "monkey-patch",
48+
value: "Array.prototype"
49+
});
50+
});
51+
52+
test("should detect monkey patching via Reflect.defineProperty", (t) => {
53+
const str = `Reflect.defineProperty(Array.prototype, "map", {
54+
configurable: true,
55+
enumerable: false,
56+
writable: true,
57+
value: function mapper(fn, thisArg) {}
58+
});`;
59+
60+
const ast = parseScript(str);
61+
const sastAnalysis = getSastAnalysis(isMonkeyPatch);
62+
sastAnalysis.execute(ast);
63+
64+
const outputWarnings = sastAnalysis.warnings();
65+
66+
t.assert.equal(outputWarnings.length, 1);
67+
t.assert.partialDeepStrictEqual(outputWarnings[0], {
68+
kind: "monkey-patch",
69+
value: "Array.prototype"
70+
});
71+
});
72+
});

workspaces/js-x-ray/test/probes/log-usage.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { AstAnalyser } from "../../src/index.ts";
1010
// CONSTANTS
1111
const kFixtureURL = new URL("fixtures/logUsage/", import.meta.url);
1212

13-
test("it should detect log methods", async () => {
13+
test("it should detect log methods", async() => {
1414
const fixtureFiles = await fs.readdir(kFixtureURL);
1515
for (const fixtureFile of fixtureFiles) {
1616
const fixture = readFileSync(new URL(fixtureFile, kFixtureURL), "utf-8");

0 commit comments

Comments
 (0)