Merge remote-tracking branch 'upstream/master' into MissingEnumCaseInSwitch-perf

Author: jbj

.codeqlmanifest.json

@@ -2,5 +2,4 @@
                "*/ql/test/qlpack.yml",
                "*/upgrades/qlpack.yml",
                "misc/legacy-support/*/qlpack.yml",
-               "misc/suite-helpers/qlpack.yml",
-               "codeql/.codeqlmanifest.json" ] }
+               "misc/suite-helpers/qlpack.yml" ] }

.github/labeler.yml

@@ -0,0 +1,24 @@
+"C++":
+  - cpp/**/*
+  - change-notes/**/*cpp*
+
+"C#":
+  - csharp/**/*
+  - change-notes/**/*csharp*
+
+Java:
+  - java/**/*
+  - change-notes/**/*java.*
+
+JS:
+  - javascript/**/*
+  - change-notes/**/*javascript*
+
+Python:
+  - python/**/*
+  - change-notes/**/*python*
+
+documentation:
+  - "**/*.qhelp"
+  - "**/*.md"
+  - docs/**/*

.gitignore

@@ -1,6 +1,7 @@
 # editor and OS artifacts
 *~
 .DS_STORE
+*.swp
 
 # query compilation caches
 .cache
@@ -15,3 +16,5 @@
 
 # It's useful (though not required) to be able to unpack codeql in the ql checkout itself
 /codeql/
+.vscode/settings.json
+csharp/extractor/Semmle.Extraction.CSharp.Driver/Properties/launchSettings.json

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-cpp.md

@@ -0,0 +1,50 @@
+# Improvements to C/C++ analysis
+
+The following changes in version 1.24 affect C/C++ analysis in all applications.
+
+## General improvements
+
+## New queries
+
+| **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
+
+| **Query**                  | **Expected impact**    | **Change**                                                       |
+|----------------------------|------------------------|------------------------------------------------------------------|
+| Buffer not sufficient for string (`cpp/overflow-calculated`) | More true positive results | This query now identifies a wider variety of buffer allocations using the `semmle.code.cpp.models.interfaces.Allocation` library. |
+| No space for zero terminator (`cpp/no-space-for-terminator`) | More true positive results | This query now identifies a wider variety of buffer allocations using the `semmle.code.cpp.models.interfaces.Allocation` library. |
+| Memory is never freed (`cpp/memory-never-freed`) | More true positive results | This query now identifies a wider variety of buffer allocations using the `semmle.code.cpp.models.interfaces.Allocation` library. |
+| Memory may not be freed (`cpp/memory-may-not-be-freed`) | More true positive results | This query now identifies a wider variety of buffer allocations using the `semmle.code.cpp.models.interfaces.Allocation` library. |
+| Missing return statement (`cpp/missing-return`) | Fewer false positive results | Functions containing `asm` statements are no longer highlighted by this query. |
+| No space for zero terminator (`cpp/no-space-for-terminator`) | More correct results | String arguments to formatting functions are now (usually) expected to be null terminated strings. |
+| Hard-coded Japanese era start date (`cpp/japanese-era/exact-era-date`) |  | This query is no longer run on LGTM. |
+| No space for zero terminator (`cpp/no-space-for-terminator`) | Fewer false positive results | This query has been modified to be more conservative when identifying which pointers point to null-terminated strings.  This approach produces fewer, more accurate results. |
+| Overloaded assignment does not return 'this' (`cpp/assignment-does-not-return-this`) | Fewer false positive results | This query no longer reports incorrect results in template classes. |
+| Unsafe array for days of the year (`cpp/leap-year/unsafe-array-for-days-of-the-year`) |  | This query is no longer run on LGTM. |
+
+## Changes to libraries
+
+* The data-flow library has been improved, which affects and improves some security queries. The improvements are:
+  - Track flow through functions that combine taint tracking with flow through fields.
+  - Track flow through clone-like functions, that is, functions that read contents of a field from a
+    parameter and stores the value in the field of a returned object.
+* Created the `semmle.code.cpp.models.interfaces.Allocation` library to model allocation such as `new` expressions and calls to `malloc`. This in intended to replace the functionality in `semmle.code.cpp.commons.Alloc` with a more consistent and useful interface.
+* Created the `semmle.code.cpp.models.interfaces.Deallocation` library to model deallocation such as `delete` expressions and calls to `free`. This in intended to replace the functionality in `semmle.code.cpp.commons.Alloc` with a more consistent and useful interface.
+* The new class `StackVariable` should be used in place of `LocalScopeVariable`
+  in most cases. The difference is that `StackVariable` does not include
+  variables declared with `static` or `thread_local`.
+  * As a rule of thumb, custom queries about the _values_ of variables should
+    be changed from `LocalScopeVariable` to `StackVariable`, while queries
+    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`.
+* The security pack taint tracking library (`semmle.code.cpp.security.TaintTracking`) uses a new intermediate representation. This provides a more precise analysis of pointers to stack variables and flow through parameters, improving the results of many security queries.
+* The global value numbering library (`semmle.code.cpp.valuenumbering.GlobalValueNumbering`) uses a new intermediate representation to provide a more precise analysis of heap allocated memory and pointers to stack variables.

change-notes/1.24/analysis-csharp.md

@@ -0,0 +1,45 @@
+# Improvements to C# analysis
+
+The following changes in version 1.24 affect C# analysis in all applications.
+
+## New queries
+
+| **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. |
+| 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. | 
+| Potentially dangerous use of non-short-circuit logic (`cs/non-short-circuit`) | Fewer false positive results | Results have been removed when the expression contains an `out` parameter. |
+| Dereferenced variable may be null (`cs/dereferenced-value-may-be-null`) | More results | Results are reported from parameters with a default value of `null`. |
+| Useless assignment to local variable (`cs/useless-assignment-to-local`) | Fewer false positive results | Results have been removed when the value assigned is an (implicitly or explicitly) cast default-like value. For example, `var s = (string)null` and `string s = default`. |
+
+## Removal of old queries
+
+## Changes to code extraction
+
+* Tuple expressions, for example `(int,bool)` in `default((int,bool))` are now extracted correctly.
+* Expression nullability flow state is extracted.
+* Implicitly typed `stackalloc` expressions are now extracted correctly.
+* The difference between `stackalloc` array creations and normal array creations is extracted.
+
+## Changes to libraries
+
+* The data-flow library has been improved, which affects and improves most security queries. The improvements are:
+  - Track flow through methods that combine taint tracking with flow through fields.
+  - Track flow through clone-like methods, that is, methods that read contents of a field from a
+    parameter and stores the value in the field of a returned object.
+* The taint tracking library now tracks flow through (implicit or explicit) conversion operator calls.
+* [Code contracts](https://docs.microsoft.com/en-us/dotnet/framework/debug-trace-profile/code-contracts) are now recognized, and are treated like any other assertion methods.
+* Expression nullability flow state is given by the predicates `Expr.hasNotNullFlowState()` and `Expr.hasMaybeNullFlowState()`.
+* `stackalloc` array creations are now represented by the QL class `Stackalloc`. Previously they were represented by the class `ArrayCreation`.
+
+## Changes to autobuilder

change-notes/1.24/analysis-java.md

@@ -0,0 +1,42 @@
+# Improvements to Java analysis
+
+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 (`// ...`).
+
+## New queries
+
+| **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. Results are shown on LGTM by default. |
+| 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. |
+| LDAP query built from user-controlled sources (`java/ldap-injection`) | security, external/cwe/cwe-090 | Finds LDAP queries vulnerable to injection of unsanitized user-controlled input. 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. Results are shown on LGTM by default. |
+
+## 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. 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
+
+* The data-flow library has been improved, which affects and improves most security queries. The improvements are:
+  - Track flow through methods that combine taint tracking with flow through fields.
+  - Track flow through clone-like methods, that is, methods that read contents of a field from a
+    parameter and stores the value in the field of a returned object.
+* 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.
+* Parentheses are now no longer modelled directly in the AST, that is, the
+  `ParExpr` class is empty. Instead, a parenthesized expression can be
+  identified with the `Expr.isParenthesized()` member predicate.