Merge remote-tracking branch 'upstream/master' into exceptionFPs

Author: erik-krogh

change-notes/1.24/analysis-cpp.md

@@ -8,6 +8,7 @@ The following changes in version 1.24 affect C/C++ analysis in all applications.
 
 | **Query**                   | **Tags**  | **Purpose**                                                        |
 |-----------------------------|-----------|--------------------------------------------------------------------|
+| Implicit function declarations (`cpp/Likely Bugs/Underspecified Functions/ImplicitFunctionDeclaration.ql`) | correctness, maintainability | This query finds calls to undeclared functions that are compiled by a C compiler. Results are shown on LGTM by default. |
 
 ## Changes to existing queries
 
@@ -35,7 +36,8 @@ The following changes in version 1.24 affect C/C++ analysis in all applications.
     about the _name or scope_ of variables should remain unchanged.
   * The `LocalScopeVariableReachability` library is deprecated in favor of
     `StackVariableReachability`. The functionality is the same.
+* The models library models `strlen` in more detail, and includes common variations such as `wcslen`.
 * The taint tracking library (`semmle.code.cpp.dataflow.TaintTracking`) has had
   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`.

change-notes/1.24/analysis-csharp.md

@@ -6,14 +6,19 @@ The following changes in version 1.24 affect C# analysis in all applications.
 
 | **Query**                   | **Tags**  | **Purpose**                                                        |
 |-----------------------------|-----------|--------------------------------------------------------------------|
+| Assembly path injection (`cs/assembly-path-injection`) | security, external/cwe/cwe-114 | Finds user-controlled data used to load an assembly. |
 | Insecure configuration for ASP.NET requestValidationMode (`cs/insecure-request-validation-mode`) | security, external/cwe/cwe-016 | Finds where this attribute has been set to a value less than 4.5, which turns off some validation features and makes the application less secure. |
-| Page request validation is disabled (`cs/web/request-validation-disabled`) | security, frameworks/asp.net, external/cwe/cwe-016 | Finds where ASP.NET page request validation has been disabled, which could makes the application less secure. |
+| Insecure SQL connection (`cs/insecure-sql-connection`) | security, external/cwe/cwe-327 | Finds unencrypted SQL connection strings. |
+| Page request validation is disabled (`cs/web/request-validation-disabled`) | security, frameworks/asp.net, external/cwe/cwe-016 | Finds where ASP.NET page request validation has been disabled, which could make the application less secure. |
+| Serialization check bypass (`cs/serialization-check-bypass`) | security, external/cwe/cwe-20 | Finds where data is not validated in a deserialization method. |
+| XML injection (`cs/xml-injection`) | security, external/cwe/cwe-091 | Finds user-controlled data that is used to write directly to an XML document. |
 
 ## Changes to existing queries
 
 | **Query**                    | **Expected impact**    | **Change**                        |
 |------------------------------|------------------------|-----------------------------------|
 | Useless assignment to local variable (`cs/useless-assignment-to-local`) | Fewer false positive results | Results have been removed when the variable is named `_` in a `foreach` statement. | 
+| Dereferenced variable may be null (`cs/dereferenced-value-may-be-null`) | More results | Results are reported from parameters with a default value of `null`. |
 
 ## Removal of old queries
 
@@ -29,4 +34,3 @@ The following changes in version 1.24 affect C# analysis in all applications.
 * Expression nullability flow state is given by the predicates `Expr.hasNotNullFlowState()` and `Expr.hasMaybeNullFlowState()`.
 
 ## Changes to autobuilder
-

change-notes/1.24/analysis-java.md

@@ -10,15 +10,25 @@ The following changes in version 1.24 affect Java analysis in all applications.
 
 | **Query**                   | **Tags**  | **Purpose**                                                        |
 |-----------------------------|-----------|--------------------------------------------------------------------|
+| Disabled Spring CSRF protection (`java/spring-disabled-csrf-protection`) | security, external/cwe/cwe-352 | Finds disabled Cross-Site Request Forgery (CSRF) protection in Spring. |
 | Failure to use HTTPS or SFTP URL in Maven artifact upload/download (`java/maven/non-https-url`) | security, external/cwe/cwe-300, external/cwe/cwe-319, external/cwe/cwe-494, external/cwe/cwe-829 | Finds use of insecure protocols during Maven dependency resolution. Results are shown on LGTM by default. |
+| Left shift by more than the type width (`java/lshift-larger-than-type-width`) | correctness | Finds left shifts of ints by 32 bits or more and left shifts of longs by 64 bits or more. Results are shown on LGTM by default. |
+| Suspicious date format (`java/suspicious-date-format`) | correctness | Finds date format patterns that use placeholders that are likely to be incorrect. |
 
 ## Changes to existing queries
 
 | **Query**                    | **Expected impact**    | **Change**                        |
 |------------------------------|------------------------|-----------------------------------|
 | Dereferenced variable may be null (`java/dereferenced-value-may-be-null`) | Fewer false positives | Final fields with a non-null initializer are no longer reported. |
-| Expression always evaluates to the same value (`java/evaluation-to-constant`) | Fewer false positives | Expressions of the form `0 * x` are usually intended and no longer reported. |
+| Expression always evaluates to the same value (`java/evaluation-to-constant`) | Fewer false positives | Expressions of the form `0 * x` are usually intended and no longer reported. Also left shift of ints by 32 bits and longs by 64 bits are no longer reported as they are not constant, these results are instead reported by the new query `java/lshift-larger-than-type-width`. |
 | Useless null check (`java/useless-null-check`) | More true positives | Useless checks on final fields with a non-null initializer are now reported. |
 
 ## Changes to libraries
 
+* 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
+  looks for the occurrence of actual unit-test annotations. This affects the
+  general file classification mechanism and thus suppression of alerts, and
+  also any security queries using taint tracking, as test classes act as
+  default barriers stopping taint flow.

change-notes/1.24/analysis-javascript.md

@@ -13,6 +13,11 @@
   - [react](https://www.npmjs.com/package/react)
   - [typeahead.js](https://www.npmjs.com/package/typeahead.js)
   - [Handlebars](https://www.npmjs.com/package/handlebars)
+  - [Electron](https://electronjs.org/)
+  - [Node.js](https://nodejs.org/)
+  - [Socket.IO](https://socket.io/)
+  - [ws](https://github.com/websockets/ws)
+  - [WebSocket](https://developer.mozilla.org/en-US/docs/Web/API/WebSockets_API)
 
 ## New queries
 
@@ -33,7 +38,9 @@
 | Unbound event handler receiver (`js/unbound-event-handler-receiver`) | Fewer false positive results | This query now recognizes additional ways event handler receivers can be bound. | 
 | 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. |
 
 ## Changes to libraries
 
 * The predicates `RegExpTerm.getSuccessor` and `RegExpTerm.getPredecessor` have been changed to reflect textual, not operational, matching order. This only makes a difference in lookbehind assertions, which are operationally matched backwards. Previously, `getSuccessor` would mimick this, so in an assertion `(?<=ab)` the term `b` would be considered the predecessor, not the successor, of `a`. Textually, however, `a` is still matched before `b`, and this is the order we now follow.
+* An extensible model of the `EventEmitter` pattern has been implemented.

config/identical-files.json

@@ -82,6 +82,14 @@
     "cpp/ql/src/semmle/code/cpp/ir/implementation/IRType.qll",
     "csharp/ql/src/semmle/code/csharp/ir/implementation/IRType.qll"
   ],
+  "IR IRConfiguration": [
+    "cpp/ql/src/semmle/code/cpp/ir/implementation/IRConfiguration.qll",
+    "csharp/ql/src/semmle/code/csharp/ir/implementation/IRConfiguration.qll"
+  ],
+  "IR UseSoundEscapeAnalysis": [
+    "cpp/ql/src/semmle/code/cpp/ir/implementation/UseSoundEscapeAnalysis.qll",
+    "csharp/ql/src/semmle/code/csharp/ir/implementation/UseSoundEscapeAnalysis.qll"
+  ],
   "IR Operand Tag": [
     "cpp/ql/src/semmle/code/cpp/ir/implementation/internal/OperandTag.qll",
     "csharp/ql/src/semmle/code/csharp/ir/implementation/internal/OperandTag.qll"

cpp/config/suites/c/correctness

@@ -18,6 +18,7 @@
 + semmlecode-cpp-queries/Likely Bugs/Likely Typos/ExprHasNoEffect.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Underspecified Functions/TooFewArguments.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Underspecified Functions/TooManyArguments.ql: /Correctness/Common Errors
++ semmlecode-cpp-queries/Likely Bugs/Underspecified Functions/ImplicitFunctionDeclaration.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Likely Typos/ShortCircuitBitMask.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Likely Typos/MissingEnumCaseInSwitch.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Arithmetic/FloatComparison.ql: /Correctness/Common Errors

cpp/config/suites/cpp/correctness

@@ -19,6 +19,7 @@
 + semmlecode-cpp-queries/Likely Bugs/Likely Typos/ExprHasNoEffect.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Underspecified Functions/TooFewArguments.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Underspecified Functions/TooManyArguments.ql: /Correctness/Common Errors
++ semmlecode-cpp-queries/Likely Bugs/Underspecified Functions/ImplicitFunctionDeclaration.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Likely Typos/ShortCircuitBitMask.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Likely Typos/MissingEnumCaseInSwitch.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Arithmetic/FloatComparison.ql: /Correctness/Common Errors

cpp/ql/src/Architecture/FeatureEnvy.ql

@@ -25,7 +25,8 @@ predicate functionUsesFunction(Function source, Function f, File target) {
 }
 
 predicate dependencyCount(Function source, File target, int res) {
-  res = strictcount(Declaration d |
+  res =
+    strictcount(Declaration d |
       functionUsesVariable(source, d, target) or
       functionUsesFunction(source, d, target)
     )

cpp/ql/src/Architecture/General Top-Level Information/GeneralStatistics.ql

@@ -38,14 +38,16 @@ where
   n = count(Function f | f.fromSource()).toString()
   or
   l = "Number of Lines Of Code" and
-  n = sum(File f, int toSum |
+  n =
+    sum(File f, int toSum |
       f.fromSource() and toSum = f.getMetrics().getNumberOfLinesOfCode()
     |
       toSum
     ).toString()
   or
   l = "Self-Containedness" and
-  n = (
+  n =
+    (
         100 * sum(Class c | c.fromSource() | c.getMetrics().getEfferentSourceCoupling()) /
           sum(Class c | c.fromSource() | c.getMetrics().getEfferentCoupling())
       ).toString() + "%"

cpp/ql/src/Architecture/Refactoring Opportunities/ClassesWithManyFields.ql

@@ -80,11 +80,8 @@ class VariableDeclarationLine extends TVariableDeclarationInfo {
    * (that is, the first is 0, the second is 1 and so on).
    */
   private int getRank() {
-    line = rank[result](VariableDeclarationLine vdl, int l |
-        vdl = TVariableDeclarationLine(c, f, l)
-      |
-        l
-      )
+    line =
+      rank[result](VariableDeclarationLine vdl, int l | vdl = TVariableDeclarationLine(c, f, l) | l)
   }
 
   /**
@@ -133,7 +130,8 @@ class VariableDeclarationGroup extends VariableDeclarationLine {
    * Gets the number of uniquely named `VariableDeclarationEntry`s in this group.
    */
   int getCount() {
-    result = count(VariableDeclarationLine l |
+    result =
+      count(VariableDeclarationLine l |
         l = getProximateNext*()
       |
         l.getAVDE().getVariable().getName()
@@ -166,7 +164,8 @@ class ExtClass extends Class {
 
 from ExtClass c, int n, VariableDeclarationGroup vdg, string suffix
 where
-  n = strictcount(string fieldName |
+  n =
+    strictcount(string fieldName |
       exists(Field f |
         f.getDeclaringType() = c and
         fieldName = f.getName() and