Merge branch 'master' into python-support-django2

Author: RasmusWL

CONTRIBUTING.md

@@ -46,6 +46,12 @@ Follow the steps below to help other users understand what your query does, and
    Query help files explain the purpose of your query to other users. Write your query help in a `.qhelp` file and save it in the same directory as your new query. 
    For more information on writing query help, see the [Query help style guide](https://github.com/Semmle/ql/blob/master/docs/query-help-style-guide.md).
 
+7. **Maintain backwards compatibility**
+
+The standard CodeQL libraries must evolve in a backwards compatible manner. If any backwards incompatible changes need to be made, the existing API must first be marked as deprecated. This is done by adding a `deprecated` annotation along with a QLDoc reference to the replacement API. Only after at least one full release cycle has elapsed may the old API be removed.
+
+In addition to contributions to our standard queries and libraries, we also welcome contributions of a more experimental nature, which do not need to fulfill all the requirements listed above. See the guidelines for [experimental queries and libraries](docs/experimental.md) for details.
+
 ## Using your personal data
 
 If you contribute to this project, we will record your name and email

change-notes/1.24/analysis-cpp.md

@@ -27,10 +27,10 @@ The following changes in version 1.24 affect C/C++ analysis in all applications.
 
 ## Changes to libraries
 
-* The data-flow library has been improved when flow through functions needs to be
-  combined with both taint tracking and flow through fields allowing more flow
-  to be tracked. This affects and improves some security queries, which may
-  report additional results.
+* The data-flow library has been improved, which affects and improves some security queries. The improvements are:
+  - Track flow through functions that combine taint tracking with flow through fields.
+  - Track flow through clone-like functions, that is, functions that read contents of a field from a
+    parameter and stores the value in the field of a returned object.
 * Created the `semmle.code.cpp.models.interfaces.Allocation` library to model allocation such as `new` expressions and calls to `malloc`. This in intended to replace the functionality in `semmle.code.cpp.commons.Alloc` with a more consistent and useful interface.
 * Created the `semmle.code.cpp.models.interfaces.Deallocation` library to model deallocation such as `delete` expressions and calls to `free`. This in intended to replace the functionality in `semmle.code.cpp.commons.Alloc` with a more consistent and useful interface.
 * The new class `StackVariable` should be used in place of `LocalScopeVariable`
@@ -46,3 +46,5 @@ The following changes in version 1.24 affect C/C++ analysis in all applications.
   the following improvements:
   * The library now models data flow through `strdup` and similar functions.
   * The library now models data flow through formatting functions such as `sprintf`.
+* The security pack taint tracking library (`semmle.code.cpp.security.TaintTracking`) uses a new intermediate representation. This provides a more precise analysis of pointers to stack variables and flow through parameters, improving the results of many security queries.
+* The global value numbering library (`semmle.code.cpp.valuenumbering.GlobalValueNumbering`) uses a new intermediate representation to provide a more precise analysis of heap allocated memory and pointers to stack variables.

change-notes/1.24/analysis-csharp.md

@@ -33,10 +33,10 @@ The following changes in version 1.24 affect C# analysis in all applications.
 
 ## Changes to libraries
 
-* The data-flow library has been improved when flow through methods needs to be
-  combined with both taint tracking and flow through fields allowing more flow
-  to be tracked. This affects and improves most security queries, which may
-  report additional results.
+* The data-flow library has been improved, which affects and improves most security queries. The improvements are:
+  - Track flow through methods that combine taint tracking with flow through fields.
+  - Track flow through clone-like methods, that is, methods that read contents of a field from a
+    parameter and stores the value in the field of a returned object.
 * The taint tracking library now tracks flow through (implicit or explicit) conversion operator calls.
 * [Code contracts](https://docs.microsoft.com/en-us/dotnet/framework/debug-trace-profile/code-contracts) are now recognized, and are treated like any other assertion methods.
 * Expression nullability flow state is given by the predicates `Expr.hasNotNullFlowState()` and `Expr.hasMaybeNullFlowState()`.

change-notes/1.24/analysis-java.md

@@ -5,6 +5,7 @@ The following changes in version 1.24 affect Java analysis in all applications.
 ## General improvements
 
 * Alert suppression can now be done with single-line block comments (`/* ... */`) as well as line comments (`// ...`).
+* A `Customizations.qll` file has been added to allow customizations of the standard library that apply to all queries.
 
 ## New queries
 
@@ -26,10 +27,10 @@ The following changes in version 1.24 affect Java analysis in all applications.
 
 ## Changes to libraries
 
-* The data-flow library has been improved when flow through methods needs to be
-  combined with both taint tracking and flow through fields allowing more flow
-  to be tracked. This affects and improves most security queries, which may
-  report additional results.
+* The data-flow library has been improved, which affects and improves most security queries. The improvements are:
+  - Track flow through methods that combine taint tracking with flow through fields.
+  - Track flow through clone-like methods, that is, methods that read contents of a field from a
+    parameter and stores the value in the field of a returned object.
 * Identification of test classes has been improved. Previously, one of the
   match conditions would classify any class with a name containing the string
   "Test" as a test class, but now this matching has been replaced with one that

change-notes/1.24/analysis-javascript.md

@@ -2,15 +2,25 @@
 
 ## General improvements
 
+* TypeScript 3.8 is now supported.
+
 * Alert suppression can now be done with single-line block comments (`/* ... */`) as well as line comments (`// ...`).
 
 * Imports with the `.js` extension can now be resolved to a TypeScript file,
   when the import refers to a file generated by TypeScript.
 
 * Imports that rely on path-mappings from a `tsconfig.json` file can now be resolved.
 
+* Export declarations of the form `export * as ns from "x"` are now analyzed more precisely.
+
 * The analysis of sanitizer guards has improved, leading to fewer false-positive results from the security queries.
 
+* The call graph construction has been improved, leading to more results from the security queries:
+  - Calls can now be resolved to indirectly-defined class members in more cases.
+  - Calls through partial invocations such as `.bind` can now be resolved in more cases.
+
+* Support for flow summaries has been more clearly marked as being experimental and moved to the new `experimental` folder.
+
 * Support for the following frameworks and libraries has been improved:
   - [Electron](https://electronjs.org/)
   - [Handlebars](https://www.npmjs.com/package/handlebars)
@@ -24,6 +34,7 @@
   - [http2](https://nodejs.org/api/http2.html)
   - [lazy-cache](https://www.npmjs.com/package/lazy-cache)
   - [react](https://www.npmjs.com/package/react)
+  - [request](https://www.npmjs.com/package/request)
   - [send](https://www.npmjs.com/package/send)
   - [typeahead.js](https://www.npmjs.com/package/typeahead.js)
   - [ws](https://github.com/websockets/ws)
@@ -35,7 +46,11 @@
 | Cross-site scripting through exception (`js/xss-through-exception`) | security, external/cwe/cwe-079, external/cwe/cwe-116              | Highlights potential XSS vulnerabilities where an exception is written to the DOM. Results are not shown on LGTM by default. |
 | Regular expression always matches (`js/regex/always-matches`) | correctness, regular-expressions | Highlights regular expression checks that trivially succeed by matching an empty substring. Results are shown on LGTM by default. |
 | Missing await (`js/missing-await`) | correctness | Highlights expressions that operate directly on a promise object in a nonsensical way, instead of awaiting its result. Results are shown on LGTM by default. |
-| Prototype pollution in utility function (`js/prototype-pollution-utility`) | security, external/cwe/cwe-400, external/cwe/cwe-471 | Highlights recursive copying operations that are susceptible to prototype pollution. Results are shown on LGTM by default. |
+| Polynomial regular expression used on uncontrolled data (`js/polynomial-redos`) | security, external/cwe/cwe-730, external/cwe/cwe-400 | Highlights expensive regular expressions that may be used on malicious input. Results are shown on LGTM by default. | 
+| Prototype pollution in utility function (`js/prototype-pollution-utility`) | security, external/cwe/cwe-400, external/cwe/cwe-471 | Highlights recursive assignment operations that are susceptible to prototype pollution. Results are shown on LGTM by default. |
+| Unsafe jQuery plugin (`js/unsafe-jquery-plugin`) | Highlights potential XSS vulnerabilities in unsafely designed jQuery plugins. Results are shown on LGTM by default. |
+| Unnecessary use of `cat` process (`js/unnecessary-use-of-cat`) | correctness, security, maintainability | Highlights command executions of `cat` where the fs API should be used instead. Results are shown on LGTM by default. |
+
 
 ## Changes to existing queries
 
@@ -48,8 +63,10 @@
 | Expression has no effect (`js/useless-expression`) | Fewer false positive results | The query now recognizes block-level flow type annotations and ignores the first statement of a try block. |
 | Use of call stack introspection in strict mode (`js/strict-mode-call-stack-introspection`) | Fewer false positive results | The query no longer flags expression statements. |
 | Missing CSRF middleware (`js/missing-token-validation`) | Fewer false positive results | The query reports fewer duplicates and only flags handlers that explicitly access cookie data. |
-| Uncontrolled data used in path expression (`js/path-injection`) | More results | This query now recognizes additional ways dangerous paths can be constructed. |
+| Uncontrolled data used in path expression (`js/path-injection`) | More results | This query now recognizes additional ways dangerous paths can be constructed and used. |
 | Uncontrolled command line (`js/command-line-injection`) | More results | This query now recognizes additional ways of constructing arguments to `cmd.exe` and `/bin/sh`. |
+| Syntax error (`js/syntax-error`) | Lower severity | This results of this query are now displayed with lower severity. |
+| Use of password hash with insufficient computational effort (`js/insufficient-password-hash`) | Fewer false positive results | This query now recognizes additional cases that do not require secure hashing. |
 
 ## Changes to libraries
 

config/identical-files.json

@@ -222,10 +222,12 @@
     "cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/PrintSSA.qll",
     "csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/PrintSSA.qll"
   ],
-  "C++ IR ValueNumberInternal": [
+  "IR ValueNumberInternal": [
     "cpp/ql/src/semmle/code/cpp/ir/implementation/raw/gvn/internal/ValueNumberingInternal.qll",
     "cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/gvn/internal/ValueNumberingInternal.qll",
-    "cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/gvn/internal/ValueNumberingInternal.qll"
+    "cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/gvn/internal/ValueNumberingInternal.qll",
+    "csharp/ql/src/semmle/code/csharp/ir/implementation/raw/gvn/internal/ValueNumberingInternal.qll",
+    "csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/gvn/internal/ValueNumberingInternal.qll"
   ],
   "C++ IR ValueNumber": [
     "cpp/ql/src/semmle/code/cpp/ir/implementation/raw/gvn/ValueNumbering.qll",

cpp/ql/src/Likely Bugs/Arithmetic/UnsignedGEZero.qll

@@ -15,24 +15,33 @@ class ConstantZero extends Expr {
   }
 }
 
+/**
+ * Holds if `candidate` is an expression such that if it's unsigned then we
+ * want an alert at `ge`.
+ */
+private predicate lookForUnsignedAt(GEExpr ge, Expr candidate) {
+  // Base case: `candidate >= 0`
+  ge.getRightOperand() instanceof ConstantZero and
+  candidate = ge.getLeftOperand().getFullyConverted() and
+  // left operand was a signed or unsigned IntegralType before conversions
+  // (not a pointer, checking a pointer >= 0 is an entirely different mistake)
+  // (not an enum, as the fully converted type of an enum is compiler dependent
+  //  so checking an enum >= 0 is always reasonable)
+  ge.getLeftOperand().getUnderlyingType() instanceof IntegralType
+  or
+  // Recursive case: `...(largerType)candidate >= 0`
+  exists(Conversion conversion |
+    lookForUnsignedAt(ge, conversion) and
+    candidate = conversion.getExpr() and
+    conversion.getType().getSize() > candidate.getType().getSize()
+  )
+}
+
 class UnsignedGEZero extends GEExpr {
   UnsignedGEZero() {
-    this.getRightOperand() instanceof ConstantZero and
-    // left operand was a signed or unsigned IntegralType before conversions
-    // (not a pointer, checking a pointer >= 0 is an entirely different mistake)
-    // (not an enum, as the fully converted type of an enum is compiler dependent
-    //  so checking an enum >= 0 is always reasonable)
-    getLeftOperand().getUnderlyingType() instanceof IntegralType and
     exists(Expr ue |
-      // ue is some conversion of the left operand
-      ue = getLeftOperand().getConversion*() and
-      // ue is unsigned
-      ue.getUnderlyingType().(IntegralType).isUnsigned() and
-      // ue may be converted to zero or more strictly larger possibly signed types
-      // before it is fully converted
-      forall(Expr following | following = ue.getConversion+() |
-        following.getType().getSize() > ue.getType().getSize()
-      )
+      lookForUnsignedAt(this, ue) and
+      ue.getUnderlyingType().(IntegralType).isUnsigned()
     )
   }
 }

cpp/ql/src/experimental/README.md

@@ -0,0 +1 @@
+This directory contains [experimental](../../../../docs/experimental.md) CodeQL queries and libraries.

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

@@ -19,6 +19,8 @@ import semmle.code.cpp.exprs.Access
 class Field extends MemberVariable {
   Field() { fieldoffsets(underlyingElement(this), _, _) }
 
+  override string getCanonicalQLClass() { result = "Field" }
+
   /**
    * Gets the offset of this field in bytes from the start of its declaring
    * type (on the machine where facts were extracted).
@@ -84,6 +86,8 @@ class Field extends MemberVariable {
 class BitField extends Field {
   BitField() { bitfield(underlyingElement(this), _, _) }
 
+  override string getCanonicalQLClass() { result = "BitField" }
+
   /**
    * Gets the size of this bitfield in bits (on the machine where facts
    * were extracted).

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

@@ -28,6 +28,8 @@ private import semmle.code.cpp.internal.ResolveClass
  * can have multiple declarations.
  */
 class Variable extends Declaration, @variable {
+  override string getCanonicalQLClass() { result = "Variable" }
+
   /** Gets the initializer of this variable, if any. */
   Initializer getInitializer() { result.getDeclaration() = this }
 
@@ -351,6 +353,8 @@ class StackVariable extends LocalScopeVariable {
  * A local variable can be declared by a `DeclStmt` or a `ConditionDeclExpr`.
  */
 class LocalVariable extends LocalScopeVariable, @localvariable {
+  override string getCanonicalQLClass() { result = "LocalVariable" }
+
   override string getName() { localvariables(underlyingElement(this), _, result) }
 
   override Type getType() { localvariables(underlyingElement(this), unresolveElement(result), _) }
@@ -396,6 +400,8 @@ class NamespaceVariable extends GlobalOrNamespaceVariable {
   NamespaceVariable() {
     exists(Namespace n | namespacembrs(unresolveElement(n), underlyingElement(this)))
   }
+
+  override string getCanonicalQLClass() { result = "NamespaceVariable" }
 }
 
 /**
@@ -415,6 +421,8 @@ class NamespaceVariable extends GlobalOrNamespaceVariable {
  */
 class GlobalVariable extends GlobalOrNamespaceVariable {
   GlobalVariable() { not this instanceof NamespaceVariable }
+
+  override string getCanonicalQLClass() { result = "GlobalVariable" }
 }
 
 /**
@@ -434,6 +442,8 @@ class GlobalVariable extends GlobalOrNamespaceVariable {
 class MemberVariable extends Variable, @membervariable {
   MemberVariable() { this.isMember() }
 
+  override string getCanonicalQLClass() { result = "MemberVariable" }
+
   /** Holds if this member is private. */
   predicate isPrivate() { this.hasSpecifier("private") }