Merge pull request #6 from Semmle/master

Merge

Author: raulgarciamsft

.codeqlmanifest.json

@@ -0,0 +1,4 @@
+{ "provide": [ "*/ql/src/qlpack.yml",
+               "misc/legacy-support/*/qlpack.yml",
+               "misc/suite-helpers/qlpack.yml",
+               "codeql/.codeqlmanifest.json" ] }

.github/ISSUE_TEMPLATE/ql---general.md

@@ -0,0 +1,14 @@
+---
+name: General issue
+about: Tell us if you think something is wrong or if you have a question
+title: General issue
+labels: question
+assignees: ''
+
+---
+
+**Description of the issue**
+
+<!-- Please explain briefly what is the problem.
+If it is about an LGTM project, please include its URL.-->
+

.gitignore

@@ -12,3 +12,6 @@
 # Visual studio temporaries, except a file used by QL4VS
 .vs/*
 !.vs/VSWorkspaceSettings.json
+
+# It's useful (though not required) to be able to unpack codeql in the ql checkout itself
+/codeql/

CODEOWNERS

@@ -2,9 +2,9 @@
 /java/ @Semmle/java
 /javascript/ @Semmle/js
 /cpp/ @Semmle/cpp-analysis
-/cpp/**/*.qhelp @semmledocs-ac
+/cpp/**/*.qhelp @hubwriter
 /csharp/**/*.qhelp @jf205
-/java/**/*.qhelp @felicity-semmle
-/javascript/**/*.qhelp @mc-semmle
-/python/**/*.qhelp @felicity-semmle
-/docs/language/ @felicity-semmle @jf205
+/java/**/*.qhelp @felicitymay
+/javascript/**/*.qhelp @mchammer01
+/python/**/*.qhelp @felicitymay
+/docs/language/ @shati-patel @jf205

change-notes/1.22/analysis-javascript.md

@@ -36,6 +36,7 @@
 | Shift out of range (`js/shift-out-of-range`| Fewer false positive results | This rule now correctly handles BigInt shift operands. |
 | Superfluous trailing arguments (`js/superfluous-trailing-arguments`) | Fewer false-positive results. | This rule no longer flags calls to placeholder functions that trivially throw an exception. |
 | Undocumented parameter (`js/jsdoc/missing-parameter`) | No changes to results | This rule is now run on LGTM, although its results are still not shown by default. |
+| Missing space in string concatenation (`js/missing-space-in-concatenation`) | Fewer false positive results | The rule now requires a word-like part exists in the string concatenation. | 
 
 ## Changes to QL libraries
 

change-notes/1.23/analysis-cpp.md

@@ -18,6 +18,10 @@ The following changes in version 1.23 affect C/C++ analysis in all applications.
 | Hard-coded Japanese era start date in call (`cpp/japanese-era/constructor-or-method-with-exact-era-date`) | Deprecated | This query has been deprecated.  Use the new combined query Hard-coded Japanese era start date (`cpp/japanese-era/exact-era-date`) instead. |
 | Hard-coded Japanese era start date in struct (`cpp/japanese-era/struct-with-exact-era-date`) | Deprecated | This query has been deprecated.  Use the new combined query Hard-coded Japanese era start date (`cpp/japanese-era/exact-era-date`) instead. |
 | Hard-coded Japanese era start date (`cpp/japanese-era/exact-era-date`) | More correct results | This query now checks for the beginning date of the Reiwa era (1st May 2019). |
+| Sign check of bitwise operation (`cpp/bitwise-sign-check`) | Fewer false positive results | Results involving `>=` or `<=` are no longer reported. |
+| Too few arguments to formatting function (`cpp/wrong-number-format-arguments`) | Fewer false positive results | Fixed false positives resulting from mistmatching declarations of a formatting function. |
+| Too many arguments to formatting function (`cpp/too-many-format-arguments`) | Fewer false positive results | Fixed false positives resulting from mistmatching declarations of a formatting function. |
+| Unclear comparison precedence (`cpp/comparison-precedence`) | Fewer false positive results | False positives involving template classes and functions have been fixed. |
 
 ## Changes to QL libraries
 
@@ -28,10 +32,20 @@ The following changes in version 1.23 affect C/C++ analysis in all applications.
   picture of the partial flow paths from a given source. The feature is
   disabled by default and can be enabled for individual configurations by
   overriding `int explorationLimit()`.
+* The data-flow library now supports flow out of C++ reference parameters.
+* The data-flow library now allows flow through the address-of operator (`&`).
 * The `DataFlow::DefinitionByReferenceNode` class now considers `f(x)` to be a
   definition of `x` when `x` is a variable of pointer type. It no longer
   considers deep paths such as `f(&x.myField)` to be definitions of `x`. These
   changes are in line with the user expectations we've observed.
 * There is now a `DataFlow::localExprFlow` predicate and a
   `TaintTracking::localExprTaint` predicate to make it easy to use the most
   common case of local data flow and taint: from one `Expr` to another.
+* The member predicates of the `FunctionInput` and `FunctionOutput` classes have been renamed for
+  clarity (e.g. `isOutReturnPointer()` to `isReturnValueDeref()`). The existing member predicates
+  have been deprecated, and will be removed in a future release. Code that uses the old member
+  predicates should be updated to use the corresponding new member predicate.
+* The control-flow graph is now computed in QL, not in the extractor. This can
+  lead to regressions (or improvements) in how queries are optimized because
+  optimization in QL relies on static size estimates, and the control-flow edge
+  relations will now have different size estimates than before.

change-notes/1.23/analysis-csharp.md

@@ -15,6 +15,8 @@ The following changes in version 1.23 affect C# analysis in all applications.
 
 | **Query**                    | **Expected impact**    | **Change**                        |
 |------------------------------|------------------------|-----------------------------------|
+| Dereferenced variable may be null (`cs/dereferenced-value-may-be-null`) | Fewer false positive results | More `null` checks are now taken into account, including `null` checks for `dynamic` expressions and `null` checks such as `object alwaysNull = null; if (x != alwaysNull) ...`. |
+| Missing Dispose call on local IDisposable (`cs/local-not-disposed`) | Fewer false positive results | The query has been rewritten in order to identify more dispose patterns. For example, a local `IDisposable` that is disposed of by passing through a fluent API is no longer reported. |
 
 ## Removal of old queries
 
@@ -37,5 +39,9 @@ The following changes in version 1.23 affect C# analysis in all applications.
   disabled by default and can be enabled for individual configurations by
   overriding `int explorationLimit()`.
 * `foreach` statements where the body is guaranteed to be executed at least once, such as `foreach (var x in new string[]{ "a", "b", "c" }) { ... }`, are now recognized by all analyses based on the control flow graph (such as SSA, data flow and taint tracking).
+* Fixed the control flow graph for `switch` statements where the `default` case was not the last case. This had caused the remaining cases to be unreachable. `SwitchStmt.getCase(int i)` now puts the `default` case last.
+* There is now a `DataFlow::localExprFlow` predicate and a
+  `TaintTracking::localExprTaint` predicate to make it easy to use the most
+  common case of local data flow and taint: from one `Expr` to another.
 
 ## Changes to autobuilder

change-notes/1.23/analysis-java.md

@@ -2,13 +2,22 @@
 
 The following changes in version 1.23 affect Java analysis in all applications.
 
+## New queries
+
+| **Query**                   | **Tags**  | **Purpose**                                                        |
+|-----------------------------|-----------|--------------------------------------------------------------------|
+| Continue statement that does not continue (`java/continue-in-false-loop`) | correctness | Finds `continue` statements in `do { ... } while (false)` loops. |
+
 ## Changes to existing queries
 
 | **Query**                    | **Expected impact**    | **Change**                        |
 |------------------------------|------------------------|-----------------------------------|
+| Dereferenced variable may be null (`java/dereferenced-value-may-be-null`) | Fewer false positives | Certain indirect null guards involving two auxiliary variables known to be equal can now be detected. |
+| Non-synchronized override of synchronized method (`java/non-sync-override`) | Fewer false positives | Results are now only reported if the immediately overridden method is synchronized. |
 | Query built from user-controlled sources (`java/sql-injection`) | More results | The query now identifies arguments to `Statement.executeLargeUpdate` and `Connection.prepareCall` as SQL expressions sinks. |
 | Query built from local-user-controlled sources (`java/sql-injection-local`) | More results | The query now identifies arguments to `Statement.executeLargeUpdate` and `Connection.prepareCall` as SQL expressions sinks. |
 | Query built without neutralizing special characters (`java/concatenated-sql-query`) | More results | The query now identifies arguments to `Statement.executeLargeUpdate` and `Connection.prepareCall` as SQL expressions sinks. |
+| Useless comparison test (`java/constant-comparison`) | Fewer false positives | Additional overflow check patterns are now recognized and no longer reported. |
 
 ## Changes to QL libraries
 

change-notes/1.23/analysis-javascript.md

@@ -2,34 +2,66 @@
 
 ## General improvements
 
+* Suppor for `globalThis` has been added.
+
 * Support for the following frameworks and libraries has been improved:
   - [firebase](https://www.npmjs.com/package/firebase)
   - [mongodb](https://www.npmjs.com/package/mongodb)
   - [mongoose](https://www.npmjs.com/package/mongoose)
+  - [rate-limiter-flexible](https://www.npmjs.com/package/rate-limiter-flexible)
 
 * The call graph has been improved to resolve method calls in more cases. This may produce more security alerts.
 
+* TypeScript 3.6 features are supported.
+
+
 ## New queries
 
 | **Query**                                                                 | **Tags**                                                          | **Purpose**                                                                                                                                                                            |
 |---------------------------------------------------------------------------|-------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
-| Unused index variable (`js/unused-index-variable`)                        | correctness                                                       | Highlights loops that iterate over an array, but do not use the index variable to access array elements, indicating a possible typo or logic error. |
-
+| Unused index variable (`js/unused-index-variable`)                        | correctness                                                       | Highlights loops that iterate over an array, but do not use the index variable to access array elements, indicating a possible typo or logic error. Results are shown on LGTM by default. |
+| Loop bound injection (`js/loop-bound-injection`)                          | security, external/cwe/cwe-834                                      | Highlights loops where a user-controlled object with an arbitrary .length value can trick the server to loop indefinitely. Results are shown on LGTM by default. |
+| Suspicious method name (`js/suspicious-method-name-declaration`)          | correctness, typescript, methods                                  | Highlights suspiciously named methods where the developer likely meant to write a constructor or function. Results are shown on LGTM by default. |
+| Shell command built from environment values (`js/shell-command-injection-from-environment`) | correctness, security, external/cwe/cwe-078, external/cwe/cwe-088 | Highlights shell commands that may change behavior inadvertently depending on the execution environment, indicating a possible violation of [CWE-78](https://cwe.mitre.org/data/definitions/78.html). Results are shown on LGTM by default.|
+| Use of returnless function (`js/use-of-returnless-function`)              | maintainability, correctness                                      | Highlights calls where the return value is used, but the callee never returns a value. Results are shown on LGTM by default. |
+| Useless regular expression character escape (`js/useless-regexp-character-escape`) | correctness, security, external/cwe/cwe-20 | Highlights regular expression strings with useless character escapes, indicating a possible violation of [CWE-20](https://cwe.mitre.org/data/definitions/20.html). Results are shown on LGTM by default. |
+| Unreachable method overloads (`js/unreachable-method-overloads`)          | correctness, typescript                                           | Highlights method overloads that are impossible to use from client code. Results are shown on LGTM by default. |
 
 ## Changes to existing queries
 
 | **Query**                      | **Expected impact**          | **Change**                                                                |
 |--------------------------------|------------------------------|---------------------------------------------------------------------------|
 | Incomplete string escaping or encoding (`js/incomplete-sanitization`) | Fewer false-positive results | This rule now recognizes additional ways delimiters can be stripped away. |
-| Client-side cross-site scripting (`js/xss`) | More results | More potential vulnerabilities involving functions that manipulate DOM attributes are now recognized. |
+| Client-side cross-site scripting (`js/xss`) | More results, fewer false-positive results | More potential vulnerabilities involving functions that manipulate DOM attributes are now recognized, and more sanitizers are detected. |
 | Code injection (`js/code-injection`) | More results | More potential vulnerabilities involving functions that manipulate DOM event handler attributes are now recognized. |
 | Hard-coded credentials (`js/hardcoded-credentials`) | Fewer false-positive results | This rule now flags fewer password examples. |
-| Incorrect suffix check (`js/incorrect-suffix-check`) | Fewer false-positive results | The query recognizes valid checks in more cases. 
+| Illegal invocation (`js/illegal-invocation`) | Fewer false-positive results | This rule now correctly handles methods named `call` and `apply`. |
+| Incorrect suffix check (`js/incorrect-suffix-check`) | Fewer false-positive results | The query recognizes valid checks in more cases. |
 | Network data written to file (`js/http-to-file-access`) | Fewer false-positive results | This query has been renamed to better match its intended purpose, and now only considers network data untrusted. | 
 | Password in configuration file (`js/password-in-configuration-file`) | Fewer false-positive results | This rule now flags fewer password examples. |
 | Prototype pollution (`js/prototype-pollution`) | More results | The query now highlights vulnerable uses of jQuery and Angular, and the results are shown on LGTM by default. |
+| Reflected cross-site scripting (`js/reflected-xss`) | Fewer false-positive results | The query now recognizes more sanitizers. |
+| Stored cross-site scripting (`js/stored-xss`) | Fewer false-positive results | The query now recognizes more sanitizers. |
 | Uncontrolled command line (`js/command-line-injection`) | More results | This query now treats responses from servers as untrusted. |
 
 ## Changes to QL libraries
 
 * `Expr.getDocumentation()` now handles chain assignments.
+
+## Removal of deprecated queries
+
+The following queries (deprecated since 1.17) are no longer available in the distribution:
+
+* Builtin redefined (js/builtin-redefinition)
+* Inefficient method definition (js/method-definition-in-constructor)
+* Bad parity check (js/incomplete-parity-check)
+* Potentially misspelled property or variable name (js/wrong-capitalization)
+* Unknown JSDoc tag (js/jsdoc/unknown-tag-type)
+* Invalid JSLint directive (js/jslint/invalid-directive)
+* Malformed JSLint directive (js/jslint/malformed-directive)
+* Use of HTML comments (js/html-comment)
+* Multi-line string literal (js/multi-line-string)
+* Octal literal (js/octal-literal)
+* Reserved word used as variable name (js/use-of-reserved-word)
+* Trailing comma in array or object expressions (js/trailing-comma-in-array-or-object)
+* Call to parseInt without radix (js/parseint-without-radix)

change-notes/1.23/analysis-python.md

@@ -11,4 +11,12 @@
 |-----------|----------|-------------|
 | Clear-text logging of sensitive information (`py/clear-text-logging-sensitive-data`) | security, external/cwe/cwe-312 | Finds instances where sensitive information is logged without encryption or hashing. Results are shown on LGTM by default. |
 | Clear-text storage of sensitive information (`py/clear-text-storage-sensitive-data`) | security, external/cwe/cwe-312 | Finds instances where sensitive information is stored without encryption or hashing. Results are shown on LGTM by default. |
+| Binding a socket to all network interfaces (`py/bind-socket-all-network-interfaces`) | security | Finds instances where a socket is bound to all network interfaces. Results are shown on LGTM by default. |
 
+
+## Changes to existing queries
+
+| **Query**                  | **Expected impact**    | **Change** |
+|----------------------------|------------------------|------------|
+| Unreachable code | Fewer false positives | Analysis now accounts for uses of `contextlib.suppress` to suppress exceptions. |
+| `__iter__` method returns a non-iterator | Better alert message | Alert now highlights which class is expected to be an iterator. |