Merge pull request #588 from adityasharad/merge/1.19-next-301118

Merge rc/1.19 into next.

Author: aibaars

change-notes/1.19/analysis-cpp.md

@@ -23,8 +23,12 @@
 | Expression has no effect | Fewer false positive results | Expressions in template instantiations are now excluded from this query. |
 | Global could be static | Fewer false positive results | Variables with declarations in header files are now excluded from this query. |
 | Resource not released in destructor | Fewer false positive results | Placement new is now excluded from the query. Also fixed an issue where false positives could occur if the destructor body was not in the snapshot. |
+| Memory is never freed | Fewer false positive results | This query now accounts for C++ _placement new_, which returns a pointer that does not need to be freed. |
 | Missing return statement (`cpp/missing-return`) | Visible by default | The precision of this query has been increased from 'medium' to 'high', which makes it visible by default in LGTM. It was 'medium' in release 1.17 and 1.18 because it had false positives due to an extractor bug that was fixed in 1.18. |
 | Missing return statement | Fewer false positive results | The query is now produces correct results when a function returns a template-dependent type, or makes a non-returning call to another function. |
+| Multiplication result converted to larger type (`cpp/integer-multiplication-cast-to-long`) | Fewer false positive results | Char-typed numbers are no longer considered to be potentially large. |
+| Non-virtual destructor in base class (`cpp/virtual-destructor`) | Fewer false positive results | This query was copied from file `AV Rule 78.ql` to `NonVirtualDestructorInBaseClass.ql` and renamed from "No virtual destructor" to "Non-virtual destructor in base class". The new version ignores base classes with non-public destructors since we consider those to be adequately protected. The new version retains the query id `cpp/virtual-destructor` and is displayed by default on LGTM, while `AV Rule 78.ql` is not run on LGTM. |
+| Overloaded assignment does not return 'this' (`cpp/assignment-does-not-return-this`) | Fewer false positive results | This query now ignores any return statements that are unreachable. |
 | Static array access may cause overflow | More correct results | Data flow to the size argument of a buffer operation is now checked in this query. |
 | Call to memory access function may overflow buffer | More correct results | Array indexing with a negative index is now detected by this query. |
 | Self comparison | Fewer false positive results | Code inside macro invocations is now excluded from the query. |
@@ -39,6 +43,8 @@
 
 ## Changes to QL libraries
 
-* Added a hash consing library for structural comparison of expressions.
-* `getBufferSize` now detects variable size structs more reliably.
-* Buffer.qll now treats arrays of zero size as a special case.
+* Added a hash consing library (`semmle.code.cpp.valuenumbering.HashCons`) for structural comparison of expressions. Unlike the existing library for global value numbering, this library implements a pure syntactic comparison of expressions and will equate expressions even if they may not compute the same value.
+* The `Buffer.qll` library has more conservative treatment of arrays embedded in structs. This reduces false positives in a number of security queries, especially `cpp/overflow-buffer`.
+    * Pre-C99 encodings of _flexible array members_ are recognized more reliably.
+    * Arrays of zero size are now treated as a special case.
+* The library `semmle.code.cpp.dataflow.RecursionPrevention` is now deprecated. It was an aid for transitioning data-flow queries from 1.16 to 1.17, and it no longer has any function. Imports of this library should simply be deleted.

change-notes/1.19/analysis-javascript.md

@@ -2,7 +2,7 @@
 
 ## General improvements
 
-* Modelling of taint flow through array operations has been improved. This may give additional results for the security queries.
+* Modeling of taint flow through array and buffer operations has been improved. This may give additional results for the security queries.
 
 * Support for AMD modules has been improved. This may give additional results for the security queries as well as any queries that use type inference on code bases that use such modules.
 
@@ -23,12 +23,14 @@
 |-----------------------------------------------|------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
 | Enabling Node.js integration for Electron web content renderers (`js/enabling-electron-renderer-node-integration`) | security, frameworks/electron, external/cwe/cwe-094  | Highlights Electron web content renderer preferences with Node.js integration enabled, indicating a violation of [CWE-94](https://cwe.mitre.org/data/definitions/94.html). Results are not shown on LGTM by default. |
 | File data in outbound network request | security, external/cwe/cwe-200 | Highlights locations where file data is sent in a network request. Results are not shown on LGTM by default. |
+| Hard-coded data interpreted as code | security, external/cwe/cwe-506 | Highlights locations where hard-coded data is transformed and then executed as code or interpreted as an import path, which may indicate embedded malicious code ([CWE-506](https://cwe.mitre.org/data/definitions/506.html)). Results are shown on LGTM by default. |
 | Host header poisoning in email generation |  security, external/cwe/cwe-640 | Highlights code that generates emails with links that can be hijacked by HTTP host header poisoning, indicating a violation of [CWE-640](https://cwe.mitre.org/data/definitions/640.html). Results shown on LGTM by default.  |
-| Unsafe dynamic method access (`js/unsafe-dynamic-method-access` ) | security, external/cwe/cwe-094 | Highlights code that invokes a user-controlled method on an object with unsafe methods. Results are shown on LGTM by default. |
 | Replacement of a substring with itself (`js/identity-replacement`) | correctness, security, external/cwe/cwe-116 | Highlights string replacements that replace a string with itself, which usually indicates a mistake. Results shown on LGTM by default. |
 | Stored cross-site scripting (`js/stored-xss`) | security, external/cwe/cwe-079, external/cwe/cwe-116 | Highlights uncontrolled stored values flowing into HTML content, indicating a violation of [CWE-079](https://cwe.mitre.org/data/definitions/79.html). Results shown on LGTM by default. |
 | Unclear precedence of nested operators (`js/unclear-operator-precedence`) | maintainability, correctness, external/cwe/cwe-783 | Highlights nested binary operators whose relative precedence is easy to misunderstand. Results shown on LGTM by default. |
 | Unneeded defensive code | correctness, external/cwe/cwe-570, external/cwe/cwe-571 | Highlights locations where defensive code is not needed. Results are shown on LGTM by default. |
+| Unsafe dynamic method access (`js/unsafe-dynamic-method-access` ) | security, external/cwe/cwe-094 | Highlights code that invokes a user-controlled method on an object with unsafe methods. Results are shown on LGTM by default. |
+| Unvalidated dynamic method access (`js/unvalidated-dynamic-method-call` ) | security, external/cwe/cwe-754 | Highlights code that invokes a user-controlled method without guarding against exceptional circumstances. Results are shown on LGTM by default. |
 | Useless assignment to property | maintainability | Highlights property assignments whose value is always overwritten. Results are shown on LGTM by default. |
 | User-controlled data in file | security, external/cwe/cwe-912 | Highlights locations where user-controlled data is written to a file. Results are not shown on LGTM by default. |
 
@@ -43,12 +45,13 @@
 | Conflicting HTML element attributes | Lower severity | The severity of this rule has been revised to "warning". |
 | Duplicate 'if' condition | Lower severity | The severity of this rule has been revised to "warning". |
 | Duplicate switch case | Lower severity | The severity of this rule has been revised to "warning". |
+| Inconsistent use of 'new' | Simpler result presentation | This rule now only shows one call with `new` and one without. |
 | Information exposure through a stack trace | More results | This rule now also flags cases where the entire exception object (including the stack trace) may be exposed. |
-| Missing CSRF middleware | Fewer false-positive results | This rule now recognizes additional CSRF protection middlewares. |
 | Missing 'this' qualifier | Fewer false-positive results | This rule now recognizes additional intentional calls to global functions. | 
+| Missing CSRF middleware | Fewer false-positive results | This rule now recognizes additional CSRF protection middlewares. |
 | Missing variable declaration | Lower severity | The severity of this rule has been revised to "warning". |
 | Regular expression injection | Fewer false-positive results | This rule now identifies calls to `String.prototype.search` with more precision. |
-| Remote property injection | Fewer results | The precision of this rule has been revised to "medium". Results are no longer shown on LGTM by default. |
+| Remote property injection | Fewer results | The precision of this rule has been revised to "medium". Furthermore, it no longer flags dynamic method calls, which are now handled by two new queries. Results are no longer shown on LGTM by default. |
 | Self assignment | Fewer false-positive results | This rule now ignores self-assignments preceded by a JSDoc comment with a `@type` tag. |
 | Server-side URL redirect | Fewer false-positive results | This rule now recognizes safe redirects in more cases. |
 | Server-side URL redirect | More results | This rule now recognizes redirection calls in more cases. |
@@ -59,6 +62,7 @@
 | Unused variable, import, function or class | Fewer false-positive results | This rule now flags fewer variables that may be used by `eval` calls. |
 | Unused variable, import, function or class | Fewer results | This rule now flags import statements with multiple unused imports once. |
 | Useless assignment to local variable | Fewer false-positive results | This rule now recognizes additional ways default values can be set. |
+| Useless conditional | More results, fewer false-positive results | This rule now recognizes conditionals in more cases, but no longer flags certain defensive coding patterns. |
 | Whitespace contradicts operator precedence | Fewer false-positive results | This rule no longer flags operators with asymmetric whitespace. |
 | Wrong use of 'this' for static method | More results, fewer false-positive results | This rule now recognizes inherited methods. |
 

change-notes/1.19/analysis-python.md

@@ -1,14 +1,12 @@
 # Improvements to Python analysis
 
-
 ## General improvements
 
-> Changes that affect alerts in many files or from many queries
-> For example, changes to file classification
-
 ### Representation of the control flow graph
 
-The representation of the control flow graph (CFG) has been modified to better reflect the semantics of Python.
+The representation of the control flow graph (CFG) has been modified to better reflect the semantics of Python. As part of these changes, a new predicate `Stmt.getAnEntryNode()` has been added to make it easier to write reachability queries involving statements.
+
+#### CFG nodes removed
 
 The following statement types no longer have a CFG node for the statement itself, as their sub-expressions already contain all the
 semantically significant information:
@@ -20,76 +18,67 @@ semantically significant information:
 
 For example, the CFG for `if cond: foo else bar` now starts with the CFG node for `cond`.
 
-For the following statement types, the CFG node for the statement now follows the CFG nodes of its sub-expressions to better reflect the semantics:
+#### CFG nodes reordered
+
+For the following statement types, the CFG node for the statement now follows the CFG nodes of its sub-expressions to follow Python semantics:
 
 * `Print`
 * `TemplateWrite`
 * `ImportStar`
 
-For example the CFG for `print foo` (in Python 2) has changed from `print -> foo` to `foo -> print`, better reflecting the runtime behavior.
-
+For example the CFG for `print foo` (in Python 2) has changed from `print -> foo` to `foo -> print`, to reflect the runtime behavior.
 
 The CFG for the `with` statement has been re-ordered to more closely reflect the semantics.
 For the `with` statement:
 ```python
 with cm as var:
     body
 ```
-The order of the CFG changes from:
-
-    <with>
-    cm
-    var
-    body
-
-to:
-
-    cm
-    <with>
-    var
-    body
-
-A new predicate `Stmt.getAnEntryNode()` has been added to make it easier to write reachability queries involving statements.
 
+* Previous CFG node order: `<with>` -> `cm` -> `var` -> `body`
+* New CFG node order: `cm` -> `<with>` -> `var` -> `body`
 
 ## New queries
 
 | **Query**                   | **Tags**  | **Purpose**                                                        |
 |-----------------------------|-----------|--------------------------------------------------------------------|
-| Information exposure through an exception (`py/stack-trace-exposure`) | security, external/cwe/cwe-209, external/cwe/cwe-497 | Finds instances where information about an exception may be leaked to an external user. Enabled on LGTM by default. |
+| Assert statement tests the truth value of a literal constant (`py/assert-literal-constant`) | reliability, correctness     | Checks whether an assert statement is testing the truth of a literal constant value. Results are hidden on LGTM by default. |
+| Flask app is run in debug mode (`py/flask-debug`) | security, external/cwe/cwe-215, external/cwe/cwe-489 | Finds instances where a Flask application is run in debug mode. Results are shown on LGTM by default. |
+| Information exposure through an exception (`py/stack-trace-exposure`) | security, external/cwe/cwe-209, external/cwe/cwe-497 | Finds instances where information about an exception may be leaked to an external user. Results are shown on LGTM by default. |
+| Jinja2 templating with autoescape=False (`py/jinja2/autoescape-false`) | security, external/cwe/cwe-079 | Finds instantiations of `jinja2.Environment` with `autoescape=False` which may allow XSS attacks. Results are hidden on LGTM by default. |
+| Request without certificate validation (`py/request-without-cert-validation`) | security, external/cwe/cwe-295 | Finds requests where certificate verification has been explicitly turned off, possibly allowing man-in-the-middle attacks. Results are hidden on LGTM by default. |
+| Use of weak cryptographic key (`py/weak-crypto-key`) | security, external/cwe/cwe-326 | Finds creation of weak cryptographic keys. Results are shown on LGTM by default. |
 
 ## Changes to existing queries
 
 All taint-tracking queries now support visualization of paths in QL for Eclipse.
-Most security alerts are now visible on LGTM by default.
+Most security alerts are now visible on LGTM by default. This means that you may see results that were previously hidden for the following queries: 
+
+* Code injection (`py/code-injection`)
+* Reflected server-side cross-site scripting (`py/reflective-xss`)
+* SQL query built from user-controlled sources (`py/sql-injection`)
+* Uncontrolled data used in path expression (`py/path-injection`)
+* Uncontrolled command line (`py/command-line-injection`)
 
 | **Query**                  | **Expected impact**    | **Change**                                                       |
 |----------------------------|------------------------|------------------------------------------------------------------|
-| Assert statement tests the truth value of a literal constant (`py/assert-literal-constant`) | reliability, correctness     | Checks whether an assert statement is testing the truth of a literal constant value. Not shown by default. |
-| Code injection (`py/code-injection`) | Supports path visualization and is now visible on LGTM by default | No change to expected results |
-| Command injection (`py/command-line-injection`) | Additional sinks in the `os`, and `popen` modules | Possibility of new results |
-| Deserializing untrusted input (`py/unsafe-deserialization`) | Supports path visualization | No change to expected results |
-| Encoding error (`py/encoding-error`) | Better alert location | Alert is now shown at the position of the first offending character, rather than at the top of the file. |
+| Command injection (`py/command-line-injection`) | More results | Additional sinks in the `os`, and `popen` modules may find more results in some projects. |
+| Encoding error (`py/encoding-error`) | Better alert location | Alerts are now shown at the start of the encoding error, rather than at the top of the file. |
 | Missing call to \_\_init\_\_ during object initialization (`py/missing-call-to-init`) | Fewer false positive results | Results where it is likely that the full call chain has not been analyzed are no longer reported. |
-| Reflected server-side cross-site scripting (`py/reflective-xss`) | Supports path visualization and is now visible on LGTM by default | No change to expected results |
-| SQL query built from user-controlled sources (`py/sql-injection`) | Supports path visualization and is now visible on LGTM by default | No change to expected results |
-| Uncontrolled data used in path expression (`py/path-injection`) | Supports path visualization and is now visible on LGTM by default | No change to expected results |
-| Uncontrolled command line (`py/command-line-injection`) | Supports path visualization and is now visible on LGTM by default | No change to expected results |
-| URL redirection from remote source (`py/url-redirection`) | Fewer false positive results and now supports path visualization | Taint is no longer tracked from the right hand side of binary expressions. In other words `SAFE + TAINTED` is now treated as safe. |
+| URL redirection from remote source (`py/url-redirection`) | Fewer false positive results | Taint is no longer tracked from the right-hand side of binary expressions. In other words `SAFE + TAINTED` is now treated as safe. |
 
 
 ## Changes to code extraction
 
 * Improved scalability: Scaling is near linear to at least 20 CPU cores.
-* Five levels of logging can be selected: `ERROR`, `WARN`, `INFO`, `DEBUG` and `TRACE`. `WARN` is the stand-alone default, but `INFO` will be used when run by LGTM.
+* Five levels of logging can be selected: `ERROR`, `WARN`, `INFO`, `DEBUG` and `TRACE`. LGTM uses `INFO` level logging. QL tools use `WARN` level logging by default.
 * The `-v` flag can be specified multiple times to increase logging level by one per `-v`.
 * The `-q` flag has been added and can be specified multiple times to reduce the logging level by one per `-q`.
 * Log lines are now in the `[SEVERITY] message` style and never overlap.
-* Extractor now outputs the location of the first offending character when an EncodingError is encountered.
+* The extractor now outputs the location of the first character that triggers an EncodingError.
 
 ## Changes to QL libraries
 
-* Taint tracking analysis now understands HTTP requests in the `twisted` library.
-
+* Taint-tracking analysis now understands HTTP requests in the `twisted` library.
 * The analysis now handles `isinstance` and `issubclass` tests involving the basic abstract base classes better. For example, the test `issubclass(list, collections.Sequence)` is now understood to be `True`
 * Taint tracking automatically tracks tainted mappings and collections, without you having to add additional taint kinds. This means that custom taints are tracked from `x` to `y` in the following flow: `l = [x]; y =l[0]`.