Merge pull request #1298 from asger-semmle/full-mode-fixes-rc120

TS: Backport full-mode fixes to rc/1.20

change-notes/1.20/support/versions-compilers.csv

@@ -4,9 +4,9 @@ C/C++,"C89, C99, C11, C++98, C++03, C++11, C++14, C++17","Clang extensions (up t
 GNU extensions (up to GCC 7.3), 
 
 Microsoft extensions (up to VS 2017)","``.cpp``, ``.c++``, ``.cxx``, ``.hpp``, ``.hh``, ``.h++``, ``.hxx``, ``.c``, ``.cc``, ``.h``"
-C#,C# up to 7.2 together with .NET versions up to 4.7.1,"Microsoft Visual Studio up to 2017, 
+C#,C# up to 7.3 together with .NET versions up to 4.7.1,"Microsoft Visual Studio up to 2017, 
 
-.NET Core up to 2.1","``.sln``, ``.csproj``, ``.cs``, ``.cshtml``, ``.xaml``"
+.NET Core up to 2.2","``.sln``, ``.csproj``, ``.cs``, ``.cshtml``, ``.xaml``"
 COBOL,ANSI 85 or newer [1]_.,Not applicable,"``.cbl``, ``.CBL``, ``.cpy``, ``.CPY``, ``.copy``, ``.COPY``"
 Java,"Java 6 to 11 [2]_.","javac (OpenJDK and Oracle JDK)
 

cpp/ql/src/Architecture/General Class-Level Information/HubClasses.ql

@@ -2,7 +2,7 @@
  * @name Hub classes
  * @description Shows coupling between classes. Large, red, boxes are hub types that depend on many other classes
  *              and are depended on by many other classes.
- * @kind treemap
+ * @kind table
  * @id cpp/architecture/hub-classes
  * @treemap.warnOn highValues
  * @tags maintainability

cpp/ql/src/Security/CWE/CWE-732/UnsafeDaclSecurityDescriptor.ql

@@ -14,6 +14,7 @@
  */
 import cpp
 import semmle.code.cpp.dataflow.DataFlow
+import semmle.code.cpp.dataflow.DataFlow2
 
 /**
  * A function call to SetSecurityDescriptorDacl to set the ACL, specified by (2nd argument) bDaclPresent = TRUE
@@ -28,9 +29,9 @@ class SetSecurityDescriptorDaclFunctionCall extends FunctionCall {
 /**
  * Dataflow that detects a call to SetSecurityDescriptorDacl with a NULL DACL as the pDacl argument
  */
-class SetSecurityDescriptorDaclFunctionConfiguration extends DataFlow::Configuration {
-  SetSecurityDescriptorDaclFunctionConfiguration() {
-    this = "SetSecurityDescriptorDaclFunctionConfiguration"
+class NullDaclConfig extends DataFlow::Configuration {
+  NullDaclConfig() {
+    this = "NullDaclConfig"
   }
 
   override predicate isSource(DataFlow::Node source) {
@@ -49,6 +50,43 @@ class SetSecurityDescriptorDaclFunctionConfiguration extends DataFlow::Configura
   }
 }
 
+/**
+ * Dataflow that detects a call to SetSecurityDescriptorDacl with a pDacl
+ * argument that's _not_ likely to be NULL.
+ */
+class NonNullDaclConfig extends DataFlow2::Configuration {
+  NonNullDaclConfig() {
+    this = "NonNullDaclConfig"
+  }
+
+  override predicate isSource(DataFlow::Node source) {
+    source.getType().getUnspecifiedType().(PointerType).getBaseType() =
+      any(Type t | t.getName() = "ACL").getUnspecifiedType() and
+    (
+      // If the value comes from a function whose body we can't see, assume
+      // it's not null.
+      exists(Call call |
+        not exists(call.getTarget().getBlock()) and
+        source.asExpr() = call
+      )
+      or
+      // If the value is assigned by reference, assume it's not null. The data
+      // flow library cannot currently follow flow from the body of a function to
+      // an assignment by reference, so this rule applies whether we see the
+      // body or not.
+      exists(Call call |
+        call.getAnArgument() = source.asDefiningArgument()
+      )
+    )
+  }
+
+  override predicate isSink(DataFlow::Node sink) {
+    exists(SetSecurityDescriptorDaclFunctionCall call |
+      sink.asExpr() = call.getArgument(2)
+    )
+  }
+}
+
 from SetSecurityDescriptorDaclFunctionCall call, string message
 where exists
   ( 
@@ -59,9 +97,10 @@ where exists
   ) or exists
   ( 
     Expr constassign, VariableAccess var, 
-    SetSecurityDescriptorDaclFunctionConfiguration config |
+    NullDaclConfig nullDaclConfig, NonNullDaclConfig nonNullDaclConfig |
     message = "Setting a DACL to NULL in a SECURITY_DESCRIPTOR using variable " + var + " that is set to NULL will result in an unprotected object." |
     var = call.getArgument(2)
-    and config.hasFlow(DataFlow::exprNode(constassign), DataFlow::exprNode(var))
+    and nullDaclConfig.hasFlow(DataFlow::exprNode(constassign), DataFlow::exprNode(var))
+    and not nonNullDaclConfig.hasFlow(_, DataFlow::exprNode(var))
   )
 select call, message

cpp/ql/src/jsf/3.02 Code Size and Complexity/AV Rule 3.ql

@@ -12,5 +12,4 @@ import cpp
 from Function f, int c
 where c = f.getMetrics().getCyclomaticComplexity() and
       c > 20
-select f, c as CyclomaticComplexity,
-       "AV Rule 3: All functions shall have a cyclomatic complexity number of 20 or less."
+select f, "AV Rule 3: All functions shall have a cyclomatic complexity number of 20 or less."

cpp/ql/src/jsf/4.10 Classes/AV Rule 81.ql

@@ -3,6 +3,7 @@
  * @description The assignment operator shall handle self-assignment correctly.
  * @kind problem
  * @id cpp/jsf/av-rule-81
+ * @precision low
  * @problem.severity error
  * @tags correctness
  *       external/jsf
@@ -77,4 +78,4 @@ where hasResource(op.getDeclaringType())
       and not exists(op.getASelfEqualityTest())
       and not exists(op.getASwapCall())
       and exists(op.getADeleteExpr())
-select op
+select op, "AV Rule 81: The assignment operator shall handle self-assignment correctly."

cpp/ql/src/semmle/code/cpp/ASTSanity.ql

@@ -1,7 +1,7 @@
 /**
  * @name AST Sanity Check
  * @description Performs sanity checks on the Abstract Syntax Tree. This query should have no results. 
- * @kind problem
+ * @kind table
  * @id cpp/ast-sanity-check
  */
 

cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll

@@ -93,9 +93,17 @@ module FlowVar_internal {
    * - Supporting fields, globals and statics like the Java SSA library does.
    * - Supporting all local variables, even if their address is taken by
    *   address-of, reference assignments, or lambdas.
+   * - Understanding that assignment to a field of a local struct is a
+   *   definition of the struct but not a complete overwrite. This is what the
+   *   IR library uses chi nodes for.
    */
   predicate fullySupportedSsaVariable(Variable v) {
     v = any(SsaDefinition def).getAVariable() and
+    // After `foo(&x.field)` we need for there to be two definitions of `x`:
+    // the one that existed before the call to `foo` and the def-by-ref from
+    // the call. It's fundamental in SSA that each use is only associated with
+    // one def, so we can't easily get this effect with SSA.
+    not definitionByReference(v.getAnAccess(), _) and
     // SSA variables do not exist before their first assignment, but one
     // feature of this data flow library is to track where uninitialized data
     // ends up.
@@ -183,8 +191,7 @@ module FlowVar_internal {
     }
 
     override predicate definedByReference(Expr arg) {
-      definitionByReference(v.getAnAccess(), arg) and
-      arg = def.getDefinition()
+      none() // Not supported for SSA. See `fullySupportedSsaVariable`.
     }
 
     override predicate definedByInitialValue(LocalScopeVariable param) {
@@ -204,8 +211,6 @@ module FlowVar_internal {
       this.definedByExpr(_, _)
       or
       this.definedByInitialValue(_)
-      or
-      this.definedByReference(_)
     }
 
     /**
@@ -236,10 +241,7 @@ module FlowVar_internal {
     BlockVar() { this = TBlockVar(sbb, v) }
 
     override VariableAccess getAnAccess() {
-      exists(SubBasicBlock reached |
-        reached = getAReachedBlockVarSBB(this) and
-        variableAccessInSBB(v, reached, result)
-      )
+      variableAccessInSBB(v, getAReachedBlockVarSBB(this), result)
     }
 
     override predicate definedByInitialValue(LocalScopeVariable lsv) {
@@ -401,8 +403,7 @@ module FlowVar_internal {
       mid = getAReachedBlockVarSBB(start) and
       result = mid.getASuccessor() and
       not skipLoop(mid, result, sbbDef, v) and
-      not assignmentLikeOperation(result, v, _) and
-      not blockVarDefinedByReference(result, v, _)
+      not assignmentLikeOperation(result, v, _)
     )
   }
 
@@ -413,12 +414,6 @@ module FlowVar_internal {
     va.getTarget() = v and
     va = sbb.getANode() and
     not overwrite(va, _)
-    or
-    // Allow flow into a `VariableAccess` that is used as definition by
-    // reference. This flow is blocked by `getAReachedBlockVarSBB` because
-    // flow should not propagate past that.
-    va = sbb.getASuccessor().(VariableAccess) and
-    blockVarDefinedByReference(va, v, _)
   }
 
   /**
@@ -516,9 +511,6 @@ module FlowVar_internal {
    */
   predicate overwrite(VariableAccess va, ControlFlowNode node) {
     va = node.(AssignExpr).getLValue()
-    or
-    va = node and
-    definitionByReference(node, _)
   }
 
   /**

cpp/ql/src/semmle/code/cpp/ir/IRSanity.ql

@@ -1,7 +1,7 @@
 /**
  * @name IR Sanity Check
  * @description Performs sanity checks on the Intermediate Representation. This query should have no results. 
- * @kind problem
+ * @kind table
  * @id cpp/ir-sanity-check
  */
 

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/IRSanity.ql

@@ -1,7 +1,7 @@
 /**
  * @name Aliased SSA IR Sanity Check
  * @description Performs sanity checks on the Intermediate Representation. This query should have no results. 
- * @kind problem
+ * @kind table
  * @id cpp/aliased-ssa-ir-sanity-check
  */
 

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/IRSanity.ql

@@ -1,7 +1,7 @@
 /**
  * @name Raw IR Sanity Check
  * @description Performs sanity checks on the Intermediate Representation. This query should have no results. 
- * @kind problem
+ * @kind table
  * @id cpp/raw-ir-sanity-check
  */
 

cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/IRSanity.ql

@@ -1,7 +1,7 @@
 /**
  * @name SSA IR Sanity Check
  * @description Performs sanity checks on the Intermediate Representation. This query should have no results. 
- * @kind problem
+ * @kind table
  * @id cpp/ssa-ir-sanity-check
  */
 

cpp/ql/src/semmle/code/cpp/padding/SanityCheck.ql

@@ -1,7 +1,7 @@
 /**
  * @name Padding Sanity Check
  * @description Performs sanity checks for the padding library. This query should have no results. 
- * @kind problem
+ * @kind table
  * @id cpp/padding-sanity-check
  */
 

cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected

@@ -1,11 +1,16 @@
 | example.c:15:37:15:37 | b | example.c:19:6:19:6 | b |
 | example.c:15:44:15:46 | pos | example.c:24:24:24:26 | pos |
 | example.c:15:44:15:46 | pos | example.c:28:23:28:25 | pos |
+| example.c:17:19:17:22 | {...} | example.c:24:2:24:7 | coords |
 | example.c:17:19:17:22 | {...} | example.c:24:13:24:18 | coords |
+| example.c:17:19:17:22 | {...} | example.c:26:2:26:7 | coords |
+| example.c:17:19:17:22 | {...} | example.c:26:19:26:24 | coords |
 | example.c:24:13:24:30 | ... = ... | example.c:24:2:24:30 | ... = ... |
 | example.c:24:24:24:30 | ... + ... | example.c:24:13:24:30 | ... = ... |
 | example.c:26:13:26:16 | call to getX | example.c:26:2:26:25 | ... = ... |
 | example.c:26:18:26:24 | ref arg & ... | example.c:26:2:26:7 | coords |
+| example.c:26:18:26:24 | ref arg & ... | example.c:26:19:26:24 | coords |
+| example.c:28:22:28:25 | ref arg & ... | example.c:28:23:28:25 | pos |
 | test.cpp:6:12:6:17 | call to source | test.cpp:7:8:7:9 | t1 |
 | test.cpp:6:12:6:17 | call to source | test.cpp:8:8:8:9 | t1 |
 | test.cpp:6:12:6:17 | call to source | test.cpp:9:8:9:9 | t1 |
@@ -31,14 +36,22 @@
 | test.cpp:24:10:24:11 | t2 | test.cpp:26:8:26:9 | t1 |
 | test.cpp:430:48:430:54 | source1 | test.cpp:432:17:432:23 | source1 |
 | test.cpp:431:12:431:13 | 0 | test.cpp:432:11:432:13 | tmp |
+| test.cpp:431:12:431:13 | 0 | test.cpp:432:33:432:35 | tmp |
+| test.cpp:431:12:431:13 | 0 | test.cpp:433:8:433:10 | tmp |
 | test.cpp:432:10:432:13 | & ... | test.cpp:432:3:432:8 | call to memcpy |
+| test.cpp:432:10:432:13 | ref arg & ... | test.cpp:432:11:432:13 | tmp |
+| test.cpp:432:10:432:13 | ref arg & ... | test.cpp:432:33:432:35 | tmp |
 | test.cpp:432:10:432:13 | ref arg & ... | test.cpp:433:8:433:10 | tmp |
 | test.cpp:432:17:432:23 | source1 | test.cpp:432:10:432:13 | ref arg & ... |
 | test.cpp:436:53:436:59 | source1 | test.cpp:439:17:439:23 | source1 |
 | test.cpp:436:66:436:66 | b | test.cpp:441:7:441:7 | b |
 | test.cpp:437:12:437:13 | 0 | test.cpp:438:19:438:21 | tmp |
 | test.cpp:437:12:437:13 | 0 | test.cpp:439:11:439:13 | tmp |
+| test.cpp:437:12:437:13 | 0 | test.cpp:439:33:439:35 | tmp |
+| test.cpp:437:12:437:13 | 0 | test.cpp:440:8:440:10 | tmp |
+| test.cpp:437:12:437:13 | 0 | test.cpp:442:10:442:12 | tmp |
 | test.cpp:439:10:439:13 | & ... | test.cpp:439:3:439:8 | call to memcpy |
+| test.cpp:439:10:439:13 | ref arg & ... | test.cpp:439:11:439:13 | tmp |
 | test.cpp:439:10:439:13 | ref arg & ... | test.cpp:439:33:439:35 | tmp |
 | test.cpp:439:10:439:13 | ref arg & ... | test.cpp:440:8:440:10 | tmp |
 | test.cpp:439:10:439:13 | ref arg & ... | test.cpp:442:10:442:12 | tmp |

cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp

@@ -443,17 +443,17 @@ void flowThroughMemcpy_blockvar_with_local_flow(int source1, int b) {
   }
 }
 
-void cleanedByMemcpy_ssa(int clean1) {
+void cleanedByMemcpy_ssa(int clean1) { // currently modeled with BlockVar, not SSA
   int tmp;
   memcpy(&tmp, &clean1, sizeof tmp);
-  sink(tmp); // clean
+  sink(tmp); // clean [FALSE POSITIVE]
 }
 
 void cleanedByMemcpy_blockvar(int clean1) {
   int tmp;
   int *capture = &tmp;
   memcpy(&tmp, &clean1, sizeof tmp);
-  sink(tmp); // clean
+  sink(tmp); // clean [FALSE POSITIVE]
 }
 
 void intRefSource(int &ref_source);

cpp/ql/test/library-tests/dataflow/dataflow-tests/test.expected

@@ -30,10 +30,17 @@
 | test.cpp:433:8:433:10 | tmp | test.cpp:430:48:430:54 | source1 |
 | test.cpp:440:8:440:10 | tmp | test.cpp:436:53:436:59 | source1 |
 | test.cpp:442:10:442:12 | tmp | test.cpp:436:53:436:59 | source1 |
+| test.cpp:449:8:449:10 | tmp | test.cpp:447:7:447:9 | tmp |
+| test.cpp:456:8:456:10 | tmp | test.cpp:453:7:453:9 | tmp |
+| test.cpp:466:8:466:12 | local | test.cpp:464:7:464:11 | local |
 | test.cpp:466:8:466:12 | local | test.cpp:465:16:465:20 | ref arg local |
+| test.cpp:472:8:472:12 | local | test.cpp:470:7:470:11 | local |
 | test.cpp:472:8:472:12 | local | test.cpp:471:20:471:25 | ref arg & ... |
+| test.cpp:478:8:478:12 | local | test.cpp:476:7:476:11 | local |
 | test.cpp:478:8:478:12 | local | test.cpp:477:20:477:24 | ref arg local |
+| test.cpp:485:8:485:12 | local | test.cpp:483:7:483:11 | local |
 | test.cpp:485:8:485:12 | local | test.cpp:484:18:484:23 | ref arg & ... |
+| test.cpp:491:8:491:12 | local | test.cpp:489:7:489:11 | local |
 | test.cpp:491:8:491:12 | local | test.cpp:490:18:490:22 | ref arg local |
 | true_upon_entry.cpp:21:8:21:8 | x | true_upon_entry.cpp:17:11:17:16 | call to source |
 | true_upon_entry.cpp:29:8:29:8 | x | true_upon_entry.cpp:27:9:27:14 | call to source |

cpp/ql/test/library-tests/dataflow/dataflow-tests/test_diff.expected

@@ -11,10 +11,17 @@
 | test.cpp:430:48:430:54 | test.cpp:433:8:433:10 | AST only |
 | test.cpp:436:53:436:59 | test.cpp:440:8:440:10 | AST only |
 | test.cpp:436:53:436:59 | test.cpp:442:10:442:12 | AST only |
+| test.cpp:447:7:447:9 | test.cpp:449:8:449:10 | AST only |
+| test.cpp:453:7:453:9 | test.cpp:456:8:456:10 | AST only |
+| test.cpp:464:7:464:11 | test.cpp:466:8:466:12 | AST only |
 | test.cpp:465:16:465:20 | test.cpp:466:8:466:12 | AST only |
+| test.cpp:470:7:470:11 | test.cpp:472:8:472:12 | AST only |
 | test.cpp:471:20:471:25 | test.cpp:472:8:472:12 | AST only |
+| test.cpp:476:7:476:11 | test.cpp:478:8:478:12 | AST only |
 | test.cpp:477:20:477:24 | test.cpp:478:8:478:12 | AST only |
+| test.cpp:483:7:483:11 | test.cpp:485:8:485:12 | AST only |
 | test.cpp:484:18:484:23 | test.cpp:485:8:485:12 | AST only |
+| test.cpp:489:7:489:11 | test.cpp:491:8:491:12 | AST only |
 | test.cpp:490:18:490:22 | test.cpp:491:8:491:12 | AST only |
 | true_upon_entry.cpp:9:11:9:16 | true_upon_entry.cpp:13:8:13:8 | IR only |
 | true_upon_entry.cpp:62:11:62:16 | true_upon_entry.cpp:66:8:66:8 | IR only |

cpp/ql/test/library-tests/dataflow/dataflow-tests/uninitialized.expected

@@ -1,4 +1,20 @@
 | test.cpp:75:7:75:8 | u1 | test.cpp:76:8:76:9 | u1 |
 | test.cpp:83:7:83:8 | u2 | test.cpp:84:13:84:14 | u2 |
 | test.cpp:83:7:83:8 | u2 | test.cpp:85:8:85:9 | u2 |
+| test.cpp:447:7:447:9 | tmp | test.cpp:448:11:448:13 | tmp |
+| test.cpp:447:7:447:9 | tmp | test.cpp:449:8:449:10 | tmp |
 | test.cpp:453:7:453:9 | tmp | test.cpp:454:19:454:21 | tmp |
+| test.cpp:453:7:453:9 | tmp | test.cpp:455:11:455:13 | tmp |
+| test.cpp:453:7:453:9 | tmp | test.cpp:456:8:456:10 | tmp |
+| test.cpp:464:7:464:11 | local | test.cpp:465:16:465:20 | local |
+| test.cpp:464:7:464:11 | local | test.cpp:466:8:466:12 | local |
+| test.cpp:470:7:470:11 | local | test.cpp:471:21:471:25 | local |
+| test.cpp:470:7:470:11 | local | test.cpp:472:8:472:12 | local |
+| test.cpp:476:7:476:11 | local | test.cpp:477:20:477:24 | local |
+| test.cpp:476:7:476:11 | local | test.cpp:478:8:478:12 | local |
+| test.cpp:476:7:476:11 | local | test.cpp:479:9:479:13 | local |
+| test.cpp:483:7:483:11 | local | test.cpp:484:19:484:23 | local |
+| test.cpp:483:7:483:11 | local | test.cpp:485:8:485:12 | local |
+| test.cpp:489:7:489:11 | local | test.cpp:490:18:490:22 | local |
+| test.cpp:489:7:489:11 | local | test.cpp:491:8:491:12 | local |
+| test.cpp:489:7:489:11 | local | test.cpp:492:9:492:13 | local |

cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected

@@ -128,13 +128,24 @@
 | taint.cpp:164:19:164:24 | call to source | taint.cpp:168:8:168:14 | tainted |  |
 | taint.cpp:164:19:164:24 | call to source | taint.cpp:172:18:172:24 | tainted |  |
 | taint.cpp:165:22:165:25 | {...} | taint.cpp:170:10:170:15 | buffer |  |
+| taint.cpp:165:22:165:25 | {...} | taint.cpp:171:8:171:13 | buffer |  |
+| taint.cpp:165:22:165:25 | {...} | taint.cpp:172:10:172:15 | buffer |  |
+| taint.cpp:165:22:165:25 | {...} | taint.cpp:173:8:173:13 | buffer |  |
 | taint.cpp:165:24:165:24 | 0 | taint.cpp:165:22:165:25 | {...} | TAINT |
 | taint.cpp:170:10:170:15 | buffer | taint.cpp:170:3:170:8 | call to strcpy |  |
+| taint.cpp:170:10:170:15 | ref arg buffer | taint.cpp:170:10:170:15 | buffer |  |
 | taint.cpp:170:10:170:15 | ref arg buffer | taint.cpp:171:8:171:13 | buffer |  |
+| taint.cpp:170:10:170:15 | ref arg buffer | taint.cpp:172:10:172:15 | buffer |  |
+| taint.cpp:170:10:170:15 | ref arg buffer | taint.cpp:173:8:173:13 | buffer |  |
+| taint.cpp:171:8:171:13 | ref arg buffer | taint.cpp:171:8:171:13 | buffer |  |
 | taint.cpp:171:8:171:13 | ref arg buffer | taint.cpp:172:10:172:15 | buffer |  |
+| taint.cpp:171:8:171:13 | ref arg buffer | taint.cpp:173:8:173:13 | buffer |  |
 | taint.cpp:172:10:172:15 | buffer | taint.cpp:172:3:172:8 | call to strcat |  |
+| taint.cpp:172:10:172:15 | ref arg buffer | taint.cpp:172:10:172:15 | buffer |  |
 | taint.cpp:172:10:172:15 | ref arg buffer | taint.cpp:173:8:173:13 | buffer |  |
+| taint.cpp:173:8:173:13 | ref arg buffer | taint.cpp:173:8:173:13 | buffer |  |
 | taint.cpp:180:19:180:19 | p | taint.cpp:181:9:181:9 | p |  |
 | taint.cpp:181:9:181:9 | p | taint.cpp:181:8:181:9 | * ... | TAINT |
 | taint.cpp:185:11:185:16 | call to source | taint.cpp:186:11:186:11 | x |  |
+| taint.cpp:186:10:186:11 | ref arg & ... | taint.cpp:186:11:186:11 | x |  |
 | taint.cpp:186:11:186:11 | x | taint.cpp:186:10:186:11 | & ... | TAINT |

cpp/ql/test/query-tests/Security/CWE/CWE-732/UnsafeDaclSecurityDescriptor.cpp

@@ -89,4 +89,44 @@ void Test()
         NULL,       // DACL is going to be removed from security descriptor. Default/inherited access ==> should not be flagged
         FALSE);
 
-}
+}
+
+PACL returnUnknownAcl();
+
+PACL returnNull() {
+  return NULL;
+}
+
+PACL returnMaybeAcl(bool b) {
+  PACL pDacl = NULL;
+  if (b) {
+    SetEntriesInAcl(0, NULL, NULL, &pDacl);
+  }
+  return pDacl;
+}
+
+void Test2()
+{
+    PSECURITY_DESCRIPTOR pSecurityDescriptor;
+
+    PACL pDacl1 = returnUnknownAcl();
+    SetSecurityDescriptorDacl(
+        pSecurityDescriptor,
+        TRUE,       // Dacl Present
+        pDacl1,     // Give `returnUnknownAcl` the benefit of the doubt ==> should not be flagged
+        FALSE);
+
+    PACL pDacl2 = returnNull();
+    SetSecurityDescriptorDacl(
+        pSecurityDescriptor,
+        TRUE,       // Dacl Present
+        pDacl2,     // NULL pointer to DACL  == BUG
+        FALSE);
+
+    PACL pDacl3 = returnMaybeAcl(true);
+    SetSecurityDescriptorDacl(
+        pSecurityDescriptor,
+        TRUE,       // Dacl Present
+        pDacl3,     // should not be flagged
+        FALSE);
+}

cpp/ql/test/query-tests/Security/CWE/CWE-732/UnsafeDaclSecurityDescriptor.expected

@@ -1,2 +1,3 @@
 | UnsafeDaclSecurityDescriptor.cpp:70:9:70:33 | call to SetSecurityDescriptorDacl | Setting a DACL to NULL in a SECURITY_DESCRIPTOR will result in an unprotected object. |
 | UnsafeDaclSecurityDescriptor.cpp:76:9:76:33 | call to SetSecurityDescriptorDacl | Setting a DACL to NULL in a SECURITY_DESCRIPTOR using variable pDacl that is set to NULL will result in an unprotected object. |
+| UnsafeDaclSecurityDescriptor.cpp:120:5:120:29 | call to SetSecurityDescriptorDacl | Setting a DACL to NULL in a SECURITY_DESCRIPTOR using variable pDacl2 that is set to NULL will result in an unprotected object. |

javascript/extractor/lib/typescript/src/ast_extractor.ts

@@ -7,6 +7,8 @@ import { Project } from "./common";
  */
 export interface AugmentedSourceFile extends ts.SourceFile {
     parseDiagnostics?: any[];
+    /** Internal property that we expose as a workaround. */
+    redirectInfo?: object | null;
     $tokens?: Token[];
     $symbol?: number;
     $lineStarts?: ReadonlyArray<number>;
@@ -154,7 +156,21 @@ export function augmentAst(ast: AugmentedSourceFile, code: string, project: Proj
         }
     }
 
-    forEachNode(ast, (node: AugmentedNode) => {
+    // Number of conditional type expressions the visitor is currently inside.
+    // We disable type extraction inside such type expressions, to avoid complications
+    // with `infer` types.
+    let insideConditionalTypes = 0;
+
+    visitAstNode(ast);
+    function visitAstNode(node: AugmentedNode) {
+        if (node.kind === ts.SyntaxKind.ConditionalType) {
+            ++insideConditionalTypes;
+        }
+        ts.forEachChild(node, visitAstNode);
+        if (node.kind === ts.SyntaxKind.ConditionalType) {
+            --insideConditionalTypes;
+        }
+
         // fill in line/column info
         if ("pos" in node) {
             node.$pos = augmentPos(node.pos, true);
@@ -174,7 +190,7 @@ export function augmentAst(ast: AugmentedSourceFile, code: string, project: Proj
             }
         }
 
-        if (typeChecker != null) {
+        if (typeChecker != null && insideConditionalTypes === 0) {
             if (isTypedNode(node)) {
                 let type = typeChecker.getTypeAtLocation(node);
                 if (type != null) {
@@ -245,7 +261,7 @@ export function augmentAst(ast: AugmentedSourceFile, code: string, project: Proj
                 }
             }
         }
-    });
+    }
 }
 
 type NamedNodeWithSymbol = AugmentedNode & (ts.ClassDeclaration | ts.InterfaceDeclaration

javascript/extractor/lib/typescript/src/main.ts

@@ -166,10 +166,7 @@ function getSourceCode(filename: string): string {
 }
 
 function extractFile(filename: string): string {
-    let {ast, code} = getAstForFile(filename);
-
-    // Get the AST and augment it.
-    ast_extractor.augmentAst(ast, code, state.project);
+    let ast = getAstForFile(filename);
 
     return stringifyAST({
         type: "ast",
@@ -204,18 +201,33 @@ function handleParseCommand(command: ParseCommand) {
     });
 }
 
+/**
+ * Returns true if the given source file has an actual AST, as opposed to
+ * a SourceFile that is a "redirect" to another SourceFile.
+ *
+ * The TypeScript API should not expose such redirecting SourceFiles,
+ * but we sometimes get them anyway.
+ */
+function isExtractableSourceFile(ast: ast_extractor.AugmentedSourceFile): boolean {
+    return ast.redirectInfo == null;
+}
+
 /**
  * Gets the AST and source code for the given file, either from
  * an already-open project, or by parsing the file.
  */
-function getAstForFile(filename: string): {ast: ts.SourceFile, code: string} {
+function getAstForFile(filename: string): ts.SourceFile {
     if (state.project != null) {
         let ast = state.project.program.getSourceFile(filename);
-        if (ast != null) {
-            return {ast, code: ast.text};
+        if (ast != null && isExtractableSourceFile(ast)) {
+            ast_extractor.augmentAst(ast, ast.text, state.project);
+            return ast;
         }
     }
-    return parseSingleFile(filename);
+    // Fall back to extracting without a project.
+    let {ast, code} = parseSingleFile(filename);
+    ast_extractor.augmentAst(ast, code, null);
+    return ast;
 }
 
 function parseSingleFile(filename: string): {ast: ts.SourceFile, code: string} {
@@ -360,7 +372,9 @@ function handleOpenProjectCommand(command: OpenProjectCommand) {
     function getEffectiveExportTarget(symbol: ts.Symbol) {
         if (symbol.exports != null && symbol.exports.has(ts.InternalSymbolName.ExportEquals)) {
             let exportAlias = symbol.exports.get(ts.InternalSymbolName.ExportEquals);
-            return typeChecker.getAliasedSymbol(exportAlias);
+            if (exportAlias.flags & ts.SymbolFlags.Alias) {
+                return typeChecker.getAliasedSymbol(exportAlias);
+            }
         }
         return symbol;
     }

javascript/extractor/lib/typescript/src/type_table.ts

@@ -819,8 +819,24 @@ export class TypeTable {
     this.isInShallowTypeContext = false;
   }
 
+  /**
+   * Returns the properties of the given type, or `null` if the properties of this
+   * type could not be computed.
+   */
+  private tryGetProperties(type: ts.Type) {
+    // Workaround for https://github.com/Microsoft/TypeScript/issues/30845
+    // Should be safe to remove once that has been fixed.
+    try {
+      return type.getProperties();
+    } catch (e) {
+      return null;
+    }
+  }
+
   private extractProperties(type: ts.Type, id: number) {
-    for (let symbol of type.getProperties()) {
+    let props = this.tryGetProperties(type);
+    if (props == null) return;
+    for (let symbol of props) {
       let propertyType = this.typeChecker.getTypeOfSymbolAtLocation(symbol, this.arbitraryAstNode);
       if (propertyType == null) continue;
       let propertyTypeId = this.getId(propertyType);

javascript/extractor/src/com/semmle/js/parser/TypeScriptParser.java

@@ -33,6 +33,8 @@ import java.io.OutputStream;
 import java.io.OutputStreamWriter;
 import java.lang.ProcessBuilder.Redirect;
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 
 /**
@@ -40,7 +42,12 @@ import java.util.List;
  *
  * <p>The Node.js half of the wrapper is expected to live at {@code
  * $SEMMLE_DIST/tools/typescript-parser-wrapper/main.js}; non-standard locations can be configured
- * using the property {@link #PARSER_WRAPPER_PATH_ENV_VAR}.
+ * using the property {@value #PARSER_WRAPPER_PATH_ENV_VAR}.
+ * 
+ * <p>The script launches the Node.js wrapper in the Node.js runtime, looking for {@code node}
+ * on the {@code PATH} by default. Non-standard locations can be configured using the property
+ * {@value #TYPESCRIPT_NODE_RUNTIME_VAR}, and additional arguments can be configured using the
+ * property {@value #TYPESCRIPT_NODE_RUNTIME_EXTRA_ARGS_VAR}.
  *
  * <p>The script is started upon parsing the first TypeScript file and then is kept running in the
  * background, passing it requests for parsing files and getting JSON-encoded ASTs as responses.
@@ -52,6 +59,18 @@ public class TypeScriptParser {
    */
   public static final String PARSER_WRAPPER_PATH_ENV_VAR = "SEMMLE_TYPESCRIPT_PARSER_WRAPPER";
 
+  /**
+   * An environment variable that can be set to indicate the location of the Node.js runtime,
+   * as an alternative to adding Node to the PATH.
+   */
+  public static final String TYPESCRIPT_NODE_RUNTIME_VAR = "SEMMLE_TYPESCRIPT_NODE_RUNTIME";
+
+  /**
+   * An environment variable that can be set to provide additional arguments to the Node.js runtime
+   * each time it is invoked. Arguments should be separated by spaces.
+   */
+  public static final String TYPESCRIPT_NODE_RUNTIME_EXTRA_ARGS_VAR = "SEMMLE_TYPESCRIPT_NODE_RUNTIME_EXTRA_ARGS";
+
   /**
    * An environment variable that can be set to specify a timeout to use when verifying the
    * TypeScript installation, in milliseconds. Default is 10000.
@@ -91,6 +110,15 @@ public class TypeScriptParser {
 
   private String nodeJsVersionString;
 
+  /** Command to launch the Node.js runtime. Initialised by {@link #verifyNodeInstallation}. */
+  private String nodeJsRuntime;
+
+  /**
+   * Arguments to pass to the Node.js runtime each time it is invoked.
+   * Initialised by {@link #verifyNodeInstallation}.
+   */
+  private List<String> nodeJsRuntimeExtraArgs = Collections.emptyList();
+
   /** If non-zero, we use this instead of relying on the corresponding environment variable. */
   private int typescriptRam = 0;
 
@@ -102,12 +130,16 @@ public class TypeScriptParser {
   /**
    * Verifies that Node.js and TypeScript are installed and throws an exception otherwise.
    *
-   * @param verbose if true, log the version strings and NODE_PATH.
+   * @param verbose if true, log the Node.js executable path, version strings, and any additional arguments.
    */
   public void verifyInstallation(boolean verbose) {
     verifyNodeInstallation();
     if (verbose) {
+      System.out.println("Found Node.js at: " + nodeJsRuntime);
       System.out.println("Found Node.js version: " + nodeJsVersionString);
+      if (!nodeJsRuntimeExtraArgs.isEmpty()) {
+        System.out.println("Additional arguments for Node.js: " + nodeJsRuntimeExtraArgs);
+      }
     }
   }
 
@@ -117,7 +149,24 @@ public class TypeScriptParser {
 
     ByteArrayOutputStream out = new ByteArrayOutputStream();
     ByteArrayOutputStream err = new ByteArrayOutputStream();
-    Builder b = new Builder(out, err, getParserWrapper().getParentFile(), "node", "--version");
+    
+    // Determine where to find the Node.js runtime.
+    String explicitNodeJsRuntime = Env.systemEnv().get(TYPESCRIPT_NODE_RUNTIME_VAR);
+    if (explicitNodeJsRuntime != null) {
+      // Use the specified Node.js executable.
+      nodeJsRuntime = explicitNodeJsRuntime;
+    } else {
+      // Look for `node` on the PATH.
+      nodeJsRuntime = "node";
+    }
+
+    // Determine any additional arguments to be passed to Node.js each time it's called.
+    String extraArgs = Env.systemEnv().get(TYPESCRIPT_NODE_RUNTIME_EXTRA_ARGS_VAR);
+    if (extraArgs != null) {
+      nodeJsRuntimeExtraArgs = Arrays.asList(extraArgs.split("\\s+"));
+    }
+
+    Builder b = new Builder(getNodeJsRuntimeInvocation("--version"), out, err, getParserWrapper().getParentFile());
     b.expectFailure(); // We want to do our own logging in case of an error.
 
     int timeout = Env.systemEnv().getInt(TYPESCRIPT_TIMEOUT_VAR, 10000);
@@ -144,6 +193,21 @@ public class TypeScriptParser {
     }
   }
 
+  /**
+   * Gets a command line to invoke the Node.js runtime.
+   * Any arguments in {@link TypeScriptParser#nodeJsRuntimeExtraArgs}
+   * are passed first, followed by those in {@code args}.
+   */
+  private List<String> getNodeJsRuntimeInvocation(String ...args) {
+    List<String> result = new ArrayList<>();
+    result.add(nodeJsRuntime);
+    result.addAll(nodeJsRuntimeExtraArgs);
+    for(String arg : args) {
+      result.add(arg);
+    }
+    return result;
+  }
+
   private static int getMegabyteCountFromPrefixedEnv(String suffix, int defaultValue) {
     String envVar = "SEMMLE_" + suffix;
     String value = Env.systemEnv().get(envVar);
@@ -172,10 +236,11 @@ public class TypeScriptParser {
     int reserveMemoryMb = getMegabyteCountFromPrefixedEnv(TYPESCRIPT_RAM_RESERVE_SUFFIX, 400);
 
     File parserWrapper = getParserWrapper();
-    List<String> cmd = new ArrayList<>();
-    cmd.add("node");
-    cmd.add("--max_old_space_size=" + (mainMemoryMb + reserveMemoryMb));
-    cmd.add(parserWrapper.getAbsolutePath());
+    
+    List<String> cmd = getNodeJsRuntimeInvocation(
+        "--max_old_space_size=" + (mainMemoryMb + reserveMemoryMb),
+        parserWrapper.getAbsolutePath()
+    );
     ProcessBuilder pb = new ProcessBuilder(cmd);
     parserWrapperCommand = StringUtil.glue(" ", cmd);
     pb.environment().put("SEMMLE_TYPESCRIPT_MEMORY_THRESHOLD", "" + mainMemoryMb);

javascript/ql/src/Security/CWE-116/IncompleteSanitization.ql

@@ -101,6 +101,34 @@ predicate allBackslashesEscaped(DataFlow::Node nd) {
   allBackslashesEscaped(nd.getAPredecessor())
 }
 
+/**
+ * Holds if `repl` looks like a call to "String.prototype.replace" that deliberately removes the first occurrence of `str`.
+ */
+predicate removesFirstOccurence(DataFlow::MethodCallNode repl, string str) {
+  repl.getMethodName() = "replace" and
+  repl.getArgument(0).getStringValue() = str and
+  repl.getArgument(1).getStringValue() = ""
+}
+
+/**
+ * Holds if `leftUnwrap` and `rightUnwrap` unwraps a string from a pair of surrounding delimiters.
+ */
+predicate isDelimiterUnwrapper(
+  DataFlow::MethodCallNode leftUnwrap, DataFlow::MethodCallNode rightUnwrap
+) {
+  exists(string left, string right |
+    left = "[" and right = "]"
+    or
+    left = "{" and right = "}"
+    or
+    left = "(" and right = ")"
+  |
+    removesFirstOccurence(leftUnwrap, left) and
+    removesFirstOccurence(rightUnwrap, right) and
+    leftUnwrap.getAMethodCall() = rightUnwrap
+  )
+}
+
 from MethodCallExpr repl, Expr old, string msg
 where
   repl.getMethodName() = "replace" and
@@ -122,7 +150,10 @@ where
       )
     ) and
     // don't flag replace operations in a loop
-    not DataFlow::valueNode(repl.getReceiver()) = DataFlow::valueNode(repl).getASuccessor+()
+    not DataFlow::valueNode(repl.getReceiver()) = DataFlow::valueNode(repl).getASuccessor+() and
+    // dont' flag unwrapper
+    not isDelimiterUnwrapper(repl.flow(), _) and
+    not isDelimiterUnwrapper(_, repl.flow())
     or
     exists(RegExpLiteral rel |
       isBackslashEscape(repl, rel) and

javascript/ql/test/library-tests/TypeScript/RegressionTests/ExportEqualsExpr/extern.d.ts

@@ -0,0 +1,14 @@
+class Foo {}
+
+declare module 'foo' {
+  export = new Foo();
+}
+
+declare module 'bar' {
+  import * as baz from "baz";
+  export = baz;
+}
+
+declare module 'baz' {
+  export class C {}
+}

javascript/ql/test/library-tests/TypeScript/RegressionTests/ExportEqualsExpr/test.expected

@@ -0,0 +1,7 @@
+| "bar" in global scope |
+| C in module 'bar' |
+| Foo in global scope |
+| Foo in tst.ts |
+| module 'bar' |
+| module 'foo' |
+| tst.ts |

javascript/ql/test/library-tests/TypeScript/RegressionTests/ExportEqualsExpr/test.ql

@@ -0,0 +1,4 @@
+import javascript
+
+from CanonicalName name
+select name

javascript/ql/test/library-tests/TypeScript/RegressionTests/ExportEqualsExpr/tsconfig.json

@@ -0,0 +1,6 @@
+{
+  "include": ["."],
+  "compilerOptions": {
+    "esModuleInterop": true
+  }
+}

javascript/ql/test/library-tests/TypeScript/RegressionTests/ExportEqualsExpr/tst.ts

@@ -0,0 +1,5 @@
+import self from "./tst";
+
+class Foo {}
+
+export = new Foo();

javascript/ql/test/library-tests/TypeScript/RegressionTests/GlobalThis/test.expected

@@ -0,0 +1 @@
+| Success |

javascript/ql/test/library-tests/TypeScript/RegressionTests/GlobalThis/test.ql

@@ -0,0 +1,3 @@
+import javascript
+
+select "Success"

javascript/ql/test/library-tests/TypeScript/RegressionTests/GlobalThis/tsconfig.json

@@ -0,0 +1,3 @@
+{
+  "include": ["."]
+}

javascript/ql/test/library-tests/TypeScript/RegressionTests/GlobalThis/tst.ts

@@ -0,0 +1,13 @@
+'use strict';
+
+var _myGlobal = this;
+
+module Test {
+  var global = _myGlobal || {};
+
+  export class C {}
+
+  export function f(x: C) {
+    global.field = x || {};
+  }
+}

javascript/ql/test/query-tests/Security/CWE-116/IncompleteSanitization/IncompleteSanitization.expected

@@ -15,3 +15,13 @@
 | tst.js:61:10:61:18 | s.replace | This replaces only the first occurrence of "'" + "". |
 | tst.js:65:10:65:18 | s.replace | This replaces only the first occurrence of "'". |
 | tst.js:69:10:69:18 | s.replace | This replaces only the first occurrence of "'" + "". |
+| tst.js:133:2:133:10 | s.replace | This replaces only the first occurrence of '<'. |
+| tst.js:133:2:133:27 | s.repla ... replace | This replaces only the first occurrence of '>'. |
+| tst.js:135:2:135:10 | s.replace | This replaces only the first occurrence of '['. |
+| tst.js:135:2:135:30 | s.repla ... replace | This replaces only the first occurrence of ']'. |
+| tst.js:136:2:136:10 | s.replace | This replaces only the first occurrence of '{'. |
+| tst.js:136:2:136:30 | s.repla ... replace | This replaces only the first occurrence of '}'. |
+| tst.js:140:2:140:10 | s.replace | This replaces only the first occurrence of /{/. |
+| tst.js:140:2:140:27 | s.repla ... replace | This replaces only the first occurrence of /}/. |
+| tst.js:141:2:141:10 | s.replace | This replaces only the first occurrence of ']'. |
+| tst.js:141:2:141:27 | s.repla ... replace | This replaces only the first occurrence of '['. |

javascript/ql/test/query-tests/Security/CWE-116/IncompleteSanitization/tst.js

@@ -126,6 +126,21 @@ function good11(s) {
   return s.replace("%d", "42");
 }
 
+function good12(s) {
+	s.replace('[', '').replace(']', ''); // OK
+	s.replace('(', '').replace(')', ''); // OK
+	s.replace('{', '').replace('}', ''); // OK
+	s.replace('<', '').replace('>', ''); // NOT OK: too common as a bad HTML sanitizer
+
+	s.replace('[', '\\[').replace(']', '\\]'); // NOT OK
+	s.replace('{', '\\{').replace('}', '\\}'); // NOT OK
+
+	s = s.replace('[', ''); // OK
+	s = s.replace(']', ''); // OK
+	s.replace(/{/, '').replace(/}/, ''); // NOT OK: should have used a string literal if a single replacement was intended
+	s.replace(']', '').replace('[', ''); // probably OK, but still flagged
+}
+
 app.get('/some/path', function(req, res) {
   let untrusted = req.param("p");
 
@@ -162,4 +177,5 @@ app.get('/some/path', function(req, res) {
   good10(untrusted);
   flowifyComments(untrusted);
   good11(untrusted);
+  good12(untrusted);
 });

python/ql/src/semmle/python/dataflow/DataFlow.qll

@@ -0,0 +1 @@
+import semmle.python.security.TaintTracking

python/ql/src/semmle/python/security/TaintTracking.qll

@@ -326,8 +326,6 @@ abstract class Sanitizer extends string {
 private predicate valid_sanitizer(Sanitizer sanitizer) {
     not exists(TaintTracking::Configuration c)
     or
-    exists(DataFlow::Configuration c | c.isSanitizer(sanitizer))
-    or
     exists(TaintTracking::Configuration c | c.isSanitizer(sanitizer))
 }
 
@@ -600,7 +598,7 @@ private newtype TTaintedNode =
         exists(DataFlow::Configuration config, TaintKind kind |
             taint = TaintFlowImplementation::TTrackedTaint(kind) and
             config.isSource(n) and context.getDepth() = 0 and
-            kind instanceof GenericFlowType
+            kind instanceof DataFlowType
         )
         or
         TaintFlowImplementation::step(_, taint, context, n) and
@@ -864,8 +862,6 @@ library module TaintFlowImplementation {
             (
                 not exists(TaintTracking::Configuration c)
                 or
-                exists(DataFlow::Configuration c | c.isExtension(fromnodenode))
-                or
                 exists(TaintTracking::Configuration c | c.isExtension(fromnodenode))
             )
             |
@@ -1090,8 +1086,6 @@ library module TaintFlowImplementation {
             (
                 not exists(TaintTracking::Configuration c)
                 or
-                exists(DataFlow::Configuration c | c.isExtension(originnode))
-                or
                 exists(TaintTracking::Configuration c | c.isExtension(originnode))
             ) and
             originnode.getASuccessorVariable() = var and
@@ -1537,16 +1531,12 @@ class CallContext extends TCallContext {
  */
 module DataFlow {
 
-    class FlowType = TaintKind;
-
     /** Generic taint kind, source and sink classes for convenience and
      * compatibility with other language libraries
      */
 
     class Node = ControlFlowNode;
 
-    class PathNode = TaintedNode;
-
     class Extension = DataFlowExtension::DataFlowNode;
 
     abstract class Configuration extends string {
@@ -1558,19 +1548,14 @@ module DataFlow {
 
         abstract predicate isSink(Node sink);
 
-        predicate isSanitizer(Sanitizer sanitizer) { none() }
-
-        predicate isExtension(Extension extension) { none() }
-
-        predicate hasFlowPath(PathNode source, PathNode sink) {
+        private predicate hasFlowPath(TaintedNode source, TaintedNode sink) {
             this.isSource(source.getNode()) and
             this.isSink(sink.getNode()) and
-            source.getTaintKind() instanceof GenericFlowType and
-            sink.getTaintKind() instanceof GenericFlowType
+            source.getASuccessor*() = sink
         }
 
         predicate hasFlow(Node source, Node sink) {
-            exists(PathNode psource, PathNode psink |
+            exists(TaintedNode psource, TaintedNode psink |
                 psource.getNode() = source and
                 psink.getNode() = sink and
                 this.isSource(source) and
@@ -1583,10 +1568,10 @@ module DataFlow {
 
 }
 
-private class GenericFlowType extends DataFlow::FlowType {
+private class DataFlowType extends TaintKind {
 
-    GenericFlowType() {
-        this = "Generic taint kind"  and
+    DataFlowType() {
+        this = "Data flow"  and
         exists(DataFlow::Configuration c)
     }
 

python/ql/test/2/library-tests/locations/general/AllLocations.ql

@@ -1,7 +1,7 @@
 /**
  * @name All must have locations
  * @description
- * @kind problem
+ * @kind table
  * @problem.severity error
  */
 

python/ql/test/3/library-tests/PointsTo/import_time/Pruned.ql

@@ -1,7 +1,7 @@
 /**
  * @name Naive
  * @description Insert description here...
- * @kind problem
+ * @kind table
  * @problem.severity warning
  */
 

python/ql/test/3/library-tests/locations/general/AllLocations.ql

@@ -1,7 +1,7 @@
 /**
  * @name All must have locations
  * @description
- * @kind problem
+ * @kind table
  * @problem.severity error
  */
 

python/ql/test/3/library-tests/types/namespaces/NameSpace.ql

@@ -1,7 +1,7 @@
 /**
  * @name ImportTimeScope
  * @description ImportTimeScope Test
- * @kind problem
+ * @kind table
  * @problem.severity warning
  */
 

python/ql/test/library-tests/ControlFlow/PointsToSupport/UseFromDefinition.ql

@@ -1,7 +1,7 @@
 /**
  * @name UseFromDefinition
  * @description Insert description here...
- * @kind problem
+ * @kind table
  * @problem.severity warning
  */
 

python/ql/test/library-tests/ControlFlow/augassign/SSA.ql

@@ -1,7 +1,7 @@
 /**
  * @name SSA
  * @description Insert description here...
- * @kind problem
+ * @kind table
  * @problem.severity warning
  */
 

python/ql/test/library-tests/ControlFlow/comparison/Compare.ql

@@ -1,7 +1,7 @@
 /**
  * @name CompareTest
  * @description CompareTest
- * @kind problem
+ * @kind table
  * @problem.severity warning
  */
 

python/ql/test/library-tests/ControlFlow/dominators/idom.ql

@@ -1,7 +1,7 @@
 /**
  * @name Check all non-scope nodes have an immediate dominator
  * @description Check all non-scope nodes have an immediate dominator
- * @kind problem
+ * @kind table
  * @problem.severity warning
  */
 

python/ql/test/library-tests/ControlFlow/truefalse/Branch.ql

@@ -1,7 +1,7 @@
 /**
  * @name Branch
  * @description Insert description here...
- * @kind problem
+ * @kind table
  * @problem.severity warning
  */
 

python/ql/test/library-tests/ControlFlow/truefalse/ExceptionalSuccessors.ql

@@ -1,7 +1,7 @@
 /**
  * @name TrueFalseSuccessors Test
  * @description Tests true/false successors
- * @kind problem
+ * @kind table
  * @problem.severity warning
  */
 

python/ql/test/library-tests/ControlFlow/truefalse/TrueFalseSuccessors.ql

@@ -1,7 +1,7 @@
 /**
  * @name TrueFalseSuccessors Test
  * @description Tests true/false successors
- * @kind problem
+ * @kind table
  * @problem.severity warning
  */
 

python/ql/test/library-tests/DefUse/Definitions.ql

@@ -1,7 +1,7 @@
 /**
  * @name Definitions
  * @description Insert description here...
- * @kind problem
+ * @kind table
  * @problem.severity warning
  */
 

python/ql/test/library-tests/DefUse/Uses.ql

@@ -1,7 +1,7 @@
 /**
  * @name Usages
  * @description Insert description here...
- * @kind problem
+ * @kind table
  * @problem.severity warning
  */
 

python/ql/test/library-tests/DuplicateCode/Duplicate.ql

@@ -1,7 +1,7 @@
 /**
  * @name Duplicate
  * @description Insert description here...
- * @kind problem
+ * @kind table
  * @problem.severity warning
  */
 

python/ql/test/library-tests/DuplicateCode/Similar.ql

@@ -1,7 +1,7 @@
 /**
  * @name Similar
  * @description Insert description here...
- * @kind problem
+ * @kind table
  * @problem.severity warning
  */
 

python/ql/test/library-tests/PointsTo/general/LocalPointsTo.ql

@@ -1,7 +1,7 @@
 /**
  * @name LocalPointsTo
  * @description Insert description here...
- * @kind problem
+ * @kind table
  * @problem.severity warning
  */
 

python/ql/test/library-tests/comments/blocks.ql

@@ -1,7 +1,7 @@
 /**
  * @name commented_out_code
  * @description Insert description here...
- * @kind problem
+ * @kind table
  * @problem.severity warning
  */
 

python/ql/test/library-tests/dependencies/Categories.ql

@@ -1,7 +1,7 @@
 /**
  * @name Categories
  * @description Insert description here...
- * @kind problem
+ * @kind table
  * @problem.severity warning
  */
 

python/ql/test/library-tests/taint/dataflow/Config.qll

@@ -0,0 +1,17 @@
+import python
+import semmle.python.dataflow.DataFlow
+
+class TestConfiguration extends DataFlow::Configuration {
+
+    TestConfiguration() { this = "Test configuration" }
+
+    override predicate isSource(ControlFlowNode source) { source.(NameNode).getId() = "SOURCE" }
+
+    override predicate isSink(ControlFlowNode sink) {
+        exists(CallNode call |
+            call.getFunction().(NameNode).getId() = "SINK" and
+            sink = call.getAnArg()
+        )
+    }
+
+}

python/ql/test/library-tests/taint/dataflow/Dataflow.expected

@@ -0,0 +1,16 @@
+| test.py:3:10:3:15 | ControlFlowNode for SOURCE | test.py:3:10:3:15 | ControlFlowNode for SOURCE |
+| test.py:6:9:6:14 | ControlFlowNode for SOURCE | test.py:7:10:7:10 | ControlFlowNode for s |
+| test.py:10:12:10:17 | ControlFlowNode for SOURCE | test.py:13:10:13:12 | ControlFlowNode for arg |
+| test.py:10:12:10:17 | ControlFlowNode for SOURCE | test.py:17:10:17:10 | ControlFlowNode for t |
+| test.py:20:9:20:14 | ControlFlowNode for SOURCE | test.py:13:10:13:12 | ControlFlowNode for arg |
+| test.py:37:13:37:18 | ControlFlowNode for SOURCE | test.py:41:14:41:14 | ControlFlowNode for t |
+| test.py:62:13:62:18 | ControlFlowNode for SOURCE | test.py:13:10:13:12 | ControlFlowNode for arg |
+| test.py:67:13:67:18 | ControlFlowNode for SOURCE | test.py:13:10:13:12 | ControlFlowNode for arg |
+| test.py:76:9:76:14 | ControlFlowNode for SOURCE | test.py:78:10:78:10 | ControlFlowNode for t |
+| test.py:108:13:108:18 | ControlFlowNode for SOURCE | test.py:112:14:112:14 | ControlFlowNode for t |
+| test.py:139:10:139:15 | ControlFlowNode for SOURCE | test.py:140:14:140:14 | ControlFlowNode for t |
+| test.py:143:9:143:14 | ControlFlowNode for SOURCE | test.py:145:10:145:10 | ControlFlowNode for s |
+| test.py:148:10:148:15 | ControlFlowNode for SOURCE | test.py:152:10:152:13 | ControlFlowNode for Subscript |
+| test.py:149:18:149:23 | ControlFlowNode for SOURCE | test.py:153:10:153:17 | ControlFlowNode for Subscript |
+| test.py:158:9:158:14 | ControlFlowNode for SOURCE | test.py:160:14:160:14 | ControlFlowNode for t |
+| test.py:158:9:158:14 | ControlFlowNode for SOURCE | test.py:166:14:166:14 | ControlFlowNode for t |

python/ql/test/library-tests/taint/dataflow/Dataflow.ql

@@ -0,0 +1,7 @@
+
+import python
+import Config
+
+from TestConfiguration config, ControlFlowNode src, ControlFlowNode sink
+where config.hasFlow(src, sink)
+select src, sink

python/ql/test/library-tests/taint/dataflow/TestNode.expected

@@ -0,0 +1,72 @@
+| Taint Data flow | test.py:3 | SOURCE |  |
+| Taint Data flow | test.py:6 | SOURCE |  |
+| Taint Data flow | test.py:7 | s |  |
+| Taint Data flow | test.py:10 | SOURCE |  |
+| Taint Data flow | test.py:12 | arg | test.py:21 |
+| Taint Data flow | test.py:12 | arg | test.py:25 |
+| Taint Data flow | test.py:12 | arg | test.py:47 from test.py:55 |
+| Taint Data flow | test.py:12 | arg | test.py:51 from test.py:63 |
+| Taint Data flow | test.py:12 | arg | test.py:51 from test.py:70 |
+| Taint Data flow | test.py:13 | arg | test.py:21 |
+| Taint Data flow | test.py:13 | arg | test.py:25 |
+| Taint Data flow | test.py:13 | arg | test.py:47 from test.py:55 |
+| Taint Data flow | test.py:13 | arg | test.py:51 from test.py:63 |
+| Taint Data flow | test.py:13 | arg | test.py:51 from test.py:70 |
+| Taint Data flow | test.py:16 | source() |  |
+| Taint Data flow | test.py:17 | t |  |
+| Taint Data flow | test.py:20 | SOURCE |  |
+| Taint Data flow | test.py:21 | t |  |
+| Taint Data flow | test.py:24 | source() |  |
+| Taint Data flow | test.py:25 | t |  |
+| Taint Data flow | test.py:31 | SOURCE |  |
+| Taint Data flow | test.py:37 | SOURCE |  |
+| Taint Data flow | test.py:41 | t |  |
+| Taint Data flow | test.py:44 | source() |  |
+| Taint Data flow | test.py:46 | arg | test.py:55 |
+| Taint Data flow | test.py:47 | arg | test.py:55 |
+| Taint Data flow | test.py:49 | arg | test.py:63 |
+| Taint Data flow | test.py:49 | arg | test.py:70 |
+| Taint Data flow | test.py:51 | arg | test.py:63 |
+| Taint Data flow | test.py:51 | arg | test.py:70 |
+| Taint Data flow | test.py:54 | source2() |  |
+| Taint Data flow | test.py:55 | t |  |
+| Taint Data flow | test.py:62 | SOURCE |  |
+| Taint Data flow | test.py:63 | t |  |
+| Taint Data flow | test.py:67 | SOURCE |  |
+| Taint Data flow | test.py:70 | t |  |
+| Taint Data flow | test.py:72 | arg | test.py:77 |
+| Taint Data flow | test.py:73 | arg | test.py:77 |
+| Taint Data flow | test.py:76 | SOURCE |  |
+| Taint Data flow | test.py:77 | hub() |  |
+| Taint Data flow | test.py:77 | t |  |
+| Taint Data flow | test.py:78 | t |  |
+| Taint Data flow | test.py:108 | SOURCE |  |
+| Taint Data flow | test.py:112 | t |  |
+| Taint Data flow | test.py:118 | SOURCE |  |
+| Taint Data flow | test.py:120 | t |  |
+| Taint Data flow | test.py:128 | SOURCE |  |
+| Taint Data flow | test.py:129 | t |  |
+| Taint Data flow | test.py:139 | SOURCE |  |
+| Taint Data flow | test.py:140 | t |  |
+| Taint Data flow | test.py:143 | SOURCE |  |
+| Taint Data flow | test.py:144 | s |  |
+| Taint Data flow | test.py:145 | s |  |
+| Taint Data flow | test.py:148 | SOURCE |  |
+| Taint Data flow | test.py:149 | SOURCE |  |
+| Taint Data flow | test.py:152 | Subscript |  |
+| Taint Data flow | test.py:153 | Subscript |  |
+| Taint Data flow | test.py:158 | SOURCE |  |
+| Taint Data flow | test.py:159 | t |  |
+| Taint Data flow | test.py:160 | t |  |
+| Taint Data flow | test.py:163 | t |  |
+| Taint Data flow | test.py:166 | t |  |
+| Taint [Data flow] | test.py:148 | List |  |
+| Taint [Data flow] | test.py:150 | l |  |
+| Taint [Data flow] | test.py:152 | x |  |
+| Taint [Data flow] | test.py:154 | l |  |
+| Taint [Data flow] | test.py:154 | list() |  |
+| Taint {Data flow} | test.py:149 | Dict |  |
+| Taint {Data flow} | test.py:151 | d |  |
+| Taint {Data flow} | test.py:153 | y |  |
+| Taint {Data flow} | test.py:155 | d |  |
+| Taint {Data flow} | test.py:155 | dict() |  |

python/ql/test/library-tests/taint/dataflow/TestNode.ql

@@ -0,0 +1,5 @@
+import python
+import Config
+
+from TaintedNode n
+select n.getTrackedValue(), n.getLocation().toString(), n.getNode().getNode().toString(), n.getContext()

python/ql/test/library-tests/taint/dataflow/test.py

@@ -0,0 +1,167 @@
+
+def test1():
+    SINK(SOURCE)
+
+def test2():
+    s = SOURCE
+    SINK(s)
+
+def source():
+    return SOURCE
+
+def sink(arg):
+    SINK(arg)
+
+def test3():
+    t = source()
+    SINK(t)
+
+def test4():
+    t = SOURCE
+    sink(t)
+
+def test5():
+    t = source()
+    sink(t)
+
+def test6(cond):
+    if cond:
+        t = "Safe"
+    else:
+        t = SOURCE
+    if cond:
+        SINK(t)
+
+def test7(cond):
+    if cond:
+        t = SOURCE
+    else:
+        t = "Safe"
+    if cond:
+        SINK(t)
+
+def source2(arg):
+    return source(arg)
+
+def sink2(arg):
+    sink(arg)
+
+def sink3(cond, arg):
+    if cond:
+        sink(arg)
+
+def test8(cond):
+    t = source2()
+    sink2(t)
+
+#False positive
+def test9(cond):
+    if cond:
+        t  = "Safe"
+    else:
+        t = SOURCE
+    sink3(cond, t)
+
+def test10(cond):
+    if cond:
+        t = SOURCE
+    else:
+        t = "Safe"
+    sink3(cond, t)
+
+def hub(arg):
+    return arg
+
+def test11():
+    t = SOURCE
+    t = hub(t)
+    SINK(t)
+
+def test12():
+    t = "safe"
+    t = hub(t)
+    SINK(t)
+
+import module
+
+def test13():
+    t = module.dangerous
+    SINK(t)
+
+def test14():
+    t = module.safe
+    SINK(t)
+
+def test15():
+    t = module.safe2
+    SINK(t)
+
+def test16():
+    t = module.dangerous_func()
+    SINK(t)
+
+
+def test20(cond):
+    if cond:
+        t = CUSTOM_SOURCE
+    else:
+        t = SOURCE
+    if cond:
+        CUSTOM_SINK(t)
+    else:
+        SINK(t)
+
+def test21(cond):
+    if cond:
+        t = CUSTOM_SOURCE
+    else:
+        t = SOURCE
+    if not cond:
+        CUSTOM_SINK(t)
+    else:
+        SINK(t)
+
+def test22(cond):
+    if cond:
+        t = CUSTOM_SOURCE
+    else:
+        t = SOURCE
+    t = TAINT_FROM_ARG(t)
+    if cond:
+        CUSTOM_SINK(t)
+    else:
+        SINK(t)
+
+from module import dangerous as unsafe
+SINK(unsafe)
+
+def test23():
+    with SOURCE as t:
+        SINK(t)
+
+def test24():
+    s = SOURCE
+    SANITIZE(s)
+    SINK(s)
+
+def test_update_extend(x, y):
+    l = [SOURCE]
+    d = {"key" : SOURCE}
+    x.extend(l)
+    y.update(d)
+    SINK(x[0])
+    SINK(y["key"])
+    l2 = list(l)
+    d2 = dict(d)
+
+def test_truth():
+    t = SOURCE
+    if t:
+        SINK(t)
+    else:
+        SINK(t)
+    if not t:
+        SINK(t)
+    else:
+        SINK(t)
+