Merge branch 'master' into python-add-points-to-for-missing-builtin-return-types

Author: tausbn

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-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
 

change-notes/1.24/analysis-javascript.md

@@ -2,6 +2,8 @@
 
 ## 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,
@@ -13,6 +15,12 @@
 
 * 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)
@@ -26,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)
@@ -37,8 +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
 
@@ -51,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
 

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") }
 

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

@@ -1,7 +1,5 @@
 private import cpp
 
-Function viableImpl(Call call) { result = viableCallable(call) }
-
 /**
  * Gets a function that might be called by `call`.
  */