From 330b342f79e950c2c24f556c0152de8b7cc79f9a Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 11 Nov 2024 15:55:59 +0100 Subject: [PATCH 1/3] JS: Add test showing misclassification of file with top-level sequence expr --- javascript/ql/test/library-tests/ModuleTypes/tests.expected | 1 + .../ql/test/library-tests/ModuleTypes/toplevel-comma.js | 4 ++++ 2 files changed, 5 insertions(+) create mode 100644 javascript/ql/test/library-tests/ModuleTypes/toplevel-comma.js diff --git a/javascript/ql/test/library-tests/ModuleTypes/tests.expected b/javascript/ql/test/library-tests/ModuleTypes/tests.expected index a76b0d2b89cb..15332636c8fd 100644 --- a/javascript/ql/test/library-tests/ModuleTypes/tests.expected +++ b/javascript/ql/test/library-tests/ModuleTypes/tests.expected @@ -8,3 +8,4 @@ | modulePackage/tst.js:1:1:1:75 | | es2015 | | require.js:1:1:7:1 | | node | | script.js:1:1:1:35 | | non-module | +| toplevel-comma.js:1:1:5:0 | | non-module | diff --git a/javascript/ql/test/library-tests/ModuleTypes/toplevel-comma.js b/javascript/ql/test/library-tests/ModuleTypes/toplevel-comma.js new file mode 100644 index 000000000000..4a0a5eee01b5 --- /dev/null +++ b/javascript/ql/test/library-tests/ModuleTypes/toplevel-comma.js @@ -0,0 +1,4 @@ +(() => {}), +(() => { + const fs = require('fs'); +})(); From 79239261a51ce3128f8d2781178a13c3f6f7c9bf Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 11 Nov 2024 16:33:54 +0100 Subject: [PATCH 2/3] JS: Refactor AbstractDetector so only one method traverses exprs --- .../semmle/js/extractor/AbstractDetector.java | 38 ++++++------------- 1 file changed, 12 insertions(+), 26 deletions(-) diff --git a/javascript/extractor/src/com/semmle/js/extractor/AbstractDetector.java b/javascript/extractor/src/com/semmle/js/extractor/AbstractDetector.java index b8a5f1df0fcf..2812772b7d41 100644 --- a/javascript/extractor/src/com/semmle/js/extractor/AbstractDetector.java +++ b/javascript/extractor/src/com/semmle/js/extractor/AbstractDetector.java @@ -38,27 +38,10 @@ protected boolean visitStatements(List stmts) { protected boolean visitStatement(Statement stmt) { if (stmt instanceof ExpressionStatement) { - Expression e = stripParens(((ExpressionStatement) stmt).getExpression()); - - // check whether `e` is an iife; if so, recursively check its body - - // strip off unary operators to handle `!function(){...}()` - if (e instanceof UnaryExpression) e = ((UnaryExpression) e).getArgument(); - - if (e instanceof CallExpression && ((CallExpression) e).getArguments().isEmpty()) { - Expression callee = stripParens(((CallExpression) e).getCallee()); - if (callee instanceof IFunction) { - Node body = ((IFunction) callee).getBody(); - if (body instanceof BlockStatement) - return visitStatements(((BlockStatement) body).getBody()); - } - } - - if (visitExpression(e)) return true; - + return visitExpression(((ExpressionStatement) stmt).getExpression()); } else if (stmt instanceof VariableDeclaration) { for (VariableDeclarator decl : ((VariableDeclaration) stmt).getDeclarations()) { - Expression init = stripParens(decl.getInit()); + Expression init = decl.getInit(); if (visitExpression(init)) return true; } @@ -77,17 +60,13 @@ protected boolean visitStatement(Statement stmt) { return false; } - private static Expression stripParens(Expression e) { - if (e instanceof ParenthesizedExpression) - return stripParens(((ParenthesizedExpression) e).getExpression()); - return e; - } - /** * Recursively check {@code e} if it's a call or an assignment. */ protected boolean visitExpression(Expression e) { - if (e instanceof CallExpression) { + if (e instanceof ParenthesizedExpression) { + return visitExpression(((ParenthesizedExpression) e).getExpression()); + } else if (e instanceof CallExpression) { CallExpression call = (CallExpression) e; Expression callee = call.getCallee(); // recurse, to handle things like `foo()()` @@ -102,6 +81,13 @@ protected boolean visitExpression(Expression e) { if (!"=".equals(assgn.getOperator())) return false; return visitExpression(assgn.getRight()); + } else if (e instanceof UnaryExpression) { + return visitExpression(((UnaryExpression) e).getArgument()); + } else if (e instanceof IFunction) { + Node body = ((IFunction) e).getBody(); + if (body instanceof BlockStatement) { + return visitStatement((BlockStatement) body); + } } return false; } From ff3a81847c7aeda0db4c6b73cd001cb6735bf168 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 11 Nov 2024 16:35:41 +0100 Subject: [PATCH 3/3] JS: Handle sequence expressions in visitExpression --- .../src/com/semmle/js/extractor/AbstractDetector.java | 6 ++++++ javascript/ql/test/library-tests/ModuleTypes/tests.expected | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/javascript/extractor/src/com/semmle/js/extractor/AbstractDetector.java b/javascript/extractor/src/com/semmle/js/extractor/AbstractDetector.java index 2812772b7d41..8b1ed9cc5a68 100644 --- a/javascript/extractor/src/com/semmle/js/extractor/AbstractDetector.java +++ b/javascript/extractor/src/com/semmle/js/extractor/AbstractDetector.java @@ -12,6 +12,7 @@ import com.semmle.js.ast.Node; import com.semmle.js.ast.ParenthesizedExpression; import com.semmle.js.ast.Program; +import com.semmle.js.ast.SequenceExpression; import com.semmle.js.ast.Statement; import com.semmle.js.ast.TryStatement; import com.semmle.js.ast.UnaryExpression; @@ -88,6 +89,11 @@ protected boolean visitExpression(Expression e) { if (body instanceof BlockStatement) { return visitStatement((BlockStatement) body); } + } else if (e instanceof SequenceExpression) { + SequenceExpression seq = (SequenceExpression) e; + for (Expression child : seq.getExpressions()) { + if (visitExpression(child)) return true; + } } return false; } diff --git a/javascript/ql/test/library-tests/ModuleTypes/tests.expected b/javascript/ql/test/library-tests/ModuleTypes/tests.expected index 15332636c8fd..103a65951db0 100644 --- a/javascript/ql/test/library-tests/ModuleTypes/tests.expected +++ b/javascript/ql/test/library-tests/ModuleTypes/tests.expected @@ -8,4 +8,4 @@ | modulePackage/tst.js:1:1:1:75 | | es2015 | | require.js:1:1:7:1 | | node | | script.js:1:1:1:35 | | non-module | -| toplevel-comma.js:1:1:5:0 | | non-module | +| toplevel-comma.js:1:1:5:0 | | node |