Merge from master

Author: dave-bartolomeo

change-notes/1.23/analysis-javascript.md

@@ -22,14 +22,16 @@
 | **Query**                      | **Expected impact**          | **Change**                                                                |
 |--------------------------------|------------------------------|---------------------------------------------------------------------------|
 | Incomplete string escaping or encoding (`js/incomplete-sanitization`) | Fewer false-positive results | This rule now recognizes additional ways delimiters can be stripped away. |
-| Client-side cross-site scripting (`js/xss`) | More results | More potential vulnerabilities involving functions that manipulate DOM attributes are now recognized. |
+| Client-side cross-site scripting (`js/xss`) | More results, fewer false-positive results | More potential vulnerabilities involving functions that manipulate DOM attributes are now recognized, and more sanitizers are detected. |
 | Code injection (`js/code-injection`) | More results | More potential vulnerabilities involving functions that manipulate DOM event handler attributes are now recognized. |
 | Hard-coded credentials (`js/hardcoded-credentials`) | Fewer false-positive results | This rule now flags fewer password examples. |
 | Illegal invocation (`js/illegal-invocation`) | Fewer false-positive results | This rule now correctly handles methods named `call` and `apply`. |
-| Incorrect suffix check (`js/incorrect-suffix-check`) | Fewer false-positive results | The query recognizes valid checks in more cases. 
+| Incorrect suffix check (`js/incorrect-suffix-check`) | Fewer false-positive results | The query recognizes valid checks in more cases. |
 | Network data written to file (`js/http-to-file-access`) | Fewer false-positive results | This query has been renamed to better match its intended purpose, and now only considers network data untrusted. | 
 | Password in configuration file (`js/password-in-configuration-file`) | Fewer false-positive results | This rule now flags fewer password examples. |
 | Prototype pollution (`js/prototype-pollution`) | More results | The query now highlights vulnerable uses of jQuery and Angular, and the results are shown on LGTM by default. |
+| Reflected cross-site scripting (`js/reflected-xss`) | Fewer false-positive results | The query now recognizes more sanitizers. |
+| Stored cross-site scripting (`js/stored-xss`) | Fewer false-positive results | The query now recognizes more sanitizers. |
 | Uncontrolled command line (`js/command-line-injection`) | More results | This query now treats responses from servers as untrusted. |
 
 ## Changes to QL libraries

cpp/ql/src/semmle/code/cpp/exprs/Expr.qll

@@ -131,6 +131,8 @@ class Expr extends StmtParent, @expr {
     valuebind(_, underlyingElement(this))
     or
     addressConstantExpression(this)
+    or
+    constantTemplateLiteral(this)
   }
 
   /**
@@ -1119,3 +1121,17 @@ private predicate isStandardPlacementNewAllocator(Function operatorNew) {
 
 // Pulled out for performance. See QL-796.
 private predicate hasNoConversions(Expr e) { not e.hasConversion() }
+
+/**
+ * Holds if `e` is a literal of unknown value in a template, or a cast thereof.
+ * We assume that such literals are constant.
+ */
+private predicate constantTemplateLiteral(Expr e) {
+  // Unknown literals in uninstantiated templates could be enum constant
+  // accesses or pointer-to-member literals.
+  e instanceof Literal and
+  e.isFromUninstantiatedTemplate(_) and
+  not exists(e.getValue())
+  or
+  constantTemplateLiteral(e.(Cast).getExpr())
+}

cpp/ql/src/semmle/code/cpp/exprs/Literal.qll

@@ -132,7 +132,6 @@ class HexLiteral extends Literal {
  * A C/C++ aggregate literal.
  */
 class AggregateLiteral extends Expr, @aggregateliteral {
-  // if this is turned into a Literal we need to change mayBeImpure
   override string getCanonicalQLClass() { result = "AggregateLiteral" }
 
   /**
@@ -145,6 +144,10 @@ class AggregateLiteral extends Expr, @aggregateliteral {
     result = this.(ClassAggregateLiteral).getFieldExpr(f)
   }
 
+  override predicate mayBeImpure() { this.getAChild().mayBeImpure() }
+
+  override predicate mayBeGloballyImpure() { this.getAChild().mayBeGloballyImpure() }
+
   /** Gets a textual representation of this aggregate literal. */
   override string toString() { result = "{...}" }
 }

cpp/ql/src/semmle/code/cpp/internal/AddressConstantExpression.qll

@@ -32,6 +32,11 @@ private predicate constantAddressLValue(Expr lvalue) {
   // tells us how it's going to be used.
   lvalue.(FunctionAccess).getType() instanceof RoutineType
   or
+  // Pointer-to-member literals in uninstantiated templates
+  lvalue instanceof Literal and
+  not exists(lvalue.getValue()) and
+  lvalue.isFromUninstantiatedTemplate(_)
+  or
   // String literals have array types and undergo array-to-pointer conversion.
   lvalue instanceof StringLiteral
   or
@@ -61,6 +66,10 @@ private predicate constantAddressPointer(Expr pointer) {
   // tells us how it's going to be used.
   pointer.(FunctionAccess).getType() instanceof FunctionPointerType
   or
+  // Pointer to member function. These accesses are always pointers even though
+  // their type is `RoutineType`.
+  pointer.(FunctionAccess).getTarget() instanceof MemberFunction
+  or
   addressConstantVariable(pointer.(VariableAccess).getTarget()) and
   pointer.getType().getUnderlyingType() instanceof PointerType
   or

cpp/ql/src/semmle/code/cpp/models/interfaces/ArrayFunction.qll

@@ -1,11 +1,11 @@
 /**
  * Provides an abstract class for accurate modeling of input and output buffers
  * in library functions when source code is not available.  To use this QL
- * library, create a QL class extending `BufferFunction` with a characteristic
+ * library, create a QL class extending `ArrayFunction` with a characteristic
  * predicate that selects the function or set of functions you are trying to
- * model. Within that class, override the predicates provided by `BufferFunction`
+ * model. Within that class, override the predicates provided by `ArrayFunction`
  * to match the flow within that function.  Finally, add a private import
- * statement to `CustomModels.qll`
+ * statement to `Models.qll`
  */
 
 import semmle.code.cpp.Function

cpp/ql/test/library-tests/namespaces/same_name/decls.expected

@@ -7,5 +7,6 @@
 | file://:0:0:0:0 | (global namespace) | file://:0:0:0:0 | p#0 |
 | file://:0:0:0:0 | (global namespace) | file://:0:0:0:0 | p#0 |
 | file://:0:0:0:0 | (global namespace) | file://:0:0:0:0 | reg_save_area |
-| same_name.cpp:4:11:4:21 | namespace_a | same_name.cpp:2:11:2:11 | c |
+| file://:0:0:0:0 | (global namespace) | same_name.cpp:2:11:2:11 | c |
 | same_name.cpp:4:11:4:21 | namespace_a | same_name.cpp:6:12:6:12 | c |
+| same_name.cpp:9:11:9:21 | namespace_b | same_name.cpp:11:12:11:12 | c |

cpp/ql/test/library-tests/namespaces/same_name/same_name.cpp

@@ -8,9 +8,5 @@ namespace namespace_a
 
 namespace namespace_b
 {
-	//const int c = 1;
-	//
-	// this example is causing a DBCheck failure along the lines of:
-	// 
-	// [INVALID_KEY] Relation namespacembrs((@namespace parentid, unique @namespacembr memberid)): Value 132 of key field memberid occurs in several tuples. Two such tuples are: (134,132) and (144,132)
+	const int c = 1;
 }

cpp/ql/test/library-tests/sideEffects/exprs/exprs.c

@@ -1,5 +1,5 @@
 
-void f1(int p) {
+int f1(int p) {
     int i;
 
     for (
@@ -11,3 +11,20 @@ void f1(int p) {
 
     return p;
 }
+
+int global_int;
+
+int f2(void) {
+    global_int = 3;
+    return 1;
+}
+
+int f3(void) {
+    return 2;
+}
+
+void f4(void) {
+    int is0[3] = { 3, 4, 5 };
+    int is1[3] = { 3, f2(), 5 };
+    int is2[3] = { 3, f3(), 5 };
+}

cpp/ql/test/library-tests/sideEffects/exprs/exprs.expected

@@ -10,6 +10,26 @@
 | exprs.c:9:3:9:5 | ++ ... |  | mayBeImpure |  |
 | exprs.c:9:5:9:5 | p | isPure |  |  |
 | exprs.c:12:12:12:12 | p | isPure |  |  |
+| exprs.c:18:5:18:14 | global_int | isPure |  |  |
+| exprs.c:18:5:18:18 | ... = ... |  | mayBeImpure | mayBeGloballyImpure |
+| exprs.c:18:18:18:18 | 3 | isPure |  |  |
+| exprs.c:19:12:19:12 | 1 | isPure |  |  |
+| exprs.c:23:12:23:12 | 2 | isPure |  |  |
+| exprs.c:27:13:27:13 | 3 | isPure |  |  |
+| exprs.c:27:17:27:28 | {...} | isPure |  |  |
+| exprs.c:27:20:27:20 | 3 | isPure |  |  |
+| exprs.c:27:23:27:23 | 4 | isPure |  |  |
+| exprs.c:27:26:27:26 | 5 | isPure |  |  |
+| exprs.c:28:13:28:13 | 3 | isPure |  |  |
+| exprs.c:28:17:28:31 | {...} |  | mayBeImpure | mayBeGloballyImpure |
+| exprs.c:28:20:28:20 | 3 | isPure |  |  |
+| exprs.c:28:23:28:24 | call to f2 |  | mayBeImpure | mayBeGloballyImpure |
+| exprs.c:28:29:28:29 | 5 | isPure |  |  |
+| exprs.c:29:13:29:13 | 3 | isPure |  |  |
+| exprs.c:29:17:29:31 | {...} | isPure |  |  |
+| exprs.c:29:20:29:20 | 3 | isPure |  |  |
+| exprs.c:29:23:29:24 | call to f3 | isPure |  |  |
+| exprs.c:29:29:29:29 | 5 | isPure |  |  |
 | exprs.cpp:7:10:7:16 | (...) | isPure |  |  |
 | exprs.cpp:7:10:7:16 | (reference to) | isPure |  |  |
 | exprs.cpp:7:11:7:15 | * ... | isPure |  |  |

cpp/ql/test/library-tests/syntax-zoo/aliased_ssa_sanity.expected

@@ -20,7 +20,7 @@ instructionWithoutSuccessor
 | ms_try_mix.cpp:11:12:11:15 | Chi: call to C |
 | ms_try_mix.cpp:28:12:28:15 | Chi: call to C |
 | ms_try_mix.cpp:48:10:48:13 | Chi: call to C |
-| pointer_to_member.cpp:35:11:35:21 | FieldAddress: {...} |
+| pointer_to_member.cpp:36:11:36:30 | FieldAddress: {...} |
 | stmt_expr.cpp:27:5:27:15 | Store: ... = ... |
 | vla.c:5:9:5:14 | VariableAddress: definition of matrix |
 | vla.c:11:6:11:16 | UnmodeledDefinition: vla_typedef |