better support for the &&=, ||=, and ??= operators

erik-krogh · erik-krogh · commit d95d427c5b60 · 2020-08-13T09:22:32.000+02:00
diff --git a/javascript/extractor/src/com/semmle/js/extractor/CFGExtractor.java b/javascript/extractor/src/com/semmle/js/extractor/CFGExtractor.java
@@ -1551,8 +1551,27 @@ public Void visit(MemberDefinition<?> nd, SuccessorInfo i) {
 
     @Override
     public Void visit(AssignmentExpression nd, SuccessorInfo i) {
-      visitAssign(nd, nd.getLeft(), nd.getRight());
-      succ(nd, i.getGuardedSuccessors(nd));
+      // `a &&= b` expands to `a || (a = b);`
+      // The CFG is a conditional assignment, so we go through the assignment `nd` last.
+      if ("&&=".equals(nd.getOperator()) || "||=".equals(nd.getOperator()) || "??=".equals(nd.getOperator())) {
+        if ("&&=".equals(nd.getOperator())) {
+          // from lhs to rhs on truthy. from lhs to false-branch on falsy.
+          visit(nd.getLeft(), First.of(nd.getRight()), i.getSuccessors(false));
+        } else if ("||=".equals(nd.getOperator())) {
+          // from lhs to true-branch on truthy. from lhs to rhs on falsy.
+          visit(nd.getLeft(), i.getSuccessors(true), First.of(nd.getRight()));
+        } else { // "??="
+          // the union of the above - truthyness is unknown.
+          visit(nd.getLeft(), union(First.of(nd.getRight()), i.getAllSuccessors()), null);
+        }
+        
+        visit(nd.getRight(), First.of(nd), null); // from right to assignment.
+
+        succ(nd, i.getGuardedSuccessors(nd));
+      } else {
+        visitAssign(nd, nd.getLeft(), nd.getRight());
+        succ(nd, i.getGuardedSuccessors(nd));
+      }
       return null;
     }
 
diff --git a/javascript/extractor/src/com/semmle/js/parser/TypeScriptASTConverter.java b/javascript/extractor/src/com/semmle/js/parser/TypeScriptASTConverter.java
@@ -852,6 +852,9 @@ private Node convertBinaryExpression(JsonObject node, SourceLocation loc) throws
       case ">>=":
       case "<<=":
       case ">>>=":
+      case "??=":
+      case "&&=":
+      case "||=":
         return new AssignmentExpression(loc, operator, convertLValue(left), right);
 
       default:
diff --git a/javascript/ql/test/library-tests/Expr/assignment2.ts b/javascript/ql/test/library-tests/Expr/assignment2.ts
@@ -0,0 +1,9 @@
+let a = 2;
+let b = 3;
+
+a = 23;
+a += 19;
+
+a &&= 4;
+a ||= 5;
+a ??= 6;
diff --git a/javascript/ql/test/library-tests/Expr/tests.expected b/javascript/ql/test/library-tests/Expr/tests.expected
@@ -1,4 +1,25 @@
 test_getParent
+| assignment2.ts:1:5:1:5 | a | assignment2.ts:1:5:1:9 | a = 2 |
+| assignment2.ts:1:5:1:9 | a = 2 | assignment2.ts:1:1:1:10 | let a = 2; |
+| assignment2.ts:1:9:1:9 | 2 | assignment2.ts:1:5:1:9 | a = 2 |
+| assignment2.ts:2:5:2:5 | b | assignment2.ts:2:5:2:9 | b = 3 |
+| assignment2.ts:2:5:2:9 | b = 3 | assignment2.ts:2:1:2:10 | let b = 3; |
+| assignment2.ts:2:9:2:9 | 3 | assignment2.ts:2:5:2:9 | b = 3 |
+| assignment2.ts:4:1:4:1 | a | assignment2.ts:4:1:4:6 | a = 23 |
+| assignment2.ts:4:1:4:6 | a = 23 | assignment2.ts:4:1:4:7 | a = 23; |
+| assignment2.ts:4:5:4:6 | 23 | assignment2.ts:4:1:4:6 | a = 23 |
+| assignment2.ts:5:1:5:1 | a | assignment2.ts:5:1:5:7 | a += 19 |
+| assignment2.ts:5:1:5:7 | a += 19 | assignment2.ts:5:1:5:8 | a += 19; |
+| assignment2.ts:5:6:5:7 | 19 | assignment2.ts:5:1:5:7 | a += 19 |
+| assignment2.ts:7:1:7:1 | a | assignment2.ts:7:1:7:7 | a &&= 4 |
+| assignment2.ts:7:1:7:7 | a &&= 4 | assignment2.ts:7:1:7:8 | a &&= 4; |
+| assignment2.ts:7:7:7:7 | 4 | assignment2.ts:7:1:7:7 | a &&= 4 |
+| assignment2.ts:8:1:8:1 | a | assignment2.ts:8:1:8:7 | a \|\|= 5 |
+| assignment2.ts:8:1:8:7 | a \|\|= 5 | assignment2.ts:8:1:8:8 | a \|\|= 5; |
+| assignment2.ts:8:7:8:7 | 5 | assignment2.ts:8:1:8:7 | a \|\|= 5 |
+| assignment2.ts:9:1:9:1 | a | assignment2.ts:9:1:9:7 | a ??= 6 |
+| assignment2.ts:9:1:9:7 | a ??= 6 | assignment2.ts:9:1:9:8 | a ??= 6; |
+| assignment2.ts:9:7:9:7 | 6 | assignment2.ts:9:1:9:7 | a ??= 6 |
 | assignment.js:1:1:1:1 | a | assignment.js:1:1:1:6 | a = 23 |
 | assignment.js:1:1:1:6 | a = 23 | assignment.js:1:1:1:7 | a = 23; |
 | assignment.js:1:5:1:6 | 23 | assignment.js:1:1:1:6 | a = 23 |
@@ -468,6 +489,27 @@ test_getParent
 | update.js:4:1:4:1 | b | update.js:4:1:4:3 | b-- |
 | update.js:4:1:4:3 | b-- | update.js:4:1:4:4 | b--; |
 test_getTopLevel
+| assignment2.ts:1:5:1:5 | a | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:1:5:1:9 | a = 2 | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:1:9:1:9 | 2 | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:2:5:2:5 | b | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:2:5:2:9 | b = 3 | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:2:9:2:9 | 3 | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:4:1:4:1 | a | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:4:1:4:6 | a = 23 | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:4:5:4:6 | 23 | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:5:1:5:1 | a | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:5:1:5:7 | a += 19 | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:5:6:5:7 | 19 | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:7:1:7:1 | a | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:7:1:7:7 | a &&= 4 | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:7:7:7:7 | 4 | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:8:1:8:1 | a | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:8:1:8:7 | a \|\|= 5 | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:8:7:8:7 | 5 | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:9:1:9:1 | a | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:9:1:9:7 | a ??= 6 | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:9:7:9:7 | 6 | assignment2.ts:1:1:9:8 | <toplevel> |
 | assignment.js:1:1:1:1 | a | assignment.js:1:1:12:7 | <toplevel> |
 | assignment.js:1:1:1:6 | a = 23 | assignment.js:1:1:12:7 | <toplevel> |
 | assignment.js:1:5:1:6 | 23 | assignment.js:1:1:12:7 | <toplevel> |
@@ -937,6 +979,20 @@ test_getTopLevel
 | update.js:4:1:4:1 | b | update.js:1:1:5:0 | <toplevel> |
 | update.js:4:1:4:3 | b-- | update.js:1:1:5:0 | <toplevel> |
 test_getChild
+| assignment2.ts:1:5:1:9 | a = 2 | 0 | assignment2.ts:1:5:1:5 | a |
+| assignment2.ts:1:5:1:9 | a = 2 | 1 | assignment2.ts:1:9:1:9 | 2 |
+| assignment2.ts:2:5:2:9 | b = 3 | 0 | assignment2.ts:2:5:2:5 | b |
+| assignment2.ts:2:5:2:9 | b = 3 | 1 | assignment2.ts:2:9:2:9 | 3 |
+| assignment2.ts:4:1:4:6 | a = 23 | 0 | assignment2.ts:4:1:4:1 | a |
+| assignment2.ts:4:1:4:6 | a = 23 | 1 | assignment2.ts:4:5:4:6 | 23 |
+| assignment2.ts:5:1:5:7 | a += 19 | 0 | assignment2.ts:5:1:5:1 | a |
+| assignment2.ts:5:1:5:7 | a += 19 | 1 | assignment2.ts:5:6:5:7 | 19 |
+| assignment2.ts:7:1:7:7 | a &&= 4 | 0 | assignment2.ts:7:1:7:1 | a |
+| assignment2.ts:7:1:7:7 | a &&= 4 | 1 | assignment2.ts:7:7:7:7 | 4 |
+| assignment2.ts:8:1:8:7 | a \|\|= 5 | 0 | assignment2.ts:8:1:8:1 | a |
+| assignment2.ts:8:1:8:7 | a \|\|= 5 | 1 | assignment2.ts:8:7:8:7 | 5 |
+| assignment2.ts:9:1:9:7 | a ??= 6 | 0 | assignment2.ts:9:1:9:1 | a |
+| assignment2.ts:9:1:9:7 | a ??= 6 | 1 | assignment2.ts:9:7:9:7 | 6 |
 | assignment.js:1:1:1:6 | a = 23 | 0 | assignment.js:1:1:1:1 | a |
 | assignment.js:1:1:1:6 | a = 23 | 1 | assignment.js:1:5:1:6 | 23 |
 | assignment.js:2:1:2:7 | a += 19 | 0 | assignment.js:2:1:2:1 | a |
@@ -1213,6 +1269,20 @@ test_getChild
 | update.js:3:1:3:3 | --b | 0 | update.js:3:3:3:3 | b |
 | update.js:4:1:4:3 | b-- | 0 | update.js:4:1:4:1 | b |
 test_isPure
+| assignment2.ts:1:5:1:5 | a |
+| assignment2.ts:1:9:1:9 | 2 |
+| assignment2.ts:2:5:2:5 | b |
+| assignment2.ts:2:9:2:9 | 3 |
+| assignment2.ts:4:1:4:1 | a |
+| assignment2.ts:4:5:4:6 | 23 |
+| assignment2.ts:5:1:5:1 | a |
+| assignment2.ts:5:6:5:7 | 19 |
+| assignment2.ts:7:1:7:1 | a |
+| assignment2.ts:7:7:7:7 | 4 |
+| assignment2.ts:8:1:8:1 | a |
+| assignment2.ts:8:7:8:7 | 5 |
+| assignment2.ts:9:1:9:1 | a |
+| assignment2.ts:9:7:9:7 | 6 |
 | assignment.js:1:1:1:1 | a |
 | assignment.js:1:5:1:6 | 23 |
 | assignment.js:2:1:2:1 | a |
@@ -1708,6 +1778,27 @@ test_stripParens
 | primaries.js:28:2:28:5 | (42) | primaries.js:28:3:28:4 | 42 |
 | unary.js:6:5:6:7 | (0) | unary.js:6:6:6:6 | 0 |
 test_getContainer
+| assignment2.ts:1:5:1:5 | a | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:1:5:1:9 | a = 2 | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:1:9:1:9 | 2 | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:2:5:2:5 | b | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:2:5:2:9 | b = 3 | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:2:9:2:9 | 3 | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:4:1:4:1 | a | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:4:1:4:6 | a = 23 | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:4:5:4:6 | 23 | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:5:1:5:1 | a | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:5:1:5:7 | a += 19 | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:5:6:5:7 | 19 | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:7:1:7:1 | a | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:7:1:7:7 | a &&= 4 | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:7:7:7:7 | 4 | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:8:1:8:1 | a | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:8:1:8:7 | a \|\|= 5 | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:8:7:8:7 | 5 | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:9:1:9:1 | a | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:9:1:9:7 | a ??= 6 | assignment2.ts:1:1:9:8 | <toplevel> |
+| assignment2.ts:9:7:9:7 | 6 | assignment2.ts:1:1:9:8 | <toplevel> |
 | assignment.js:1:1:1:1 | a | assignment.js:1:1:12:7 | <toplevel> |
 | assignment.js:1:1:1:6 | a = 23 | assignment.js:1:1:12:7 | <toplevel> |
 | assignment.js:1:5:1:6 | 23 | assignment.js:1:1:12:7 | <toplevel> |
@@ -2177,6 +2268,27 @@ test_getContainer
 | update.js:4:1:4:1 | b | update.js:1:1:5:0 | <toplevel> |
 | update.js:4:1:4:3 | b-- | update.js:1:1:5:0 | <toplevel> |
 test_getEnclosingStmt
+| assignment2.ts:1:5:1:5 | a | assignment2.ts:1:1:1:10 | let a = 2; |
+| assignment2.ts:1:5:1:9 | a = 2 | assignment2.ts:1:1:1:10 | let a = 2; |
+| assignment2.ts:1:9:1:9 | 2 | assignment2.ts:1:1:1:10 | let a = 2; |
+| assignment2.ts:2:5:2:5 | b | assignment2.ts:2:1:2:10 | let b = 3; |
+| assignment2.ts:2:5:2:9 | b = 3 | assignment2.ts:2:1:2:10 | let b = 3; |
+| assignment2.ts:2:9:2:9 | 3 | assignment2.ts:2:1:2:10 | let b = 3; |
+| assignment2.ts:4:1:4:1 | a | assignment2.ts:4:1:4:7 | a = 23; |
+| assignment2.ts:4:1:4:6 | a = 23 | assignment2.ts:4:1:4:7 | a = 23; |
+| assignment2.ts:4:5:4:6 | 23 | assignment2.ts:4:1:4:7 | a = 23; |
+| assignment2.ts:5:1:5:1 | a | assignment2.ts:5:1:5:8 | a += 19; |
+| assignment2.ts:5:1:5:7 | a += 19 | assignment2.ts:5:1:5:8 | a += 19; |
+| assignment2.ts:5:6:5:7 | 19 | assignment2.ts:5:1:5:8 | a += 19; |
+| assignment2.ts:7:1:7:1 | a | assignment2.ts:7:1:7:8 | a &&= 4; |
+| assignment2.ts:7:1:7:7 | a &&= 4 | assignment2.ts:7:1:7:8 | a &&= 4; |
+| assignment2.ts:7:7:7:7 | 4 | assignment2.ts:7:1:7:8 | a &&= 4; |
+| assignment2.ts:8:1:8:1 | a | assignment2.ts:8:1:8:8 | a \|\|= 5; |
+| assignment2.ts:8:1:8:7 | a \|\|= 5 | assignment2.ts:8:1:8:8 | a \|\|= 5; |
+| assignment2.ts:8:7:8:7 | 5 | assignment2.ts:8:1:8:8 | a \|\|= 5; |
+| assignment2.ts:9:1:9:1 | a | assignment2.ts:9:1:9:8 | a ??= 6; |
+| assignment2.ts:9:1:9:7 | a ??= 6 | assignment2.ts:9:1:9:8 | a ??= 6; |
+| assignment2.ts:9:7:9:7 | 6 | assignment2.ts:9:1:9:8 | a ??= 6; |
 | assignment.js:1:1:1:1 | a | assignment.js:1:1:1:7 | a = 23; |
 | assignment.js:1:1:1:6 | a = 23 | assignment.js:1:1:1:7 | a = 23; |
 | assignment.js:1:5:1:6 | 23 | assignment.js:1:1:1:7 | a = 23; |
@@ -2612,6 +2724,14 @@ test_getEnclosingStmt
 | update.js:4:1:4:1 | b | update.js:4:1:4:4 | b--; |
 | update.js:4:1:4:3 | b-- | update.js:4:1:4:4 | b--; |
 test_inNullSensitiveContext
+| assignment2.ts:5:1:5:1 | a |
+| assignment2.ts:5:6:5:7 | 19 |
+| assignment2.ts:7:1:7:1 | a |
+| assignment2.ts:7:7:7:7 | 4 |
+| assignment2.ts:8:1:8:1 | a |
+| assignment2.ts:8:7:8:7 | 5 |
+| assignment2.ts:9:1:9:1 | a |
+| assignment2.ts:9:7:9:7 | 6 |
 | assignment.js:2:1:2:1 | a |
 | assignment.js:2:6:2:7 | 19 |
 | assignment.js:3:1:3:1 | a |
diff --git a/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected b/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected
diff --git a/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/typescript.ts b/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/typescript.ts
