Merge remote-tracking branch 'upstream/master' into dbartol/static-locals

Author: Dave Bartolomeo

CONTRIBUTING.md

@@ -1,62 +1,65 @@
 # Contributing to CodeQL
 
-We welcome contributions to our standard library and standard checks. Got an idea for a new check, or how to improve an existing query? Then please go ahead and open a pull request!
+We welcome contributions to our CodeQL libraries and queries. Got an idea for a new check, or how to improve an existing query? Then please go ahead and open a pull request!
 
-Before we accept your pull request, we require that you have agreed to our Contributor License Agreement, this is not something that you need to do before you submit your pull request, but until you've done so, we will be unable to accept your contribution.
+There is lots of useful documentation to help you write queries, ranging from information about query file structure to tutorials for specific target languages. For more information on the documentation available, see [Writing CodeQL queries](https://help.semmle.com/QL/learn-ql/writing-queries/writing-queries.html) on [help.semmle.com](https://help.semmle.com).
 
-## Adding a new query
 
-If you have an idea for a query that you would like to share with other Semmle users, please open a pull request to add it to this repository. 
-Follow the steps below to help other users understand what your query does, and to ensure that your query is consistent with the other Semmle queries.
+## Submitting a new experimental query
 
-1. **Consult the documentation for query writers**
+If you have an idea for a query that you would like to share with other CodeQL users, please open a pull request to add it to this repository. New queries start out in a `<language>/ql/src/experimental` directory, to which they can be merged when they meet the following requirements.
 
-   There is lots of useful documentation to help you write queries, ranging from information about query file structure to tutorials for specific target languages. For more information on the documentation available, see [Writing CodeQL queries](https://help.semmle.com/QL/learn-ql/writing-queries/writing-queries.html) on [help.semmle.com](https://help.semmle.com).
+1. **Directory structure**
 
-2. **Format your code correctly**
+    There are five language-specific query directories in this repository:
 
-   All of Semmle's standard queries and libraries are uniformly formatted for clarity and consistency, so we strongly recommend that all contributions follow the same formatting guidelines. If you use CodeQL for VS Code, you can autoformat your query in the [Editor](https://help.semmle.com/codeql/codeql-for-vscode/reference/editor.html#autoformatting). For more information, see the [CodeQL style guide](https://github.com/Semmle/ql/blob/master/docs/ql-style-guide.md).
+      * C/C++: `cpp/ql/src`
+      * C#: `csharp/ql/src`
+      * Java: `java/ql/src`
+      * JavaScript: `javascript/ql/src`
+      * Python: `python/ql/src`
 
-3. **Make sure your query has the correct metadata**
+    Each language-specific directory contains further subdirectories that group queries based on their `@tags` or purpose.
+    - Experimental queries and libraries are stored in the `experimental` subdirectory within each language-specific directory in the [CodeQL repository](https://github.com/Semmle/ql). For example, experimental Java queries and libraries are stored in `java/ql/src/experimental` and any corresponding tests in `java/ql/test/experimental`.
+    - The structure of an `experimental` subdirectory mirrors the structure of its parent directory.
+    - Select or create an appropriate directory in `experimental` based on the existing directory structure of `experimental` or its parent directory.
 
-   Query metadata is used by Semmle's analysis to identify your query and make sure the query results are displayed properly. 
-   The most important metadata to include are the `@name`, `@description`, and the `@kind`.
-   Other metadata properties (`@precision`, `@severity`, and `@tags`) are usually added after the query has been reviewed by Semmle staff.
-   For more information on writing query metadata, see the [Query metadata style guide](https://github.com/Semmle/ql/blob/master/docs/query-metadata-style-guide.md).
+2. **Query metadata**
 
-4. **Make sure the `select` statement is compatible with the query type**
+    - The query `@id` must conform to all the requirements in the [guide on query metadata](docs/query-metadata-style-guide.md#query-id-id). In particular, it must not clash with any other queries in the repository, and it must start with the appropriate language-specific prefix.
+    - The query must have a `@name` and `@description` to explain its purpose.
+    - The query must have a `@kind` and `@problem.severity` as required by CodeQL tools.
 
-   The `select` statement of your query must be compatible with the query type (determined by the `@kind` metadata property) for alert or path results to be displayed correctly in LGTM and CodeQL for VS Code.
-   For more information on `select` statement format, see [Introduction to query files](https://help.semmle.com/QL/learn-ql/writing-queries/introduction-to-queries.html#select-clause) on help.semmle.com.
+    For details, see the [guide on query metadata](docs/query-metadata-style-guide.md).
 
-5. **Save your query in a `.ql` file in the correct language directory in this repository**
+    Make sure the `select` statement is compatible with the query `@kind`. See [Introduction to query files](https://help.semmle.com/QL/learn-ql/writing-queries/introduction-to-queries.html#select-clause) on help.semmle.com.
 
-   There are five language-specific directories in this repository:
-   
-     * C/C++: `ql/cpp/ql/src`
-     * C#: `ql/csharp/ql/src`
-     * Java: `ql/java/ql/src`
-     * JavaScript: `ql/javascript/ql/src`
-     * Python: `ql/python/ql/src`
+3. **Formatting**
 
-   Each language-specific directory contains further subdirectories that group queries based on their `@tags` properties or purpose. Select the appropriate subdirectory for your new query, or create a new one if necessary. 
+    - The queries and libraries must be [autoformatted](https://help.semmle.com/codeql/codeql-for-vscode/reference/editor.html#autoformatting).
 
-6. **Write a query help file**
+4. **Compilation**
 
-   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).
+    - Compilation of the query and any associated libraries and tests must be resilient to future development of the [supported](docs/supported-queries.md) libraries. This means that the functionality cannot use internal libraries, cannot depend on the output of `getAQlClass`, and cannot make use of regexp matching on `toString`.
+    - The query and any associated libraries and tests must not cause any compiler warnings to be emitted (such as use of deprecated functionality or missing `override` annotations).
 
-7. **Maintain backwards compatibility**
+5. **Results**
 
-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.
+    - The query must have at least one true positive result on some revision of a real project.
 
-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.
+6. **Contributor License Agreement**
+
+    - The contributor can satisfy the [CLA](#contributor-license-agreement).
+
+Experimental queries and libraries may not be actively maintained as the [supported](docs/supported-queries.md) libraries evolve. They may also be changed in backwards-incompatible ways or may be removed entirely in the future without deprecation warnings.
+
+After the experimental query is merged, we welcome pull requests to improve it. Before a query can be moved out of the `experimental` subdirectory, it must satisfy [the requirements for being a supported query](docs/supported-queries.md).
 
 ## Using your personal data
 
 If you contribute to this project, we will record your name and email
 address (as provided by you with your contributions) as part of the code
-repositories, which might be made public. We might also use this information
+repositories, which are public. We might also use this information
 to contact you in relation to your contributions, as well as in the
 normal course of software development. We also store records of your
 CLA agreements. Under GDPR legislation, we do this

README.md

@@ -1,6 +1,6 @@
 # CodeQL
 
-This open source repository contains the standard CodeQL libraries and queries that power [LGTM](https://lgtm.com), and the other products that [Semmle](https://semmle.com) makes available to its customers worldwide.
+This open source repository contains the standard CodeQL libraries and queries that power [LGTM](https://lgtm.com) and the other CodeQL products that [GitHub](https://github.com) makes available to its customers worldwide.
 
 ## How do I learn CodeQL and run queries?
 
@@ -13,4 +13,4 @@ We welcome contributions to our standard library and standard checks. Do you hav
 
 ## License
 
-The code in this repository is licensed under [Apache License 2.0](LICENSE) by [Semmle](https://semmle.com).
+The code in this repository is licensed under [Apache License 2.0](LICENSE) by [GitHub](https://github.com).

change-notes/1.24/analysis-cpp.md

@@ -18,12 +18,14 @@ The following changes in version 1.24 affect C/C++ analysis in all applications.
 | 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. |
+| Mismatching new/free or malloc/delete (`cpp/new-free-mismatch`) | Fewer false positive results | Fixed false positive results in template code. |
 | 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. |
+| Unsigned comparison to zero (`cpp/unsigned-comparison-zero`) | More correct results | This query now also looks for comparisons of the form `0 <= x`. |
 
 ## Changes to libraries
 

change-notes/1.24/analysis-javascript.md

@@ -23,7 +23,9 @@
 
 * Support for the following frameworks and libraries has been improved:
   - [Electron](https://electronjs.org/)
+  - [fstream](https://www.npmjs.com/package/fstream)
   - [Handlebars](https://www.npmjs.com/package/handlebars)
+  - [jsonfile](https://www.npmjs.com/package/jsonfile)
   - [Koa](https://www.npmjs.com/package/koa)
   - [Node.js](https://nodejs.org/)
   - [Socket.IO](https://socket.io/)
@@ -32,11 +34,20 @@
   - [for-in](https://www.npmjs.com/package/for-in)
   - [for-own](https://www.npmjs.com/package/for-own)
   - [http2](https://nodejs.org/api/http2.html)
+  - [jQuery](https://jquery.com/)
   - [lazy-cache](https://www.npmjs.com/package/lazy-cache)
+  - [mongodb](https://www.npmjs.com/package/mongodb)
+  - [ncp](https://www.npmjs.com/package/ncp)
+  - [node-dir](https://www.npmjs.com/package/node-dir)
+  - [path-exists](https://www.npmjs.com/package/path-exists)
   - [react](https://www.npmjs.com/package/react)
+  - [recursive-readdir](https://www.npmjs.com/package/recursive-readdir)
   - [request](https://www.npmjs.com/package/request)
+  - [rimraf](https://www.npmjs.com/package/rimraf)
   - [send](https://www.npmjs.com/package/send)
   - [typeahead.js](https://www.npmjs.com/package/typeahead.js)
+  - [vinyl-fs](https://www.npmjs.com/package/vinyl-fs)
+  - [write-file-atomic](https://www.npmjs.com/package/write-file-atomic)
   - [ws](https://github.com/websockets/ws)
 
 ## New queries
@@ -67,6 +78,8 @@
 | Uncontrolled command line (`js/command-line-injection`) | More results | This query now recognizes additional ways of constructing arguments to `cmd.exe` and `/bin/sh`. |
 | Syntax error (`js/syntax-error`) | Lower severity | This results of this query are now displayed with lower severity. |
 | Use of password hash with insufficient computational effort (`js/insufficient-password-hash`) | Fewer false positive results | This query now recognizes additional cases that do not require secure hashing. |
+| Useless regular-expression character escape (`js/useless-regexp-character-escape`) | Fewer false positive results | This query now distinguishes escapes in strings and regular expression literals. |
+| Identical operands (`js/redundant-operation`) | Fewer results | This query now recognizes cases where the operands change a value using ++/-- expressions. |
 
 ## Changes to libraries
 

cpp/ql/src/Critical/NewDelete.qll

@@ -12,6 +12,7 @@ import semmle.code.cpp.dataflow.DataFlow
  */
 predicate allocExpr(Expr alloc, string kind) {
   isAllocationExpr(alloc) and
+  not alloc.isFromUninstantiatedTemplate(_) and
   (
     alloc instanceof FunctionCall and
     kind = "malloc"

cpp/ql/src/Likely Bugs/Arithmetic/UnsignedGEZero.qll

@@ -19,15 +19,19 @@ class ConstantZero extends Expr {
  * Holds if `candidate` is an expression such that if it's unsigned then we
  * want an alert at `ge`.
  */
-private predicate lookForUnsignedAt(GEExpr ge, Expr candidate) {
-  // Base case: `candidate >= 0`
-  ge.getRightOperand() instanceof ConstantZero and
-  candidate = ge.getLeftOperand().getFullyConverted() and
-  // left operand was a signed or unsigned IntegralType before conversions
+private predicate lookForUnsignedAt(RelationalOperation ge, Expr candidate) {
+  // Base case: `candidate >= 0` (or `0 <= candidate`)
+  (
+    ge instanceof GEExpr or
+    ge instanceof LEExpr
+  ) and
+  ge.getLesserOperand() instanceof ConstantZero and
+  candidate = ge.getGreaterOperand().getFullyConverted() and
+  // left/greater operand was a signed or unsigned IntegralType before conversions
   // (not a pointer, checking a pointer >= 0 is an entirely different mistake)
   // (not an enum, as the fully converted type of an enum is compiler dependent
   //  so checking an enum >= 0 is always reasonable)
-  ge.getLeftOperand().getUnderlyingType() instanceof IntegralType
+  ge.getGreaterOperand().getUnderlyingType() instanceof IntegralType
   or
   // Recursive case: `...(largerType)candidate >= 0`
   exists(Conversion conversion |
@@ -37,7 +41,7 @@ private predicate lookForUnsignedAt(GEExpr ge, Expr candidate) {
   )
 }
 
-class UnsignedGEZero extends GEExpr {
+class UnsignedGEZero extends ComparisonOperation {
   UnsignedGEZero() {
     exists(Expr ue |
       lookForUnsignedAt(this, ue) and

cpp/ql/src/experimental/Security/CWE/CWE-273/PrivilegeDroppingOutoforder.c

@@ -0,0 +1,138 @@
+#include <stdlib.h>
+#include <sys/param.h>
+#include <unistd.h>
+#include <pwd.h>
+
+void callSetuidAndCheck(int uid) {
+    if (setuid(uid) != 0) {
+        exit(1);
+    }
+}
+
+void callSetgidAndCheck(int gid) {
+    if (setgid(gid) != 0) {
+        exit(1);
+    }
+}
+
+/// Correct ways to drop priv.
+
+void correctDropPrivInline() {
+    if (setgroups(0, NULL)) {
+        exit(1);
+    }
+
+    if (setgid(-2) != 0) {
+        exit(1);
+    }
+
+    if (setuid(-2) != 0) {
+        exit(1);
+    }
+}
+
+void correctDropPrivInScope() {
+    {
+        if (setgroups(0, NULL)) {
+            exit(1);
+        }
+    }
+
+    {
+        if (setgid(-2) != 0) {
+            exit(1);
+        }
+    }
+
+    {
+        if (setuid(-2) != 0) {
+            exit(1);
+        }
+    }
+}
+
+void correctOrderForInitgroups() {
+    struct passwd *pw = getpwuid(0);
+    if (pw) {
+        if (initgroups(pw->pw_name, -2)) {
+            exit(1);
+        }
+    } else {
+        // Unhandled.
+    }
+    int rc = setuid(-2);
+    if (rc) {
+        exit(1);
+    }
+}
+
+void correctDropPrivInScopeParent() {
+    {
+        callSetgidAndCheck(-2);
+    }
+    correctOrderForInitgroups();
+}
+
+void incorrectNoReturnCodeCheck() {
+    int user = -2;
+    if (user) {
+        if (user) {
+            int rc = setgid(user);
+            (void)rc;
+            initgroups("nobody", user);
+        }
+        if (user) {
+            setuid(user);
+        }
+    }
+}
+
+void correctDropPrivInFunctionCall() {
+    if (setgroups(0, NULL)) {
+        exit(1);
+    }
+
+    callSetgidAndCheck(-2);
+    callSetuidAndCheck(-2);
+}
+
+/// Incorrect, out of order gid and uid.
+/// Calling uid before gid will fail.
+
+void incorrectDropPrivOutOfOrderInline() {
+    if (setuid(-2) != 0) {
+        exit(1);
+    }
+
+    if (setgid(-2) != 0) {
+        exit(1);
+    }
+}
+
+void incorrectDropPrivOutOfOrderInScope() {
+    {
+        if (setuid(-2) != 0) {
+            exit(1);
+        }
+    }
+
+    setgid(-2);
+}
+
+void incorrectDropPrivOutOfOrderWithFunction() {
+    callSetuidAndCheck(-2);
+
+    if (setgid(-2) != 0) {
+        exit(1);
+    }
+}
+
+void incorrectDropPrivOutOfOrderWithFunction2() {
+    callSetuidAndCheck(-2);
+    callSetgidAndCheck(-2);
+}
+
+void incorrectDropPrivNoCheck() {
+    setgid(-2);
+    setuid(-2);
+}

cpp/ql/src/experimental/Security/CWE/CWE-273/PrivilegeDroppingOutoforder.qhelp

@@ -0,0 +1,35 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>The code attempts to drop privilege in an incorrect order by
+erroneous dropping user privilege before groups. This has security
+impact if the return codes are not checked.</p>
+
+<p>False positives include code performing negative checks, making
+sure that setgid or setgroups does not work, meaning permissions are
+dropped. Additionally, other forms of sandboxing may be present removing
+any residual risk, for example a dedicated user namespace.</p>
+
+</overview>
+<recommendation>
+
+<p>Set the new group ID, then set the target user's intended groups by
+dropping previous supplemental source groups and initializing target
+groups, and finally set the target user.</p>
+
+</recommendation>
+<example>
+<p>The following example demonstrates out of order calls.</p>
+<sample src="PrivilegeDroppingOutoforder.c" />
+
+</example>
+<references>
+
+<li>CERT C Coding Standard:
+<a href="https://wiki.sei.cmu.edu/confluence/display/c/POS37-C.+Ensure+that+privilege+relinquishment+is+successful">POS37-C. Ensure that privilege relinquishment is successful</a>.
+</li>
+
+</references>
+</qhelp>