Skip to content

Commit

Permalink
refactor: consider Function("return this") as safe
Browse files Browse the repository at this point in the history
  • Loading branch information
fraxken committed Jan 17, 2024
1 parent 0b807a0 commit 4976606
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 1 deletion.
7 changes: 7 additions & 0 deletions src/probes/isUnsafeCallee.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@ function validateNode(node) {
function main(node, options) {
const { analysis, data: calleeName } = options;

if (
calleeName === "Function" &&
node.callee.arguments.length > 0 &&
node.callee.arguments[0].value === "return this"
) {
return ProbeSignals.Skip;
}
analysis.addWarning("unsafe-stmt", calleeName, node.loc);

return ProbeSignals.Skip;
Expand Down
12 changes: 11 additions & 1 deletion test/probes/isUnsafeCallee.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,19 @@ test("should detect eval", () => {
assert.equal(result.value, "eval");
});

test("should detect Function", () => {
test("should not detect warnings for Function with return this", () => {
const str = "Function(\"return this\")()";

const ast = parseScript(str);
const sastAnalysis = getSastAnalysis(str, isUnsafeCallee)
.execute(ast.body);

assert.strictEqual(sastAnalysis.warnings.length, 0);
});

test("should detect for unsafe Function statement", () => {
const str = "Function(\"anything in here\")()";

const ast = parseScript(str);
const sastAnalysis = getSastAnalysis(str, isUnsafeCallee)
.execute(ast.body);
Expand Down
21 changes: 21 additions & 0 deletions test/runASTAnalysis.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,27 @@ test("it should be capable to follow a malicious code with hexa computation and
assert.deepEqual([...dependencies.keys()], ["./test/data"]);
});

test.skip("it should be capable to follow a malicious code with return Function this", () => {
const { warnings, dependencies } = runASTAnalysis(`
function unhex(r) {
return Buffer.from(r, "hex").toString();
}
const g = freeGlobal || freeSelf || Function('return this')();
const p = g["pro" + "cess"];
const evil = p["mainMod" + "ule"][unhex("72657175697265")];
const work = evil(unhex("2e2f746573742f64617461"));
`);

assert.deepEqual(getWarningKind(warnings), [
"encoded-literal",
"unsafe-import",
"unsafe-stmt"
].sort());
assert.deepEqual([...dependencies.keys()], ["./test/data"]);
});

test("it should throw a 'short-identifiers' warning for a code with only one-character identifiers", () => {
const { warnings } = runASTAnalysis(`
var a = 0, b, c, d;
Expand Down

0 comments on commit 4976606

Please sign in to comment.