Merge branch 'main' into rdmarsh2/cpp/output-iterators-2

Resolve merge conflict in tests

Author: Robert Marsh

change-notes/1.26/analysis-javascript.md

@@ -53,7 +53,9 @@
 | Client-side URL redirect (`js/client-side-unvalidated-url-redirection`) | More results | This query now recognizes some unsafe uses of `importScripts()` inside WebWorkers. |
 | Missing CSRF middleware (`js/missing-token-validation`) | More results | This query now recognizes writes to cookie and session variables as potentially vulnerable to CSRF attacks. |
 | Missing CSRF middleware (`js/missing-token-validation`) | Fewer results | This query now recognizes more ways of protecting against CSRF attacks. |
+| Client-side cross-site scripting (`js/xss`) | More results | This query now tracks data flow from `location.hash` more precisely. |
 
 
 ## Changes to libraries
 * The predicate `TypeAnnotation.hasQualifiedName` now works in more cases when the imported library was not present during extraction.
+* The class `DomBasedXss::Configuration` has been deprecated, as it has been split into `DomBasedXss::HtmlInjectionConfiguration` and `DomBasedXss::JQueryHtmlOrSelectorInjectionConfiguration`. Unless specifically working with jQuery sinks, subclasses should instead be based on `HtmlInjectionConfiguration`. To use both configurations in a query, see [Xss.ql](https://github.com/github/codeql/blob/main/javascript/ql/src/Security/CWE-079/Xss.ql) for an example.

cpp/change-notes/2020-10-21-erroneous-types.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* The `cpp/wrong-type-format-argument` and `cpp/non-portable-printf` queries have been hardened so that they do not produce nonsensical results on databases that contain errors (specifically the `ErroneousType`).

cpp/change-notes/2020-10-21-size-check-queries.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* The 'Not enough memory allocated for pointer type' (cpp/allocation-too-small) and 'Not enough memory allocated for array of pointer type' (cpp/suspicious-allocation-size) queries have been improved. Previously some allocations would be reported by both queries, this no longer occurs. In addition more allocation functions are now understood by both queries.

cpp/ql/src/Critical/SizeCheck.ql

@@ -13,30 +13,9 @@
  */
 
 import cpp
+import semmle.code.cpp.models.Models
 
-class Allocation extends FunctionCall {
-  Allocation() {
-    exists(string name |
-      this.getTarget().hasGlobalOrStdName(name) and
-      (name = "malloc" or name = "calloc" or name = "realloc")
-    )
-  }
-
-  private string getName() { this.getTarget().hasGlobalOrStdName(result) }
-
-  int getSize() {
-    this.getName() = "malloc" and
-    this.getArgument(0).getValue().toInt() = result
-    or
-    this.getName() = "realloc" and
-    this.getArgument(1).getValue().toInt() = result
-    or
-    this.getName() = "calloc" and
-    result = this.getArgument(0).getValue().toInt() * this.getArgument(1).getValue().toInt()
-  }
-}
-
-predicate baseType(Allocation alloc, Type base) {
+predicate baseType(AllocationExpr alloc, Type base) {
   exists(PointerType pointer |
     pointer.getBaseType() = base and
     (
@@ -54,11 +33,12 @@ predicate decideOnSize(Type t, int size) {
   size = min(t.getSize())
 }
 
-from Allocation alloc, Type base, int basesize, int allocated
+from AllocationExpr alloc, Type base, int basesize, int allocated
 where
   baseType(alloc, base) and
-  allocated = alloc.getSize() and
+  allocated = alloc.getSizeBytes() and
   decideOnSize(base, basesize) and
+  alloc.(FunctionCall).getTarget() instanceof AllocationFunction and // exclude `new` and similar
   basesize > allocated
 select alloc,
   "Type '" + base.getName() + "' is " + basesize.toString() + " bytes, but only " +

cpp/ql/src/Critical/SizeCheck2.ql

@@ -13,25 +13,9 @@
  */
 
 import cpp
+import semmle.code.cpp.models.Models
 
-class Allocation extends FunctionCall {
-  Allocation() { this.getTarget().hasGlobalOrStdName(["malloc", "calloc", "realloc"]) }
-
-  private string getName() { this.getTarget().hasGlobalOrStdName(result) }
-
-  int getSize() {
-    this.getName() = "malloc" and
-    this.getArgument(0).getValue().toInt() = result
-    or
-    this.getName() = "realloc" and
-    this.getArgument(1).getValue().toInt() = result
-    or
-    this.getName() = "calloc" and
-    result = this.getArgument(0).getValue().toInt() * this.getArgument(1).getValue().toInt()
-  }
-}
-
-predicate baseType(Allocation alloc, Type base) {
+predicate baseType(AllocationExpr alloc, Type base) {
   exists(PointerType pointer |
     pointer.getBaseType() = base and
     (
@@ -44,16 +28,23 @@ predicate baseType(Allocation alloc, Type base) {
   )
 }
 
-from Allocation alloc, Type base, int basesize, int allocated
+predicate decideOnSize(Type t, int size) {
+  // If the codebase has more than one type with the same name, it can have more than one size.
+  size = min(t.getSize())
+}
+
+from AllocationExpr alloc, Type base, int basesize, int allocated
 where
   baseType(alloc, base) and
-  allocated = alloc.getSize() and
+  allocated = alloc.getSizeBytes() and
+  decideOnSize(base, basesize) and
+  alloc.(FunctionCall).getTarget() instanceof AllocationFunction and // exclude `new` and similar
   // If the codebase has more than one type with the same name, check if any matches
   not exists(int size | base.getSize() = size |
     size = 0 or
     (allocated / size) * size = allocated
   ) and
-  basesize = min(base.getSize())
+  not basesize > allocated // covered by SizeCheck.ql
 select alloc,
   "Allocated memory (" + allocated.toString() + " bytes) is not a multiple of the size of '" +
     base.getName() + "' (" + basesize.toString() + " bytes)."

cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql

@@ -155,7 +155,8 @@ where
     not actual.getUnspecifiedType().(IntegralType).getSize() = sizeof_IntType()
   ) and
   not arg.isAffectedByMacro() and
-  not arg.isFromUninstantiatedTemplate(_)
+  not arg.isFromUninstantiatedTemplate(_) and
+  not actual.getUnspecifiedType() instanceof ErroneousType
 select arg,
   "This argument should be of type '" + expected.getName() + "' but is of type '" +
     actual.getUnspecifiedType().getName() + "'"

cpp/ql/src/Likely Bugs/Memory Management/Padding/NonPortablePrintf.ql

@@ -88,7 +88,8 @@ where
   not arg.isAffectedByMacro() and
   size32 = ilp32.paddedSize(actual) and
   size64 = lp64.paddedSize(actual) and
-  size64 != size32
+  size64 != size32 and
+  not actual instanceof ErroneousType
 select arg,
   "This argument should be of type '" + expected.getName() + "' but is of type '" + actual.getName()
     + "' (which changes size from " + size32 + " to " + size64 + " on 64-bit systems)."

cpp/ql/src/semmle/code/cpp/Print.qll

@@ -1,4 +1,17 @@
 import cpp
+private import PrintAST
+
+/**
+ * Print function declarations only if there is a `PrintASTConfiguration`
+ * that requests that function, or no `PrintASTConfiguration` exists.
+ */
+private predicate shouldPrintDeclaration(Declaration decl) {
+  not decl instanceof Function
+  or
+  not exists(PrintASTConfiguration c)
+  or
+  exists(PrintASTConfiguration config | config.shouldPrintFunction(decl))
+}
 
 /**
  * Gets a string containing the scope in which this declaration is declared.
@@ -48,6 +61,8 @@ private string getTemplateArgumentString(Declaration d, int i) {
  * A `Declaration` extended to add methods for generating strings useful only for dumps and debugging.
  */
 abstract private class DumpDeclaration extends Declaration {
+  DumpDeclaration() { shouldPrintDeclaration(this) }
+
   /**
    * Gets a string that uniquely identifies this declaration, suitable for use when debugging queries. Only holds for
    * functions, user-defined types, global and namespace-scope variables, and member variables.

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

@@ -7208,3 +7208,52 @@
 | vector.cpp:449:6:449:7 | it | vector.cpp:449:4:449:4 | call to operator++ |  |
 | vector.cpp:449:11:449:16 | call to source | vector.cpp:449:3:449:3 | ref arg call to operator* | TAINT |
 | vector.cpp:450:8:450:10 | ref arg out | vector.cpp:451:2:451:2 | out |  |
+| vector.cpp:467:22:467:25 | call to vector | vector.cpp:471:8:471:8 | v |  |
+| vector.cpp:467:22:467:25 | call to vector | vector.cpp:472:11:472:11 | v |  |
+| vector.cpp:467:22:467:25 | call to vector | vector.cpp:473:8:473:8 | v |  |
+| vector.cpp:467:22:467:25 | call to vector | vector.cpp:474:2:474:2 | v |  |
+| vector.cpp:468:11:468:16 | call to source | vector.cpp:472:18:472:18 | s |  |
+| vector.cpp:469:10:469:11 | 0 | vector.cpp:472:13:472:13 | i |  |
+| vector.cpp:471:8:471:8 | ref arg v | vector.cpp:472:11:472:11 | v |  |
+| vector.cpp:471:8:471:8 | ref arg v | vector.cpp:473:8:473:8 | v |  |
+| vector.cpp:471:8:471:8 | ref arg v | vector.cpp:474:2:474:2 | v |  |
+| vector.cpp:472:10:472:14 | & ... | vector.cpp:472:3:472:8 | call to memcpy |  |
+| vector.cpp:472:10:472:14 | ref arg & ... | vector.cpp:472:12:472:12 | call to operator[] [inner post update] |  |
+| vector.cpp:472:11:472:11 | ref arg v | vector.cpp:473:8:473:8 | v |  |
+| vector.cpp:472:11:472:11 | ref arg v | vector.cpp:474:2:474:2 | v |  |
+| vector.cpp:472:11:472:11 | v | vector.cpp:472:12:472:12 | call to operator[] | TAINT |
+| vector.cpp:472:12:472:12 | call to operator[] | vector.cpp:472:10:472:14 | & ... |  |
+| vector.cpp:472:12:472:12 | call to operator[] [inner post update] | vector.cpp:472:11:472:11 | ref arg v | TAINT |
+| vector.cpp:472:17:472:18 | & ... | vector.cpp:472:3:472:8 | call to memcpy | TAINT |
+| vector.cpp:472:17:472:18 | & ... | vector.cpp:472:10:472:14 | ref arg & ... | TAINT |
+| vector.cpp:472:18:472:18 | s | vector.cpp:472:10:472:14 | ref arg & ... |  |
+| vector.cpp:472:18:472:18 | s | vector.cpp:472:17:472:18 | & ... |  |
+| vector.cpp:473:8:473:8 | ref arg v | vector.cpp:474:2:474:2 | v |  |
+| vector.cpp:477:24:477:27 | call to vector | vector.cpp:483:8:483:9 | cs |  |
+| vector.cpp:477:24:477:27 | call to vector | vector.cpp:484:11:484:12 | cs |  |
+| vector.cpp:477:24:477:27 | call to vector | vector.cpp:486:8:486:9 | cs |  |
+| vector.cpp:477:24:477:27 | call to vector | vector.cpp:487:2:487:2 | cs |  |
+| vector.cpp:478:21:478:37 | call to source | vector.cpp:480:22:480:24 | src |  |
+| vector.cpp:478:21:478:37 | call to source | vector.cpp:482:8:482:10 | src |  |
+| vector.cpp:478:21:478:37 | call to source | vector.cpp:484:25:484:27 | src |  |
+| vector.cpp:478:21:478:37 | call to source | vector.cpp:485:8:485:10 | src |  |
+| vector.cpp:479:23:479:24 | 10 | vector.cpp:484:14:484:17 | offs |  |
+| vector.cpp:480:26:480:31 | call to length | vector.cpp:484:38:484:40 | len |  |
+| vector.cpp:482:8:482:10 | ref arg src | vector.cpp:484:25:484:27 | src |  |
+| vector.cpp:482:8:482:10 | ref arg src | vector.cpp:485:8:485:10 | src |  |
+| vector.cpp:483:8:483:9 | ref arg cs | vector.cpp:484:11:484:12 | cs |  |
+| vector.cpp:483:8:483:9 | ref arg cs | vector.cpp:486:8:486:9 | cs |  |
+| vector.cpp:483:8:483:9 | ref arg cs | vector.cpp:487:2:487:2 | cs |  |
+| vector.cpp:484:10:484:22 | & ... | vector.cpp:484:3:484:8 | call to memcpy |  |
+| vector.cpp:484:10:484:22 | ref arg & ... | vector.cpp:484:13:484:13 | call to operator[] [inner post update] |  |
+| vector.cpp:484:11:484:12 | cs | vector.cpp:484:13:484:13 | call to operator[] | TAINT |
+| vector.cpp:484:11:484:12 | ref arg cs | vector.cpp:486:8:486:9 | cs |  |
+| vector.cpp:484:11:484:12 | ref arg cs | vector.cpp:487:2:487:2 | cs |  |
+| vector.cpp:484:13:484:13 | call to operator[] | vector.cpp:484:10:484:22 | & ... |  |
+| vector.cpp:484:13:484:13 | call to operator[] [inner post update] | vector.cpp:484:11:484:12 | ref arg cs | TAINT |
+| vector.cpp:484:14:484:17 | offs | vector.cpp:484:14:484:21 | ... + ... | TAINT |
+| vector.cpp:484:21:484:21 | 1 | vector.cpp:484:14:484:21 | ... + ... | TAINT |
+| vector.cpp:484:25:484:27 | src | vector.cpp:484:29:484:33 | call to c_str | TAINT |
+| vector.cpp:484:29:484:33 | call to c_str | vector.cpp:484:3:484:8 | call to memcpy | TAINT |
+| vector.cpp:484:29:484:33 | call to c_str | vector.cpp:484:10:484:22 | ref arg & ... | TAINT |
+| vector.cpp:486:8:486:9 | ref arg cs | vector.cpp:487:2:487:2 | cs |  |

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

@@ -192,7 +192,7 @@ void *memcpy(void *dest, void *src, int len);
 void test_memcpy(int *source) {
 	int x;
 	memcpy(&x, source, sizeof(int));
-	sink(x);
+	sink(x); // tainted
 }
 
 // --- std::swap ---