Merge remote-tracking branch 'upstream/rc/1.20' into mergeback-2019-03-19

Author: Max Schaefer

README.md

@@ -13,4 +13,4 @@ We welcome contributions to our standard library and standard checks. Do you hav
 
 ## License
 
-The LGTM queries are licensed under [Apache License 2.0](LICENSE) by [Semmle](https://semmle.com).
+The QL queries in this repository are licensed under [Apache License 2.0](LICENSE) by [Semmle](https://semmle.com).

change-notes/1.20/analysis-csharp.md

@@ -1,24 +1,21 @@
 # Improvements to C# analysis
 
-## General improvements
-
-## New queries
-
-| **Query**                   | **Tags**  | **Purpose**                                                        |
-|-----------------------------|-----------|--------------------------------------------------------------------|
-
 ## Changes to existing queries
 
-| *@name of query (Query ID)*  | *Impact on results*    | *How/why the query has changed*   |
+| **Query**                    | **Expected impact**    | **Change**                        |
 |------------------------------|------------------------|-----------------------------------|
-| Off-by-one comparison against container length (`cs/index-out-of-bounds`) | Fewer false positives | Results have been removed when there are additional guards on the index. |
-| Dereferenced variable is always null (`cs/dereferenced-value-is-always-null`) | Improved results | The query has been rewritten from scratch, and the analysis is now based on static single assignment (SSA) forms. The query is now enabled by default in LGTM. |
-| Dereferenced variable may be null (`cs/dereferenced-value-may-be-null`) | Improved results | The query has been rewritten from scratch, and the analysis is now based on static single assignment (SSA) forms. The query is now enabled by default in LGTM. |
-| SQL query built from user-controlled sources (`cs/sql-injection`), Improper control of generation of code (`cs/code-injection`), Uncontrolled format string (`cs/uncontrolled-format-string`), Clear text storage of sensitive information (`cs/cleartext-storage-of-sensitive-information`), Exposure of private information (`cs/exposure-of-sensitive-information`) | More results | Data sources have been added from user controls in `System.Windows.Forms`. |
-| Use of default ToString() (`cs/call-to-object-tostring`) | Fewer false positives | Results have been removed for `char` arrays passed to `StringBuilder.Append()`, which were incorrectly marked as using `ToString`. |
-| Use of default ToString() (`cs/call-to-object-tostring`) | Fewer results | Results have been removed when the object is an interface or an abstract class. |
-| Unused format argument (`cs/format-argument-unused`) | Fewer false positives | Results have been removed where the format string is empty. This is often used as a default value and is not an interesting result. |
-| Double-checked lock is not thread-safe (`cs/unsafe-double-checked-lock`) | Fewer false positives, more true positives | Results have been removed where the underlying field was not updated in the `lock` statement, or where the field is a `struct`. Results have been added where there are other statements inside the `lock` statement. |
+| Clear text storage of sensitive information (`cs/cleartext-storage-of-sensitive-information`) | More results | Now includes data sources for user controls in `System.Windows.Forms`. |
+| Dereferenced variable is always null (`cs/dereferenced-value-is-always-null`) | Improved results | The query has been rewritten from scratch, and the analysis is now based on static single assignment (SSA) forms. Results are now shown by default in LGTM. |
+| Dereferenced variable may be null (`cs/dereferenced-value-may-be-null`) | Improved results | The query has been rewritten from scratch, and the analysis is now based on static single assignment (SSA) forms. Results are now shown by default in LGTM. |
+| Double-checked lock is not thread-safe (`cs/unsafe-double-checked-lock`) | Fewer false positive and more true positive results | No longer highlights code where the underlying field was not updated in the `lock` statement, or where the field is a `struct`. Results have been added where there are other statements inside the `lock` statement. |
+| Exposure of private information (`cs/exposure-of-sensitive-information`) | More results | Now includes data sources for user controls in `System.Windows.Forms`. |
+| Improper control of generation of code (`cs/code-injection`) | More results | Now includes data sources for user controls in `System.Windows.Forms`. |
+| Off-by-one comparison against container length (`cs/index-out-of-bounds`) | Fewer false positive results | No longer reports results when there are additional guards on the index. |
+| SQL query built from user-controlled sources (`cs/sql-injection`) | More results | Now includes data sources for user controls in `System.Windows.Forms`. |
+| Uncontrolled format string (`cs/uncontrolled-format-string`) | More results | Now includes data sources for user controls in `System.Windows.Forms`. |
+| Unused format argument (`cs/format-argument-unused`) | Fewer false positive results | No longer reports results where the format string is empty. This is often used as a default value and is not an interesting result. |
+| Use of default ToString() (`cs/call-to-object-tostring`) | Fewer false positive results | No longer reports results for `char` arrays passed to `StringBuilder.Append()`, which were incorrectly marked as using `ToString`. |
+| Use of default ToString() (`cs/call-to-object-tostring`) | Fewer results | No longer reports results when the object is an interface or an abstract class. |
 | Using a package with a known vulnerability (`cs/use-of-vulnerable-package`) | More results | This query detects packages vulnerable to CVE-2019-0657. |
 
 ## Changes to code extraction
@@ -29,14 +26,13 @@
 ## Changes to QL libraries
 
 * The class `TrivialProperty` now includes library properties determined to be trivial using CIL analysis. This may increase the number of results for all queries that use data flow.
-* Taint-tracking steps have been added for the `Json.NET` package. This will improve results for queries that use taint-tracking.
+* Taint-tracking steps have been added for the `Json.NET` package. This will improve results for queries that use taint tracking.
 * Support has been added for EntityFrameworkCore, including
   - Stored data flow sources
   - Sinks for SQL expressions
-  - Data flow through fields that are mapped to the database.
+  - Data flow through fields that are mapped to the database
 * Support has been added for NHibernate-Core, including
   - Stored data flow sources
   - Sinks for SQL expressions
-  - Data flow through fields that are mapped to the database.  
+  - Data flow through fields that are mapped to the database 
 
-## Changes to the autobuilder

change-notes/1.20/analysis-java.md

@@ -1,8 +1,5 @@
 # Improvements to Java analysis
 
-## General improvements
-
-
 ## New queries
 
 | **Query**                   | **Tags**  | **Purpose**                                                        |
@@ -15,7 +12,6 @@
 | **Query**                  | **Expected impact**    | **Change**                                                       |
 |----------------------------|------------------------|------------------------------------------------------------------|
 | Arbitrary file write during archive extraction ("Zip Slip") (`java/zipslip`) | Fewer false positive results | Results involving a sanitization step that converts a destination `Path` to a `File` are no longer reported. |
-| Double-checked locking is not thread-safe (`java/unsafe-double-checked-locking`) | Fewer false positive results and more true positive results | Results that use safe publication through a `final` field are no longer reported. Results that initialize immutable types like `String` incorrectly are now reported. |
 | Result of multiplication cast to wider type (`java/integer-multiplication-cast-to-long`) | Fewer results | Results involving conversions to `float` or `double` are no longer reported, as they were almost exclusively false positives. |
 
 ## Changes to QL libraries
@@ -29,7 +25,7 @@
   collections, maps, and iterators. This affects all security queries, which
   can report more results based on such paths.
 * The `FlowSources` and `TaintTracking` libraries are extended to cover additional remote user
-  input and taint steps from the Apache Thrift, Apache Struts, Guice and Protobuf frameworks.
+  input and taint steps from the following frameworks: Guice, Protobuf, Thrift and Struts.
   This affects all security queries, which may yield additional results on projects
   that use these frameworks.
 

change-notes/1.20/analysis-javascript.md

@@ -2,7 +2,7 @@
 
 ## General improvements
 
-* Support for many frameworks and libraries has been improved, in particular including the following:
+* Support for many frameworks and libraries has been improved, in particular for:
   - [a-sync-waterfall](https://www.npmjs.com/package/a-sync-waterfall)
   - [Electron](https://electronjs.org)
   - [Express](https://npmjs.org/express)
@@ -12,7 +12,7 @@
   - [socket.io](http://socket.io)
   - [Vue](https://vuejs.org/)
 
-* File classification has been improved to recognize additional generated files, for example files from [HTML Tidy](html-tidy.org).
+* File classification now recognizes additional generated files, for example, files from [HTML Tidy](html-tidy.org).
 
 * The taint tracking library now recognizes flow through persistent storage, class fields, and callbacks in certain cases. Handling of regular expressions has also been improved. This may give more results for the security queries.
 
@@ -24,7 +24,7 @@
 
 | **Query**                                     | **Tags**                                             | **Purpose**                                                                                                                                                                 |
 |-----------------------------------------------|------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
-| Arbitrary file write during archive extraction ("Zip Slip") (`js/zipslip`) | security, external/cwe/cwe-022 | Identifies extraction routines that allow arbitrary file overwrite vulnerabilities. Results are hidden on LGTM by default. |
+| Arbitrary file write during archive extraction ("Zip Slip") (`js/zipslip`) | security, external/cwe/cwe-022 | Identifies extraction routines that allow arbitrary file overwrite vulnerabilities, indicating a possible violation of [CWE-022](https://cwe.mitre.org/data/definitions/22.html). Results are shown on LGTM by default. |
 | Arrow method on Vue instance (`js/vue/arrow-method-on-vue-instance`) | reliability, frameworks/vue | Highlights arrow functions that are used as methods on Vue instances. Results are shown on LGTM by default.|
 | Cross-window communication with unrestricted target origin (`js/cross-window-information-leak`) | security, external/cwe/201, external/cwe/359 | Highlights code that sends potentially sensitive information to another window without restricting the receiver window's origin, indicating a possible violation of [CWE-201](https://cwe.mitre.org/data/definitions/201.html). Results are shown on LGTM by default. |
 | Double escaping or unescaping (`js/double-escaping`) | correctness, security, external/cwe/cwe-116 | Highlights potential double escaping or unescaping of special characters, indicating a possible violation of [CWE-116](https://cwe.mitre.org/data/definitions/116.html). Results are shown on LGTM by default. |
@@ -39,24 +39,24 @@
 
 | **Query**                                  | **Expected impact**          | **Change**                                                                   |
 |--------------------------------------------|------------------------------|------------------------------------------------------------------------------|
-| Ambiguous HTML id attribute                | Fewer false-positive results | This rule now treats templates more conservatively. Its precision has been revised to 'high'. |
-| Assignment to exports variable             | Fewer results                | This rule no longer flags code that is also flagged by the rule "Useless assignment to local variable". |
-| Client-side cross-site scripting           | More true-positive results, fewer false-positive results. | This rule now recognizes WinJS functions that are vulnerable to HTML injection. It no longer flags certain safe uses of jQuery, and recognizes custom sanitizers. |
-| Hard-coded credentials                     | Fewer false-positive results | This rule no longer flag the empty string as a hardcoded username. |
-| Insecure randomness | More results | This rule now flags insecure uses of `crypto.pseudoRandomBytes`. |
-| Reflected cross-site scripting             | Fewer false-positive results. | This rule now recognizes custom sanitizers. |
-| Stored cross-site scripting                | Fewer false-positive results. | This rule now recognizes custom sanitizers. |
-| Unbound event handler receiver (`js/unbound-event-handler-receiver`) | Fewer false positive results | Additional ways that class methods can be bound are recognized. |
-| Uncontrolled data used in network request  | More results                 | This rule now recognizes host values that are vulnerable to injection. |
-| Unused parameter                           | Fewer false-positive results | This rule no longer flags parameters with leading underscore. |
-| Unused variable, import, function or class | Fewer false-positive results | This rule now flags fewer variables that are implictly used by JSX elements, no longer flags variables with leading underscore, and no longer flags variables in dead code. |
-| Uncontrolled data used in path expression | Fewer false-positive results | This rule now recognizes the Express `root` option, which prevents path traversal. |
-| Unneeded defensive code | More true-positive results, fewer false-positive results. | This rule now recognizes additional defensive code patterns. |
+| Ambiguous HTML id attribute                | Fewer false positive results | This query now treats templates more conservatively. Its precision has been revised to 'high'. |
+| Assignment to exports variable             | Fewer results                | This query no longer flags code that is also flagged by the query "Useless assignment to local variable". |
+| Client-side cross-site scripting           | More true positive and fewer false positive results. | This query now recognizes WinJS functions that are vulnerable to HTML injection. It no longer flags certain safe uses of jQuery, and recognizes custom sanitizers. |
+| Hard-coded credentials                     | Fewer false positive results | This query no longer flags the empty string as a hardcoded username. |
+| Insecure randomness | More results | This query now flags insecure uses of `crypto.pseudoRandomBytes`. |
+| Reflected cross-site scripting             | Fewer false positive results. | This query now recognizes custom sanitizers. |
+| Stored cross-site scripting                | Fewer false positive results. | This query now recognizes custom sanitizers. |
+| Unbound event handler receiver (`js/unbound-event-handler-receiver`) | Fewer false positive results | Additional ways that class methods can be bound are now recognized. |
+| Uncontrolled data used in network request  | More results                 | This query now recognizes host values that are vulnerable to injection. |
+| Uncontrolled data used in path expression | Fewer false positive results | This query now recognizes the Express `root` option, which prevents path traversal. |
+| Unneeded defensive code | More true positive and fewer false positive results | This query now recognizes additional defensive code patterns. |
+| Unsafe dynamic method access              | Fewer false positive results | This query no longer flags concatenated strings as unsafe method names. |
+| Unused parameter                           | Fewer false positive results | This query no longer flags parameters with leading underscore. |
+| Unused variable, import, function or class | Fewer false positive results | This query now flags fewer variables that are implictly used by JSX elements. It no longer flags variables with a leading underscore and variables in dead code. |
+| Unvalidated dynamic method call           | More true positive results | This query now flags concatenated strings as unvalidated method names in more cases. |
+| Useless assignment to property. | Fewer false positive results | This query now treats assignments with complex right-hand sides correctly. |
 | Useless conditional | Fewer results | Additional defensive coding patterns are now ignored. |
-| Useless assignment to property. | Fewer false-positive results | This rule now treats assignments with complex right-hand sides correctly. |
-| Unsafe dynamic method access              | Fewer false-positive results | This rule no longer flags concatenated strings as unsafe method names. |
-| Unvalidated dynamic method call           | More true-positive results | This rule now flags concatenated strings as unvalidated method names in more cases. |
-| Useless conditional | More true-positive results | This rule now flags additional uses of function call values. |
+| Useless conditional | More true positive results | This query now flags additional uses of function call values. |
 
 ## Changes to QL libraries
 

change-notes/1.20/analysis-python.md

@@ -11,6 +11,7 @@ Removes false positives seen when using Python 3.6, but not when using earlier v
 The API has been improved to declutter the global namespace and improve discoverability and readability.
  * New predicates `ModuleObject::named(name)` and `ModuleObject.attr(name)` have been added, allowing more readable access to common objects. For example, `(any ModuleObject m | m.getName() = "sys").getAttribute("exit")` can be replaced with `ModuleObject::named("sys").attr("exit")`
  * The API for accessing builtin functions has been improved. Predicates of the form `theXXXFunction()`, such as `theLenFunction()`, have been deprecated in favour of `Object::builtin(name)`.
+ * A configuration based API has been added for writing data flow and taint tracking queries. This is provided as a convenience for query authors who have written data flow or taint tracking queries for other languages, so they can use a similar format of query across multiple languages.
 
  ## New queries
 
@@ -30,6 +31,7 @@ The API has been improved to declutter the global namespace and improve discover
 | Comparison using is when operands support \_\_eq\_\_ (`py/comparison-using-is`) | Fewer false positive results | Results where one of the objects being compared is an enum member are no longer reported. |
 | Modification of parameter with default (`py/modification-of-default-value`) | More true positive results | Instances where the mutable default value is mutated inside other functions are now also reported. |
 | Mutation of descriptor in \_\_get\_\_ or \_\_set\_\_ method (`py/mutable-descriptor`) | Fewer false positive results | Results where the mutation does not occur when calling one of the  `__get__`, `__set__` or `__delete__` methods are no longer reported. |
+| Redundant comparison (`py/redundant-comparison`) | Fewer false positive results | Results in chained comparisons are no longer reported. |
 | Unused import (`py/unused-import`) | Fewer false positive results | Results where the imported module is used in a `doctest` string are no longer reported. |
 | Unused import (`py/unused-import`) | Fewer false positive results | Results where the imported module is used in a type-hint comment are no longer reported. |
 

change-notes/1.20/support/framework-support.rst

@@ -1,7 +1,7 @@
 Frameworks and libraries
 ########################
 
-The QL libraries and queries in this version have been explicitly checked against the libraries and frameworks listed below.
+The QL libraries and queries in version |version| have been explicitly checked against the libraries and frameworks listed below.
 
 .. pull-quote::