Merge branch 'master' into av114

This commit is contained in:
Geoffrey White
2020-04-01 18:23:21 +01:00
2606 changed files with 125735 additions and 60729 deletions

View File

@@ -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" ] }

4
.gitignore vendored
View File

@@ -1,6 +1,7 @@
# editor and OS artifacts
*~
.DS_STORE
*.swp
# query compilation caches
.cache
@@ -13,6 +14,9 @@
.vs/*
!.vs/VSWorkspaceSettings.json
# Byte-compiled python files
*.pyc
# It's useful (though not required) to be able to unpack codeql in the ql checkout itself
/codeql/
.vscode/settings.json

View File

@@ -1,56 +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).
5. **Results**
- The query must have at least one true positive result on some revision of a real project.
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

View File

@@ -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).

View File

@@ -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
@@ -17,15 +18,22 @@ 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. |
| Missing return statement (`cpp/missing-return`) | More accurate locations | Locations reported by this query are now more accurate in some cases. |
| 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
* 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`
@@ -36,3 +44,11 @@ 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 models library models `gets` and similar functions.
* 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.

View File

@@ -6,27 +6,41 @@ 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. |
| 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`. |
| XPath injection (`cs/xml/xpath-injection`) | More results | The query now recognizes calls to methods on `System.Xml.XPath.XPathNavigator` objects. |
## 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.
* 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

View File

@@ -5,20 +5,39 @@ 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 (`// ...`).
* A `Customizations.qll` file has been added to allow customizations of the standard library that apply to all queries.
## 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. |
| 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.

View File

@@ -2,17 +2,58 @@
## General improvements
* TypeScript 3.8 is now supported.
* Alert suppression can now be done with single-line block comments (`/* ... */`) as well as line comments (`// ...`).
* Imports with the `.js` extension can now be resolved to a TypeScript file,
* Resolution of imports has improved, leading to more results from the security queries:
- Imports with the `.js` extension can now be resolved to a TypeScript file,
when the import refers to a file generated by TypeScript.
- Imports that rely on path-mappings from a `tsconfig.json` file can now be resolved.
- Export declarations of the form `export * as ns from "x"` are now analyzed more precisely.
- The analysis of sanitizer guards has improved, leading to fewer false-positive results from the security queries.
* The analysis of sanitizers has improved, leading to more accurate results from the security queries.
In particular:
- Sanitizer guards now act across function boundaries in more cases.
- Sanitizers can now better distinguish between a tainted value and an object _containing_ a tainted value.
* Call graph construction has been improved, leading to more results from the security queries:
- Calls can now be resolved to indirectly-defined class members in more cases.
- Calls through partial invocations such as `.bind` can now be resolved in more cases.
* Support for flow summaries has been more clearly marked as being experimental and moved to the new `experimental` folder.
* Support for the following frameworks and libraries has been improved:
- [react](https://www.npmjs.com/package/react)
- [typeahead.js](https://www.npmjs.com/package/typeahead.js)
- [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/)
- [WebSocket](https://developer.mozilla.org/en-US/docs/Web/API/WebSockets_API)
- [chrome-remote-interface](https://www.npmjs.com/package/chrome-remote-interface)
- [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)
- [pg](https://www.npmjs.com/package/pg)
- [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)
- [SockJS](https://www.npmjs.com/package/sockjs)
- [SockJS-client](https://www.npmjs.com/package/sockjs-client)
- [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
@@ -21,6 +62,11 @@
| Cross-site scripting through exception (`js/xss-through-exception`) | security, external/cwe/cwe-079, external/cwe/cwe-116 | Highlights potential XSS vulnerabilities where an exception is written to the DOM. Results are not shown on LGTM by default. |
| Regular expression always matches (`js/regex/always-matches`) | correctness, regular-expressions | Highlights regular expression checks that trivially succeed by matching an empty substring. Results are shown on LGTM by default. |
| Missing await (`js/missing-await`) | correctness | Highlights expressions that operate directly on a promise object in a nonsensical way, instead of awaiting its result. Results are shown on LGTM by default. |
| Polynomial regular expression used on uncontrolled data (`js/polynomial-redos`) | security, external/cwe/cwe-730, external/cwe/cwe-400 | Highlights expensive regular expressions that may be used on malicious input. Results are shown on LGTM by default. |
| Prototype pollution in utility function (`js/prototype-pollution-utility`) | security, external/cwe/cwe-400, external/cwe/cwe-471 | Highlights recursive assignment operations that are susceptible to prototype pollution. Results are shown on LGTM by default. |
| Unsafe jQuery plugin (`js/unsafe-jquery-plugin`) | Highlights potential XSS vulnerabilities in unsafely designed jQuery plugins. Results are shown on LGTM by default. |
| Unnecessary use of `cat` process (`js/unnecessary-use-of-cat`) | correctness, security, maintainability | Highlights command executions of `cat` where the fs API should be used instead. Results are shown on LGTM by default. |
## Changes to existing queries
@@ -30,8 +76,23 @@
| Duplicate parameter names (`js/duplicate-parameter-name`) | Fewer results | This query now recognizes additional parameters that reasonably can have duplicated names. |
| Incomplete string escaping or encoding (`js/incomplete-sanitization`) | Fewer false positive results | This query now recognizes additional cases where a single replacement is likely to be intentional. |
| 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. |
| 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. |
| Uncontrolled data used in path expression (`js/path-injection`) | More results | This query now recognizes additional ways dangerous paths can be constructed and used. |
| 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. |
| Superfluous trailing arguments (`js/superfluous-trailing-arguments`) | Fewer results | This query now recognizes cases where a function uses the `Function.arguments` value to process a variable number of parameters. |
## 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.
* Taint-tracking configurations now interact differently with the `data` flow label, which may affect queries
that combine taint-tracking and flow labels.
- Sources added by the 1-argument `isSource` predicate are associated with the `taint` label now, instead of the `data` label.
- Sanitizers now only block the `taint` label. As a result, sanitizers no longer block the flow of tainted values wrapped inside a property of an object.
To retain the old behavior, instead use a barrier, or block the `data` flow label using a labeled sanitizer.

View File

@@ -0,0 +1,40 @@
# Improvements to Python analysis
The following changes in version 1.24 affect Python analysis in all applications.
## General improvements
Support for Django version 2.x and 3.x
## New queries
| **Query** | **Tags** | **Purpose** |
|-----------------------------|-----------|--------------------------------------------------------------------|
## Changes to existing queries
| **Query** | **Expected impact** | **Change** |
|----------------------------|------------------------|------------------------------------------------------------------|
| Uncontrolled command line (`py/command-line-injection`) | More results | We now model the `fabric` and `invoke` pacakges for command execution. |
### Web framework support
The QL-library support for the web frameworks Bottle, CherryPy, Falcon, Pyramid, TurboGears, Tornado, and Twisted have
been fixed so they provide a proper HttpRequestTaintSource, instead of a TaintSource. This will enable results for the following queries:
- py/path-injection
- py/command-line-injection
- py/reflective-xss
- py/sql-injection
- py/code-injection
- py/unsafe-deserialization
- py/url-redirection
The QL-library support for the web framework Twisted have been fixed so they provide a proper
HttpResponseTaintSink, instead of a TaintSink. This will enable results for the following
queries:
- py/reflective-xss
- py/stack-trace-exposure
## Changes to libraries

View File

@@ -39,6 +39,12 @@
"java/ql/src/semmle/code/java/dataflow/internal/tainttracking1/TaintTrackingImpl.qll",
"java/ql/src/semmle/code/java/dataflow/internal/tainttracking2/TaintTrackingImpl.qll"
],
"DataFlow Java/C++/C# Consistency checks": [
"java/ql/src/semmle/code/java/dataflow/internal/DataFlowImplConsistency.qll",
"cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImplConsistency.qll",
"cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImplConsistency.qll",
"csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImplConsistency.qll"
],
"C++ SubBasicBlocks": [
"cpp/ql/src/semmle/code/cpp/controlflow/SubBasicBlocks.qll",
"cpp/ql/src/semmle/code/cpp/dataflow/internal/SubBasicBlocks.qll"
@@ -82,6 +88,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"
@@ -182,9 +196,14 @@
"cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSAConstructionImports.qll",
"cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstructionImports.qll"
],
"C++ SSA AliasAnalysis": [
"SSA AliasAnalysis": [
"cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasAnalysis.qll",
"cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysis.qll"
"cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysis.qll",
"csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/AliasAnalysis.qll"
],
"C++ SSA AliasAnalysisImports": [
"cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasAnalysisImports.qll",
"cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysisImports.qll"
],
"C++ IR ValueNumberingImports": [
"cpp/ql/src/semmle/code/cpp/ir/implementation/raw/gvn/internal/ValueNumberingImports.qll",
@@ -195,6 +214,10 @@
"cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SimpleSSA.qll",
"csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/SimpleSSA.qll"
],
"IR AliasConfiguration (unaliased_ssa)": [
"cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasConfiguration.qll",
"csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/AliasConfiguration.qll"
],
"IR SSA SSAConstruction": [
"cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll",
"cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstruction.qll",
@@ -205,7 +228,14 @@
"cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/PrintSSA.qll",
"csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/PrintSSA.qll"
],
"IR ValueNumber": [
"IR ValueNumberInternal": [
"cpp/ql/src/semmle/code/cpp/ir/implementation/raw/gvn/internal/ValueNumberingInternal.qll",
"cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/gvn/internal/ValueNumberingInternal.qll",
"cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/gvn/internal/ValueNumberingInternal.qll",
"csharp/ql/src/semmle/code/csharp/ir/implementation/raw/gvn/internal/ValueNumberingInternal.qll",
"csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/gvn/internal/ValueNumberingInternal.qll"
],
"C++ IR ValueNumber": [
"cpp/ql/src/semmle/code/cpp/ir/implementation/raw/gvn/ValueNumbering.qll",
"cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/gvn/ValueNumbering.qll",
"cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/gvn/ValueNumbering.qll",

140
config/sync-files.py Normal file
View File

@@ -0,0 +1,140 @@
#!/usr/bin/env python3
# Due to various technical limitations, we sometimes have files that need to be
# kept identical in the repository. This script loads a database of such
# files and can perform two functions: check whether they are still identical,
# and overwrite the others with a master copy if needed.
import hashlib
import shutil
import os
import sys
import json
import re
path = os.path
file_groups = {}
def add_prefix(prefix, relative):
result = path.join(prefix, relative)
if path.commonprefix((path.realpath(result), path.realpath(prefix))) != \
path.realpath(prefix):
raise Exception("Path {} is not below {}".format(
result, prefix))
return result
def load_if_exists(prefix, json_file_relative):
json_file_name = path.join(prefix, json_file_relative)
if path.isfile(json_file_name):
print("Loading file groups from", json_file_name)
with open(json_file_name, 'r', encoding='utf-8') as fp:
raw_groups = json.load(fp)
prefixed_groups = {
name: [
add_prefix(prefix, relative)
for relative in relatives
]
for name, relatives in raw_groups.items()
}
file_groups.update(prefixed_groups)
# Generates a list of C# test files that should be in sync
def csharp_test_files():
test_file_re = re.compile('.*(Bad|Good)[0-9]*\\.cs$')
csharp_doc_files = {
file:os.path.join(root, file)
for root, dirs, files in os.walk("csharp/ql/src")
for file in files
if test_file_re.match(file)
}
return {
"C# test '" + file + "'" : [os.path.join(root, file), csharp_doc_files[file]]
for root, dirs, files in os.walk("csharp/ql/test")
for file in files
if file in csharp_doc_files
}
def file_checksum(filename):
with open(filename, 'rb') as file_handle:
return hashlib.sha1(file_handle.read()).hexdigest()
def check_group(group_name, files, master_file_picker, emit_error):
checksums = {file_checksum(f) for f in files}
if len(checksums) == 1:
return
master_file = master_file_picker(files)
if master_file is None:
emit_error(__file__, 0,
"Files from group '"+ group_name +"' not in sync.")
emit_error(__file__, 0,
"Run this script with a file-name argument among the "
"following to overwrite the remaining files with the contents "
"of that file or run with the --latest switch to update each "
"group of files from the most recently modified file in the group.")
for filename in files:
emit_error(__file__, 0, " " + filename)
else:
print(" Syncing others from", master_file)
for filename in files:
if filename == master_file:
continue
print(" " + filename)
os.replace(filename, filename + '~')
shutil.copy(master_file, filename)
print(" Backups written with '~' appended to file names")
def chdir_repo_root():
root_path = os.path.join(os.path.dirname(os.path.abspath(__file__)), '..')
os.chdir(root_path)
def choose_master_file(master_file, files):
if master_file in files:
return master_file
else:
return None
def choose_latest_file(files):
latest_time = None
latest_file = None
for filename in files:
file_time = os.path.getmtime(filename)
if (latest_time is None) or (latest_time < file_time):
latest_time = file_time
latest_file = filename
return latest_file
local_error_count = 0
def emit_local_error(path, line, error):
print('ERROR: ' + path + ':' + line + " - " + error)
global local_error_count
local_error_count += 1
# This function is invoked directly by a CI script, which passes a different error-handling
# callback.
def sync_identical_files(emit_error):
if len(sys.argv) == 1:
master_file_picker = lambda files: None
elif len(sys.argv) == 2:
if sys.argv[1] == "--latest":
master_file_picker = choose_latest_file
elif os.path.isfile(sys.argv[1]):
master_file_picker = lambda files: choose_master_file(sys.argv[1], files)
else:
raise Exception("File not found")
else:
raise Exception("Bad command line or file not found")
chdir_repo_root()
load_if_exists('.', 'config/identical-files.json')
file_groups.update(csharp_test_files())
for group_name, files in file_groups.items():
check_group(group_name, files, master_file_picker, emit_error)
def main():
sync_identical_files(emit_local_error)
if local_error_count > 0:
exit(1)
if __name__ == "__main__":
main()

View File

@@ -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

View File

@@ -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

View File

@@ -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)
)

View File

@@ -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() + "%"

View File

@@ -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

View File

@@ -50,7 +50,8 @@ class BlockOrNonChild extends Element {
private int getNonContiguousStartRankIn(AffectedFile file) {
// When using `rank` with `order by`, the ranks may not be contiguous.
this = rank[result](BlockOrNonChild boc, int startLine, int startCol |
this =
rank[result](BlockOrNonChild boc, int startLine, int startCol |
boc.getLocation().hasLocationInfo(file.getAbsolutePath(), startLine, startCol, _, _)
|
boc order by startLine, startCol
@@ -58,13 +59,15 @@ class BlockOrNonChild extends Element {
}
int getStartRankIn(AffectedFile file) {
this.getNonContiguousStartRankIn(file) = rank[result](int rnk |
this.getNonContiguousStartRankIn(file) =
rank[result](int rnk |
exists(BlockOrNonChild boc | boc.getNonContiguousStartRankIn(file) = rnk)
)
}
int getNonContiguousEndRankIn(AffectedFile file) {
this = rank[result](BlockOrNonChild boc, int endLine, int endCol |
this =
rank[result](BlockOrNonChild boc, int endLine, int endCol |
boc.getLocation().hasLocationInfo(file.getAbsolutePath(), _, _, endLine, endCol)
|
boc order by endLine, endCol
@@ -79,9 +82,8 @@ predicate emptyBlockContainsNonchild(Block b) {
emptyBlock(_, b) and
exists(BlockOrNonChild c, AffectedFile file |
c.(BlockOrNonChild).getStartRankIn(file) = 1 + b.(BlockOrNonChild).getStartRankIn(file) and
c.(BlockOrNonChild).getNonContiguousEndRankIn(file) < b
.(BlockOrNonChild)
.getNonContiguousEndRankIn(file)
c.(BlockOrNonChild).getNonContiguousEndRankIn(file) <
b.(BlockOrNonChild).getNonContiguousEndRankIn(file)
)
}

View File

@@ -307,7 +307,8 @@ predicate nonTrivialValue(string value, Literal literal) {
}
predicate valueOccurrenceCount(string value, int n) {
n = strictcount(Location loc |
n =
strictcount(Location loc |
exists(Literal lit | lit.getLocation() = loc | nonTrivialValue(value, lit)) and
// Exclude generated files (they do not have the same maintainability
// concerns as ordinary source files)
@@ -338,7 +339,8 @@ predicate check(Literal lit, string value, int n, File f) {
}
predicate checkWithFileCount(string value, int overallCount, int fileCount, File f) {
fileCount = strictcount(Location loc |
fileCount =
strictcount(Location loc |
exists(Literal lit | lit.getLocation() = loc | check(lit, value, overallCount, f))
)
}
@@ -364,7 +366,8 @@ predicate firstOccurrence(Literal lit, string value, int n) {
predicate magicConstant(Literal e, string msg) {
exists(string value, int n |
firstOccurrence(e, value, n) and
msg = "Magic constant: literal '" + value + "' is repeated " + n.toString() +
msg =
"Magic constant: literal '" + value + "' is repeated " + n.toString() +
" times and should be encapsulated in a constant."
)
}

View File

@@ -28,13 +28,15 @@ import cpp
// design question and carries has no safety risk.
predicate generatedCopyAssignment(CopyConstructor cc, string msg) {
cc.getDeclaringType().hasImplicitCopyAssignmentOperator() and
msg = "No matching copy assignment operator in class " + cc.getDeclaringType().getName() +
msg =
"No matching copy assignment operator in class " + cc.getDeclaringType().getName() +
". It is good practice to match a copy constructor with a " + "copy assignment operator."
}
predicate generatedCopyConstructor(CopyAssignmentOperator ca, string msg) {
ca.getDeclaringType().hasImplicitCopyConstructor() and
msg = "No matching copy constructor in class " + ca.getDeclaringType().getName() +
msg =
"No matching copy constructor in class " + ca.getDeclaringType().getName() +
". It is good practice to match a copy assignment operator with a " + "copy constructor."
}

View File

@@ -6,7 +6,7 @@
<overview>
<p>
This rule finds calls to <code>open</code> or <code>socket</code> where there is no corresponding <code>close</code> call in the program analyzed.
This rule finds calls to <code>socket</code> where there is no corresponding <code>close</code> call in the program analyzed.
Leaving descriptors open will cause a resource leak that will persist even after the program terminates.
</p>
@@ -14,7 +14,7 @@ Leaving descriptors open will cause a resource leak that will persist even after
</overview>
<recommendation>
<p>Ensure that all file or socket descriptors allocated by the program are freed before it terminates.</p>
<p>Ensure that all socket descriptors allocated by the program are freed before it terminates.</p>
</recommendation>
<example>

View File

@@ -1,6 +1,6 @@
/**
* @name Open descriptor never closed
* @description Functions that always return before closing the socket or file they opened leak resources.
* @description Functions that always return before closing the socket they opened leak resources.
* @kind problem
* @id cpp/descriptor-never-closed
* @problem.severity warning

View File

@@ -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"

View File

@@ -33,7 +33,8 @@ predicate spaceProblem(FunctionCall append, string msg) {
malloc.getASuccessor+() = insert and
insert.getArgument(1) = buffer.getAnAccess() and
insert.getASuccessor+() = append and
msg = "This buffer only contains enough room for '" + buffer.getName() + "' (copied on line " +
msg =
"This buffer only contains enough room for '" + buffer.getName() + "' (copied on line " +
insert.getLocation().getStartLine().toString() + ")"
)
}

View File

@@ -51,7 +51,8 @@ predicate overflowOffsetInLoop(BufferAccess bufaccess, string msg) {
loop.getStmt().getAChild*() = bufaccess.getEnclosingStmt() and
loop.limit() >= bufaccess.bufferSize() and
loop.counter().getAnAccess() = bufaccess.getArrayOffset() and
msg = "Potential buffer-overflow: counter '" + loop.counter().toString() + "' <= " +
msg =
"Potential buffer-overflow: counter '" + loop.counter().toString() + "' <= " +
loop.limit().toString() + " but '" + bufaccess.buffer().getName() + "' has " +
bufaccess.bufferSize().toString() + " elements."
)
@@ -106,8 +107,9 @@ predicate wrongBufferSize(Expr error, string msg) {
statedSize = min(call.statedSizeValue()) and
statedSize > bufsize and
error = call.statedSizeExpr() and
msg = "Potential buffer-overflow: '" + buf.getName() + "' has size " + bufsize.toString() +
" not " + statedSize + "."
msg =
"Potential buffer-overflow: '" + buf.getName() + "' has size " + bufsize.toString() + " not " +
statedSize + "."
)
}
@@ -121,8 +123,9 @@ predicate outOfBounds(BufferAccess bufaccess, string msg) {
or
access = size and not exists(AddressOfExpr addof | bufaccess = addof.getOperand())
) and
msg = "Potential buffer-overflow: '" + buf + "' has size " + size.toString() + " but '" + buf +
"[" + access.toString() + "]' is accessed here."
msg =
"Potential buffer-overflow: '" + buf + "' has size " + size.toString() + " but '" + buf + "[" +
access.toString() + "]' is accessed here."
)
}

View File

@@ -23,7 +23,8 @@ predicate important(Function f, string message) {
predicate dubious(Function f, string message) {
not important(f, _) and
exists(Options opts, int used, int total, int percentage |
used = count(FunctionCall fc |
used =
count(FunctionCall fc |
fc.getTarget() = f and not opts.okToIgnoreReturnValue(fc) and not unused(fc)
) and
total = count(FunctionCall fc | fc.getTarget() = f and not opts.okToIgnoreReturnValue(fc)) and

View File

@@ -18,14 +18,13 @@ string getCommentTextCaptioned(Comment c, string caption) {
dontCare = commentBody.regexpFind("\\n[/* \\t\\x0B\\f\\r]*" + caption, _, offset) and
interestingSuffix = commentBody.suffix(offset) and
endOfLine = interestingSuffix.indexOf("\n", 1, 0) and
captionedLine = interestingSuffix
captionedLine =
interestingSuffix
.prefix(endOfLine)
.regexpReplaceAll("^[/*\\s]*" + caption + "\\s*:?", "")
.trim() and
followingLine = interestingSuffix
.prefix(interestingSuffix.indexOf("\n", 2, 0))
.suffix(endOfLine)
.trim() and
followingLine =
interestingSuffix.prefix(interestingSuffix.indexOf("\n", 2, 0)).suffix(endOfLine).trim() and
if captionedLine = ""
then result = caption + " comment"
else

View File

@@ -56,7 +56,8 @@ VariableAccess getAnIncrement(Variable var) {
exists(AssignAddExpr a | a.getLValue() = result and a.getRValue().getValue().toInt() > 0)
or
exists(AssignExpr a | a.getLValue() = result |
a.getRValue() = any(AddExpr ae |
a.getRValue() =
any(AddExpr ae |
ae.getAnOperand() = var.getAnAccess() and
ae.getAnOperand().getValue().toInt() > 0
)
@@ -72,7 +73,8 @@ VariableAccess getADecrement(Variable var) {
exists(AssignSubExpr a | a.getLValue() = result and a.getRValue().getValue().toInt() > 0)
or
exists(AssignExpr a | a.getLValue() = result |
a.getRValue() = any(SubExpr ae |
a.getRValue() =
any(SubExpr ae |
ae.getLeftOperand() = var.getAnAccess() and
ae.getRightOperand().getValue().toInt() > 0
)
@@ -128,14 +130,16 @@ where
exists(VariableAccess bound |
upperBoundCheck(loop, bound) and
reachesNoInc(bound, bound) and
msg = "The loop counter " + bound.getTarget().getName() +
msg =
"The loop counter " + bound.getTarget().getName() +
" is not always incremented in the loop body."
)
or
exists(VariableAccess bound |
lowerBoundCheck(loop, bound) and
reachesNoDec(bound, bound) and
msg = "The loop counter " + bound.getTarget().getName() +
msg =
"The loop counter " + bound.getTarget().getName() +
" is not always decremented in the loop body."
)
)

View File

@@ -21,6 +21,7 @@ where
if call.getTarget() = call.getEnclosingFunction()
then msg = "This call directly invokes its containing function $@."
else
msg = "The function " + call.getEnclosingFunction() +
msg =
"The function " + call.getEnclosingFunction() +
" is indirectly recursive via this call to $@."
select call, msg, call.getTarget(), call.getTarget().getName()

View File

@@ -17,7 +17,8 @@ predicate lockOrder(LockOperation outer, LockOperation inner) {
}
int orderCount(Declaration outerLock, Declaration innerLock) {
result = strictcount(LockOperation outer, LockOperation inner |
result =
strictcount(LockOperation outer, LockOperation inner |
outer.getLocked() = outerLock and
inner.getLocked() = innerLock and
lockOrder(outer, inner)
@@ -27,6 +28,6 @@ int orderCount(Declaration outerLock, Declaration innerLock) {
from LockOperation outer, LockOperation inner
where
lockOrder(outer, inner) and
orderCount(outer.getLocked(), inner.getLocked()) <= orderCount(inner.getLocked(),
outer.getLocked())
orderCount(outer.getLocked(), inner.getLocked()) <=
orderCount(inner.getLocked(), outer.getLocked())
select inner, "Out-of-order locks: A " + inner.say() + " usually precedes a $@.", outer, outer.say()

View File

@@ -81,10 +81,8 @@ class LockingPrimitive extends FunctionCall, LockOperation {
override Function getLocked() { result = this.getTarget() }
override UnlockOperation getMatchingUnlock() {
result.(UnlockingPrimitive).getTarget().getName() = this
.getTarget()
.getName()
.replaceAll("Lock", "Unlock")
result.(UnlockingPrimitive).getTarget().getName() =
this.getTarget().getName().replaceAll("Lock", "Unlock")
}
override string say() { result = "call to " + getLocked().getName() }

View File

@@ -29,7 +29,8 @@ where
numStmt(f, line) = cnt and
cnt > 1 and
o.onLine(f, line) and
o.getLocation().getStartColumn() = min(OneLineStmt other, int toMin |
o.getLocation().getStartColumn() =
min(OneLineStmt other, int toMin |
other.onLine(f, line) and toMin = other.getLocation().getStartColumn()
|
toMin

View File

@@ -14,7 +14,8 @@ import cpp
string var(Variable v) {
exists(int level | level = v.getType().getPointerIndirectionLevel() |
level > 2 and
result = "The type of " + v.getName() + " uses " + level +
result =
"The type of " + v.getName() + " uses " + level +
" levels of pointer indirection -- maximum allowed is 2."
)
}
@@ -22,7 +23,8 @@ string var(Variable v) {
string fun(Function f) {
exists(int level | level = f.getType().getPointerIndirectionLevel() |
level > 2 and
result = "The return type of " + f.getName() + " uses " + level +
result =
"The return type of " + f.getName() + " uses " + level +
" levels of pointer indirection -- maximum allowed is 2."
)
}

View File

@@ -12,7 +12,8 @@
import cpp
int firstCodeLine(File f) {
result = min(Declaration d, Location l, int toMin |
result =
min(Declaration d, Location l, int toMin |
(
l = d.getLocation() and
l.getFile() = f and

View File

@@ -59,7 +59,8 @@ Expr getMulOperand(MulExpr me) { result = me.getAnOperand() }
* ```
*/
int getEffectiveMulOperands(MulExpr me) {
result = count(Expr op |
result =
count(Expr op |
op = getMulOperand*(me) and
not op instanceof MulExpr and
not likelySmall(op)
@@ -109,24 +110,28 @@ class MulAnalyzableExpr extends AnalyzableExpr, MulExpr {
class AddAnalyzableExpr extends AnalyzableExpr, AddExpr {
override float maxValue() {
result = this.getLeftOperand().getFullyConverted().(AnalyzableExpr).maxValue() +
result =
this.getLeftOperand().getFullyConverted().(AnalyzableExpr).maxValue() +
this.getRightOperand().getFullyConverted().(AnalyzableExpr).maxValue()
}
override float minValue() {
result = this.getLeftOperand().getFullyConverted().(AnalyzableExpr).minValue() +
result =
this.getLeftOperand().getFullyConverted().(AnalyzableExpr).minValue() +
this.getRightOperand().getFullyConverted().(AnalyzableExpr).minValue()
}
}
class SubAnalyzableExpr extends AnalyzableExpr, SubExpr {
override float maxValue() {
result = this.getLeftOperand().getFullyConverted().(AnalyzableExpr).maxValue() -
result =
this.getLeftOperand().getFullyConverted().(AnalyzableExpr).maxValue() -
this.getRightOperand().getFullyConverted().(AnalyzableExpr).minValue()
}
override float minValue() {
result = this.getLeftOperand().getFullyConverted().(AnalyzableExpr).minValue() -
result =
this.getLeftOperand().getFullyConverted().(AnalyzableExpr).minValue() -
this.getRightOperand().getFullyConverted().(AnalyzableExpr).maxValue()
}
}

View File

@@ -15,24 +15,37 @@ class ConstantZero extends Expr {
}
}
class UnsignedGEZero extends GEExpr {
/**
* Holds if `candidate` is an expression such that if it's unsigned then we
* want an alert at `ge`.
*/
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.getGreaterOperand().getUnderlyingType() instanceof IntegralType
or
// Recursive case: `...(largerType)candidate >= 0`
exists(Conversion conversion |
lookForUnsignedAt(ge, conversion) and
candidate = conversion.getExpr() and
conversion.getType().getSize() > candidate.getType().getSize()
)
}
class UnsignedGEZero extends ComparisonOperation {
UnsignedGEZero() {
this.getRightOperand() instanceof ConstantZero and
// left 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)
getLeftOperand().getUnderlyingType() instanceof IntegralType and
exists(Expr ue |
// ue is some conversion of the left operand
ue = getLeftOperand().getConversion*() and
// ue is unsigned
ue.getUnderlyingType().(IntegralType).isUnsigned() and
// ue may be converted to zero or more strictly larger possibly signed types
// before it is fully converted
forall(Expr following | following = ue.getConversion+() |
following.getType().getSize() > ue.getType().getSize()
)
lookForUnsignedAt(this, ue) and
ue.getUnderlyingType().(IntegralType).isUnsigned()
)
}
}

View File

@@ -52,10 +52,7 @@ predicate introducesNewField(Class derived, Class base) {
from DataFlow::PathNode source, DataFlow::PathNode sink, CastToPointerArithFlow cfg
where
cfg.hasFlowPath(source, sink) and
source.getNode().asExpr().getFullyConverted().getUnspecifiedType() = sink
.getNode()
.asExpr()
.getFullyConverted()
.getUnspecifiedType()
source.getNode().asExpr().getFullyConverted().getUnspecifiedType() =
sink.getNode().asExpr().getFullyConverted().getUnspecifiedType()
select sink, source, sink,
"Pointer arithmetic here may be done with the wrong type because of the cast $@.", source, "here"

View File

@@ -130,11 +130,8 @@ predicate trivialConversion(ExpectedType expected, Type actual) {
or
// allow a pointer to any integral type of the same size
// (this permits signedness changes)
expected.(PointerType).getBaseType().(IntegralType).getSize() = actual
.(PointerType)
.getBaseType()
.(IntegralType)
.getSize()
expected.(PointerType).getBaseType().(IntegralType).getSize() =
actual.(PointerType).getBaseType().(IntegralType).getSize()
or
expected = actual
)

View File

@@ -65,11 +65,8 @@ predicate functionDefinedInIfDefRecursive(Function f) {
* break encapsulation.
*/
predicate baseCall(FunctionCall call) {
call.getNameQualifier().getQualifyingElement() = call
.getEnclosingFunction()
.getDeclaringType()
.(Class)
.getABaseClass+()
call.getNameQualifier().getQualifyingElement() =
call.getEnclosingFunction().getDeclaringType().(Class).getABaseClass+()
}
from PureExprInVoidContext peivc, Locatable parent, Locatable info, string info_text, string tail

View File

@@ -35,7 +35,8 @@ predicate booleanLiteral(Literal l) {
string boolLiteralInLogicalOp(Literal literal) {
booleanLiteral(literal) and
literal.getParent() instanceof BinaryLogicalOperation and
result = "Literal value " + literal.getValueText() +
result =
"Literal value " + literal.getValueText() +
" is used in a logical expression; simplify or use a constant."
}

View File

@@ -40,7 +40,8 @@ where
l = v.log2().floor() and
if v = 2.pow(l)
then
msg = "Operand to short-circuiting operator looks like a flag (" + v + " = 2 ^ " + l +
msg =
"Operand to short-circuiting operator looks like a flag (" + v + " = 2 ^ " + l +
"), may be typo for bitwise operator."
else
exists(string kind |
@@ -49,7 +50,8 @@ where
or
e instanceof OctalLiteral and kind = "an octal literal"
) and
msg = "Operand to short-circuiting operator is " + kind +
msg =
"Operand to short-circuiting operator is " + kind +
", and therefore likely a flag; a bitwise operator may be intended."
)
select e, msg

View File

@@ -63,7 +63,8 @@ predicate isStringCopyUsedInLogicalOperationOrCondition(FunctionCall func, Expr
func = ce.getCondition()
)
) and
msg = "Return value of " + func.getTarget().getName() +
msg =
"Return value of " + func.getTarget().getName() +
" used directly in a conditional expression."
)
}

View File

@@ -111,17 +111,20 @@ predicate illDefinedForStmt(ForStmt for, string message) {
illDefinedForStmtWrongDirection(for, v, initialCondition, terminalCondition, isIncr) and
if for.conditionAlwaysFalse()
then
message = "Ill-defined for-loop: a loop using variable \"" + v + "\" counts " +
message =
"Ill-defined for-loop: a loop using variable \"" + v + "\" counts " +
forLoopdirection(isIncr) + " from a value (" + initialCondition +
"), but the terminal condition is always false."
else
if for.conditionAlwaysTrue()
then
message = "Ill-defined for-loop: a loop using variable \"" + v + "\" counts " +
message =
"Ill-defined for-loop: a loop using variable \"" + v + "\" counts " +
forLoopdirection(isIncr) + " from a value (" + initialCondition +
"), but the terminal condition is always true."
else
message = "Ill-defined for-loop: a loop using variable \"" + v + "\" counts " +
message =
"Ill-defined for-loop: a loop using variable \"" + v + "\" counts " +
forLoopdirection(isIncr) + " from a value (" + initialCondition +
"), but the terminal condition is " + forLoopTerminalConditionRelationship(isIncr) +
" (" + terminalCondition + ")."

View File

@@ -122,7 +122,7 @@ class MallocSizeExpr extends BufferAccess, FunctionCall {
override Expr getPointer() { none() }
override Expr getAccessedLength() { result = getArgument(1) }
override Expr getAccessedLength() { result = getArgument(0) }
}
class NetworkFunctionCall extends FunctionCall {

View File

@@ -30,9 +30,9 @@ class SprintfCall extends FunctionCall {
predicate isDangerous() { this.getMaxConvertedLength() > this.getBufferSize() }
string getDescription() {
result = "This conversion may yield a string of length " +
this.getMaxConvertedLength().toString() + ", which exceeds the allocated buffer size of " +
this.getBufferSize().toString()
result =
"This conversion may yield a string of length " + this.getMaxConvertedLength().toString() +
", which exceeds the allocated buffer size of " + this.getBufferSize().toString()
}
}

View File

@@ -28,7 +28,8 @@ class NullInstruction extends ConstantValueInstruction {
}
predicate explicitNullTestOfInstruction(Instruction checked, Instruction bool) {
bool = any(CompareInstruction cmp |
bool =
any(CompareInstruction cmp |
exists(NullInstruction null |
cmp.getLeft() = null and cmp.getRight() = checked
or
@@ -40,7 +41,8 @@ predicate explicitNullTestOfInstruction(Instruction checked, Instruction bool) {
)
)
or
bool = any(ConvertInstruction convert |
bool =
any(ConvertInstruction convert |
checked = convert.getUnary() and
convert.getResultType() instanceof BoolType and
checked.getResultType() instanceof PointerType

View File

@@ -17,7 +17,9 @@ where
hasSuperfluousConstReturn(f) and
if f.hasSpecifier("const") or f.isStatic()
then
message = "The 'const' modifier has no effect on return types. The 'const' modifying the return type can be removed."
message =
"The 'const' modifier has no effect on return types. The 'const' modifying the return type can be removed."
else
message = "The 'const' modifier has no effect on return types. For a const function, the 'const' should go after the parameter list."
message =
"The 'const' modifier has no effect on return types. For a const function, the 'const' should go after the parameter list."
select f, message

View File

@@ -0,0 +1,8 @@
/* '#include <stdlib.h>' was forgotton */
int main(void) {
/* 'int malloc()' assumed */
unsigned char *p = malloc(100);
*p = 'a';
return 0;
}

View File

@@ -0,0 +1,29 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>A function is called without a prior function declaration or definition.
When this happens, the compiler generates an implicit declaration of the function,
specifying an integer return type and no parameters.
If the implicit declaration does not match the true signature of the function, the
function may behave unpredictably.</p>
<p>This may indicate a misspelled function name, or that the required header containing
the function declaration has not been included.</p>
</overview>
<recommendation>
<p>Provide an explicit declaration of the function before invoking it.</p>
</recommendation>
<example><sample src="ImplicitFunctionDeclaration.c" />
</example>
<references>
<li>SEI CERT C Coding Standard: <a href="https://wiki.sei.cmu.edu/confluence/display/c/DCL31-C.+Declare+identifiers+before+using+them">DCL31-C. Declare identifiers before using them</a></li>
</references>
</qhelp>

View File

@@ -0,0 +1,48 @@
/**
* @name Implicit function declaration
* @description An implicitly declared function is assumed to take no
* arguments and return an integer. If this assumption does not hold, it
* may lead to unpredictable behavior.
* @kind problem
* @problem.severity warning
* @precision high
* @id cpp/implicit-function-declaration
* @tags correctness
* maintainability
*/
import cpp
import MistypedFunctionArguments
import TooFewArguments
import TooManyArguments
import semmle.code.cpp.commons.Exclusions
predicate locInfo(Locatable e, File file, int line, int col) {
e.getFile() = file and
e.getLocation().getStartLine() = line and
e.getLocation().getStartColumn() = col
}
predicate sameLocation(FunctionDeclarationEntry fde, FunctionCall fc) {
exists(File file, int line, int col |
locInfo(fde, file, line, col) and
locInfo(fc, file, line, col)
)
}
predicate isCompiledAsC(File f) {
f.compiledAsC()
or
exists(File src | isCompiledAsC(src) | src.getAnIncludedFile() = f)
}
from FunctionDeclarationEntry fdeIm, FunctionCall fc
where
isCompiledAsC(fdeIm.getFile()) and
not isFromMacroDefinition(fc) and
fdeIm.isImplicit() and
sameLocation(fdeIm, fc) and
not mistypedFunctionArguments(fc, _, _) and
not tooFewArguments(fc, _) and
not tooManyArguments(fc, _)
select fc, "Function call implicitly declares '" + fdeIm.getName() + "'."

View File

@@ -12,95 +12,10 @@
*/
import cpp
predicate arithTypesMatch(Type arg, Type parm) {
arg = parm
or
arg.getSize() = parm.getSize() and
(
arg instanceof IntegralOrEnumType and
parm instanceof IntegralOrEnumType
or
arg instanceof FloatingPointType and
parm instanceof FloatingPointType
)
}
pragma[inline]
predicate nestedPointerArgTypeMayBeUsed(Type arg, Type parm) {
// arithmetic types
arithTypesMatch(arg, parm)
or
// conversion to/from pointers to void is allowed
arg instanceof VoidType
or
parm instanceof VoidType
}
pragma[inline]
predicate pointerArgTypeMayBeUsed(Type arg, Type parm) {
nestedPointerArgTypeMayBeUsed(arg, parm)
or
// nested pointers
nestedPointerArgTypeMayBeUsed(arg.(PointerType).getBaseType().getUnspecifiedType(),
parm.(PointerType).getBaseType().getUnspecifiedType())
or
nestedPointerArgTypeMayBeUsed(arg.(ArrayType).getBaseType().getUnspecifiedType(),
parm.(PointerType).getBaseType().getUnspecifiedType())
}
pragma[inline]
predicate argTypeMayBeUsed(Type arg, Type parm) {
// arithmetic types
arithTypesMatch(arg, parm)
or
// pointers to compatible types
pointerArgTypeMayBeUsed(arg.(PointerType).getBaseType().getUnspecifiedType(),
parm.(PointerType).getBaseType().getUnspecifiedType())
or
pointerArgTypeMayBeUsed(arg.(ArrayType).getBaseType().getUnspecifiedType(),
parm.(PointerType).getBaseType().getUnspecifiedType())
or
// C11 arrays
pointerArgTypeMayBeUsed(arg.(PointerType).getBaseType().getUnspecifiedType(),
parm.(ArrayType).getBaseType().getUnspecifiedType())
or
pointerArgTypeMayBeUsed(arg.(ArrayType).getBaseType().getUnspecifiedType(),
parm.(ArrayType).getBaseType().getUnspecifiedType())
}
// This predicate holds whenever expression `arg` may be used to initialize
// function parameter `parm` without need for run-time conversion.
pragma[inline]
predicate argMayBeUsed(Expr arg, Parameter parm) {
argTypeMayBeUsed(arg.getFullyConverted().getUnspecifiedType(), parm.getUnspecifiedType())
}
// True if function was ()-declared, but not (void)-declared or K&R-defined
predicate hasZeroParamDecl(Function f) {
exists(FunctionDeclarationEntry fde | fde = f.getADeclarationEntry() |
not fde.hasVoidParamList() and fde.getNumberOfParameters() = 0 and not fde.isDefinition()
)
}
// True if this file (or header) was compiled as a C file
predicate isCompiledAsC(File f) {
f.compiledAsC()
or
exists(File src | isCompiledAsC(src) | src.getAnIncludedFile() = f)
}
import MistypedFunctionArguments
from FunctionCall fc, Function f, Parameter p
where
f = fc.getTarget() and
p = f.getAParameter() and
hasZeroParamDecl(f) and
isCompiledAsC(f.getFile()) and
not f.isVarargs() and
not f instanceof BuiltInFunction and
p.getIndex() < fc.getNumberOfArguments() and
// Parameter p and its corresponding call argument must have mismatched types
not argMayBeUsed(fc.getArgument(p.getIndex()), p)
where mistypedFunctionArguments(fc, f, p)
select fc, "Calling $@: argument $@ of type $@ is incompatible with parameter $@.", f, f.toString(),
fc.getArgument(p.getIndex()) as arg, arg.toString(),
arg.getExplicitlyConverted().getUnspecifiedType() as atype, atype.toString(), p, p.getTypedName()

View File

@@ -0,0 +1,96 @@
/**
* Provides the implementation of the MistypedFunctionArguments query. The
* query is implemented as a library, so that we can avoid producing
* duplicate results in other similar queries.
*/
import cpp
private predicate arithTypesMatch(Type arg, Type parm) {
arg = parm
or
arg.getSize() = parm.getSize() and
(
arg instanceof IntegralOrEnumType and
parm instanceof IntegralOrEnumType
or
arg instanceof FloatingPointType and
parm instanceof FloatingPointType
)
}
pragma[inline]
private predicate nestedPointerArgTypeMayBeUsed(Type arg, Type parm) {
// arithmetic types
arithTypesMatch(arg, parm)
or
// conversion to/from pointers to void is allowed
arg instanceof VoidType
or
parm instanceof VoidType
}
pragma[inline]
private predicate pointerArgTypeMayBeUsed(Type arg, Type parm) {
nestedPointerArgTypeMayBeUsed(arg, parm)
or
// nested pointers
nestedPointerArgTypeMayBeUsed(arg.(PointerType).getBaseType().getUnspecifiedType(),
parm.(PointerType).getBaseType().getUnspecifiedType())
or
nestedPointerArgTypeMayBeUsed(arg.(ArrayType).getBaseType().getUnspecifiedType(),
parm.(PointerType).getBaseType().getUnspecifiedType())
}
pragma[inline]
private predicate argTypeMayBeUsed(Type arg, Type parm) {
// arithmetic types
arithTypesMatch(arg, parm)
or
// pointers to compatible types
pointerArgTypeMayBeUsed(arg.(PointerType).getBaseType().getUnspecifiedType(),
parm.(PointerType).getBaseType().getUnspecifiedType())
or
pointerArgTypeMayBeUsed(arg.(ArrayType).getBaseType().getUnspecifiedType(),
parm.(PointerType).getBaseType().getUnspecifiedType())
or
// C11 arrays
pointerArgTypeMayBeUsed(arg.(PointerType).getBaseType().getUnspecifiedType(),
parm.(ArrayType).getBaseType().getUnspecifiedType())
or
pointerArgTypeMayBeUsed(arg.(ArrayType).getBaseType().getUnspecifiedType(),
parm.(ArrayType).getBaseType().getUnspecifiedType())
}
// This predicate holds whenever expression `arg` may be used to initialize
// function parameter `parm` without need for run-time conversion.
pragma[inline]
private predicate argMayBeUsed(Expr arg, Parameter parm) {
argTypeMayBeUsed(arg.getFullyConverted().getUnspecifiedType(), parm.getUnspecifiedType())
}
// True if function was ()-declared, but not (void)-declared or K&R-defined
private predicate hasZeroParamDecl(Function f) {
exists(FunctionDeclarationEntry fde | fde = f.getADeclarationEntry() |
not fde.hasVoidParamList() and fde.getNumberOfParameters() = 0 and not fde.isDefinition()
)
}
// True if this file (or header) was compiled as a C file
private predicate isCompiledAsC(File f) {
f.compiledAsC()
or
exists(File src | isCompiledAsC(src) | src.getAnIncludedFile() = f)
}
predicate mistypedFunctionArguments(FunctionCall fc, Function f, Parameter p) {
f = fc.getTarget() and
p = f.getAParameter() and
hasZeroParamDecl(f) and
isCompiledAsC(f.getFile()) and
not f.isVarargs() and
not f instanceof BuiltInFunction and
p.getIndex() < fc.getNumberOfArguments() and
// Parameter p and its corresponding call argument must have mismatched types
not argMayBeUsed(fc.getArgument(p.getIndex()), p)
}

View File

@@ -15,31 +15,8 @@
*/
import cpp
// True if function was ()-declared, but not (void)-declared or K&R-defined
predicate hasZeroParamDecl(Function f) {
exists(FunctionDeclarationEntry fde | fde = f.getADeclarationEntry() |
not fde.hasVoidParamList() and fde.getNumberOfParameters() = 0 and not fde.isDefinition()
)
}
// True if this file (or header) was compiled as a C file
predicate isCompiledAsC(File f) {
f.compiledAsC()
or
exists(File src | isCompiledAsC(src) | src.getAnIncludedFile() = f)
}
import TooFewArguments
from FunctionCall fc, Function f
where
f = fc.getTarget() and
not f.isVarargs() and
not f instanceof BuiltInFunction and
hasZeroParamDecl(f) and
isCompiledAsC(f.getFile()) and
// There is an explicit declaration of the function whose parameter count is larger
// than the number of call arguments
exists(FunctionDeclarationEntry fde | fde = f.getADeclarationEntry() |
fde.getNumberOfParameters() > fc.getNumberOfArguments()
)
where tooFewArguments(fc, f)
select fc, "This call has fewer arguments than required by $@.", f, f.toString()

View File

@@ -0,0 +1,34 @@
/**
* Provides the implementation of the TooFewArguments query. The
* query is implemented as a library, so that we can avoid producing
* duplicate results in other similar queries.
*/
import cpp
// True if function was ()-declared, but not (void)-declared or K&R-defined
private predicate hasZeroParamDecl(Function f) {
exists(FunctionDeclarationEntry fde | fde = f.getADeclarationEntry() |
not fde.hasVoidParamList() and fde.getNumberOfParameters() = 0 and not fde.isDefinition()
)
}
// True if this file (or header) was compiled as a C file
private predicate isCompiledAsC(File f) {
f.compiledAsC()
or
exists(File src | isCompiledAsC(src) | src.getAnIncludedFile() = f)
}
predicate tooFewArguments(FunctionCall fc, Function f) {
f = fc.getTarget() and
not f.isVarargs() and
not f instanceof BuiltInFunction and
hasZeroParamDecl(f) and
isCompiledAsC(f.getFile()) and
// There is an explicit declaration of the function whose parameter count is larger
// than the number of call arguments
exists(FunctionDeclarationEntry fde | fde = f.getADeclarationEntry() |
fde.getNumberOfParameters() > fc.getNumberOfArguments()
)
}

View File

@@ -12,35 +12,8 @@
*/
import cpp
// True if function was ()-declared, but not (void)-declared or K&R-defined
// or implicitly declared (i.e., lacking a prototype)
predicate hasZeroParamDecl(Function f) {
exists(FunctionDeclarationEntry fde | fde = f.getADeclarationEntry() |
not fde.isImplicit() and
not fde.hasVoidParamList() and
fde.getNumberOfParameters() = 0 and
not fde.isDefinition()
)
}
// True if this file (or header) was compiled as a C file
predicate isCompiledAsC(File f) {
f.compiledAsC()
or
exists(File src | isCompiledAsC(src) | src.getAnIncludedFile() = f)
}
import TooManyArguments
from FunctionCall fc, Function f
where
f = fc.getTarget() and
not f.isVarargs() and
hasZeroParamDecl(f) and
isCompiledAsC(f.getFile()) and
exists(f.getBlock()) and
// There must not exist a declaration with the number of parameters
// at least as large as the number of call arguments
not exists(FunctionDeclarationEntry fde | fde = f.getADeclarationEntry() |
fde.getNumberOfParameters() >= fc.getNumberOfArguments()
)
where tooManyArguments(fc, f)
select fc, "This call has more arguments than required by $@.", f, f.toString()

View File

@@ -0,0 +1,38 @@
/**
* Provides the implementation of the TooManyArguments query. The
* query is implemented as a library, so that we can avoid producing
* duplicate results in other similar queries.
*/
import cpp
// True if function was ()-declared, but not (void)-declared or K&R-defined
// or implicitly declared (i.e., lacking a prototype)
private predicate hasZeroParamDecl(Function f) {
exists(FunctionDeclarationEntry fde | fde = f.getADeclarationEntry() |
not fde.isImplicit() and
not fde.hasVoidParamList() and
fde.getNumberOfParameters() = 0 and
not fde.isDefinition()
)
}
// True if this file (or header) was compiled as a C file
private predicate isCompiledAsC(File f) {
f.compiledAsC()
or
exists(File src | isCompiledAsC(src) | src.getAnIncludedFile() = f)
}
predicate tooManyArguments(FunctionCall fc, Function f) {
f = fc.getTarget() and
not f.isVarargs() and
hasZeroParamDecl(f) and
isCompiledAsC(f.getFile()) and
exists(f.getBlock()) and
// There must not exist a declaration with the number of parameters
// at least as large as the number of call arguments
not exists(FunctionDeclarationEntry fde | fde = f.getADeclarationEntry() |
fde.getNumberOfParameters() >= fc.getNumberOfArguments()
)
}

View File

@@ -14,6 +14,7 @@ import cpp
from Class c, int n
where
c.fromSource() and
n = c.getMetrics().getNumberOfMembers() +
n =
c.getMetrics().getNumberOfMembers() +
sum(Function f | c.getACanonicalMemberFunction() = f | f.getMetrics().getNumberOfLinesOfCode())
select c, n order by n desc

View File

@@ -15,16 +15,15 @@ import cpp
from Class c, int ccLoc, int loc
where
c.fromSource() and
ccLoc = sum(Function f |
ccLoc =
sum(Function f |
c.getACanonicalMemberFunction() = f and
f.getMetrics().getCyclomaticComplexity() > 18
|
f.getMetrics().getNumberOfLinesOfCode()
) and
loc = sum(Function f |
c.getACanonicalMemberFunction() = f
|
f.getMetrics().getNumberOfLinesOfCode()
) + c.getMetrics().getNumberOfMembers() and
loc =
sum(Function f | c.getACanonicalMemberFunction() = f | f.getMetrics().getNumberOfLinesOfCode()) +
c.getMetrics().getNumberOfMembers() and
loc != 0
select c, (ccLoc * 100).(float) / loc as n order by n desc

View File

@@ -76,7 +76,8 @@ class Library extends LibraryT {
* `sourceFile` is not in `destLib`).
*/
predicate libDependencies(File sourceFile, Library destLib, int num) {
num = strictcount(Element source, Element dest, File destFile |
num =
strictcount(Element source, Element dest, File destFile |
// dependency from source -> dest.
dependsOnSimple(source, dest) and
sourceFile = source.getFile() and
@@ -101,7 +102,7 @@ predicate libDependencies(File sourceFile, Library destLib, int num) {
predicate encodedDependencies(File source, string encodedDependency, int num) {
exists(Library destLib |
libDependencies(source, destLib, num) and
encodedDependency = "/" + source.getRelativePath() + "<|>" + destLib.getName() + "<|>" +
destLib.getVersion()
encodedDependency =
"/" + source.getRelativePath() + "<|>" + destLib.getName() + "<|>" + destLib.getVersion()
)
}

View File

@@ -85,7 +85,8 @@ private predicate closeWithDepth(int depth, File f, PreprocessorDirective close,
predicate length(PreprocessorDirective open, int length) {
exists(int depth, File f, int start, int end |
openWithDepth(depth, f, open, start) and
end = min(PreprocessorDirective endif, int closeLine |
end =
min(PreprocessorDirective endif, int closeLine |
closeWithDepth(depth, f, endif, closeLine) and
closeLine > start
|

View File

@@ -19,7 +19,8 @@ where
if loc > 0
then
// Weighted average of complexity by function length
complexity = sum(FunctionDeclarationEntry fde |
complexity =
sum(FunctionDeclarationEntry fde |
fde.getFile() = f
|
fde.getNumberOfLines() * fde.getCyclomaticComplexity()

View File

@@ -17,7 +17,8 @@ import external.CodeDuplication
from File f, int n
where
n = count(int line |
n =
count(int line |
exists(DuplicateBlock d | d.sourceFile() = f |
line in [d.sourceStartLine() .. d.sourceEndLine()]
) and

View File

@@ -15,7 +15,8 @@ import cpp
from File f, int n
where
f.fromSource() and
n = count(Comment c |
n =
count(Comment c |
c.getFile() = f and
(
c.getContents().matches("%TODO%") or

View File

@@ -20,7 +20,8 @@ predicate callToOperator(FunctionCall fc) {
from Function f, int n, int o
where
strictcount(f.getEntryPoint()) = 1 and
o = count(FunctionCall c |
o =
count(FunctionCall c |
c.getEnclosingFunction() = f and
not c.isInMacroExpansion() and
not c.isCompilerGenerated() and

View File

@@ -16,7 +16,8 @@ import external.VCS
from File f, int n
where
n = sum(Commit entry, int churn |
n =
sum(Commit entry, int churn |
churn = entry.getRecentChurnForFile(f) and
not artificialChange(entry)
|

View File

@@ -16,7 +16,8 @@ import external.VCS
from File f, int n
where
n = sum(Commit entry, int churn |
n =
sum(Commit entry, int churn |
churn = entry.getRecentAdditionsForFile(f) and
not artificialChange(entry)
|

View File

@@ -16,7 +16,8 @@ import external.VCS
from File f, int n
where
n = sum(Commit entry, int churn |
n =
sum(Commit entry, int churn |
churn = entry.getRecentDeletionsForFile(f) and
not artificialChange(entry)
|

View File

@@ -25,7 +25,8 @@ predicate inRange(Commit first, Commit second) {
}
int recommitsForFile(File f) {
result = count(Commit recommit |
result =
count(Commit recommit |
f = recommit.getAnAffectedFile() and
exists(Commit prev | inRange(prev, recommit))
)

View File

@@ -15,7 +15,8 @@ import external.VCS
from File f, int n
where
n = count(Commit e |
n =
count(Commit e |
e.getAnAffectedFile() = f and
e.daysToNow() <= 180 and
not artificialChange(e)

View File

@@ -147,7 +147,8 @@ library class SALElement extends Element {
exists(Location loc |
loc = this.(FunctionDeclarationEntry).getBlock().getLocation()
or
this = any(VariableDeclarationEntry vde |
this =
any(VariableDeclarationEntry vde |
vde.isDefinition() and
loc = vde.getVariable().getInitializer().getLocation()
)
@@ -194,7 +195,8 @@ private predicate salAnnotationPos(SALPosition pos) {
* ordering positions lexicographically by their start line and start column.
*/
private SALPosition salRelevantPositionAt(File file, int idx) {
result = rank[idx](SALPosition pos, int line, int col |
result =
rank[idx](SALPosition pos, int line, int col |
pos = MkSALPosition(file, line, col)
|
pos order by line, col

View File

@@ -21,6 +21,7 @@ where
if call.getTarget() = call.getEnclosingFunction()
then msg = "This call directly invokes its containing function $@."
else
msg = "The function " + call.getEnclosingFunction() +
msg =
"The function " + call.getEnclosingFunction() +
" is indirectly recursive via this call to $@."
select call, msg, call.getTarget(), call.getTarget().getName()

View File

@@ -58,7 +58,8 @@ VariableAccess getAnIncrement(Variable var) {
exists(AssignAddExpr a | a.getLValue() = result and a.getRValue().getValue().toInt() > 0)
or
exists(AssignExpr a | a.getLValue() = result |
a.getRValue() = any(AddExpr ae |
a.getRValue() =
any(AddExpr ae |
ae.getAnOperand() = var.getAnAccess() and
ae.getAnOperand().getValue().toInt() > 0
)
@@ -74,7 +75,8 @@ VariableAccess getADecrement(Variable var) {
exists(AssignSubExpr a | a.getLValue() = result and a.getRValue().getValue().toInt() > 0)
or
exists(AssignExpr a | a.getLValue() = result |
a.getRValue() = any(SubExpr ae |
a.getRValue() =
any(SubExpr ae |
ae.getLeftOperand() = var.getAnAccess() and
ae.getRightOperand().getValue().toInt() > 0
)
@@ -125,14 +127,16 @@ where
exists(VariableAccess bound |
upperBoundCheck(loop, bound) and
reachesNoInc(bound, bound) and
msg = "The loop counter " + bound.getTarget().getName() +
msg =
"The loop counter " + bound.getTarget().getName() +
" is not always incremented in the loop body."
)
or
exists(VariableAccess bound |
lowerBoundCheck(loop, bound) and
reachesNoDec(bound, bound) and
msg = "The loop counter " + bound.getTarget().getName() +
msg =
"The loop counter " + bound.getTarget().getName() +
" is not always decremented in the loop body."
)
)

View File

@@ -23,7 +23,8 @@ class MacroFunctionCall extends MacroInvocation {
}
int logicalLength(FunctionDeclarationEntry f) {
result = count(Stmt s |
result =
count(Stmt s |
s.getEnclosingFunction() = f.getFunction() and
s.getFile() = f.getFile() and
not s instanceof Block and

View File

@@ -39,7 +39,8 @@ where
numStmt(f, line) = cnt and
cnt > 1 and
o.onLine(f, line) and
o.getLocation().getStartColumn() = min(OneLineStmt other, int toMin |
o.getLocation().getStartColumn() =
min(OneLineStmt other, int toMin |
other.onLine(f, line) and toMin = other.getLocation().getStartColumn()
|
toMin

View File

@@ -23,7 +23,8 @@ class MacroFunctionCall extends MacroInvocation {
}
int logicalLength(FunctionDeclarationEntry f) {
result = count(Stmt s |
result =
count(Stmt s |
s.getEnclosingFunction() = f.getFunction() and
s.getFile() = f.getFile() and
not s instanceof Block and
@@ -34,7 +35,8 @@ int logicalLength(FunctionDeclarationEntry f) {
}
int assertionCount(FunctionDeclarationEntry f) {
result = count(Assertion a |
result =
count(Assertion a |
a.getAsserted().getEnclosingFunction() = f.getFunction() and a.getFile() = f.getFile()
)
}

View File

@@ -34,22 +34,24 @@ where
) and
if accessType = 1
then
message = "This '" + ba.getName() + "' operation accesses " +
plural(accessSize, " byte", " bytes") + " but the $@ is only " +
plural(bufferSize, " byte", " bytes") + "."
message =
"This '" + ba.getName() + "' operation accesses " + plural(accessSize, " byte", " bytes") +
" but the $@ is only " + plural(bufferSize, " byte", " bytes") + "."
else
if accessType = 2
then
message = "This '" + ba.getName() + "' operation may access " +
plural(accessSize, " byte", " bytes") + " but the $@ is only " +
plural(bufferSize, " byte", " bytes") + "."
message =
"This '" + ba.getName() + "' operation may access " + plural(accessSize, " byte", " bytes") +
" but the $@ is only " + plural(bufferSize, " byte", " bytes") + "."
else (
if accessSize > 0
then
message = "This array indexing operation accesses byte offset " + (accessSize - 1) +
message =
"This array indexing operation accesses byte offset " + (accessSize - 1) +
" but the $@ is only " + plural(bufferSize, " byte", " bytes") + "."
else
message = "This array indexing operation accesses a negative index " +
message =
"This array indexing operation accesses a negative index " +
((accessSize / ba.getActualType().getSize()) - 1) + " on the $@."
)
select ba, message, bufferAlloc, bufferDesc

View File

@@ -22,16 +22,25 @@ import semmle.code.cpp.models.interfaces.Allocation
predicate terminationProblem(AllocationExpr malloc, string msg) {
// malloc(strlen(...))
exists(StrlenCall strlen | DataFlow::localExprFlow(strlen, malloc.getSizeExpr())) and
// flows into a null-terminated string function
// flows to a call that implies this is a null-terminated string
exists(ArrayFunction af, FunctionCall fc, int arg |
DataFlow::localExprFlow(malloc, fc.getArgument(arg)) and
fc.getTarget() = af and
(
// null terminated string
// flows into null terminated string argument
af.hasArrayWithNullTerminator(arg)
or
// likely a null terminated string (such as `strcpy`, `strcat`)
// flows into likely null terminated string argument (such as `strcpy`, `strcat`)
af.hasArrayWithUnknownSize(arg)
or
// flows into string argument to a formatting function (such as `printf`)
exists(int n, FormatLiteral fl |
fc.getArgument(arg) = fc.(FormattingFunctionCall).getConversionArgument(n) and
fl = fc.(FormattingFunctionCall).getFormat() and
fl.getConversionType(n) instanceof PointerType and // `%s`, `%ws` etc
not fl.getConversionType(n) instanceof VoidPointerType and // exclude: `%p`
not fl.hasPrecision(n) // exclude: `%.*s`
)
)
) and
msg = "This allocation does not include space to null-terminate the string."

View File

@@ -56,9 +56,12 @@ where
// very noisy on codebases that started as 32-bit
small.getExplicitlyConverted().getType().getSize() < 4 and
// Ignore cases where integer promotion has occurred on /, -, or >> expressions.
not getComparisonSize(large.(DivExpr).getLeftOperand().getExplicitlyConverted()) <= getComparisonSize(small) and
not getComparisonSize(large.(SubExpr).getLeftOperand().getExplicitlyConverted()) <= getComparisonSize(small) and
not getComparisonSize(large.(RShiftExpr).getLeftOperand().getExplicitlyConverted()) <= getComparisonSize(small) and
not getComparisonSize(large.(DivExpr).getLeftOperand().getExplicitlyConverted()) <=
getComparisonSize(small) and
not getComparisonSize(large.(SubExpr).getLeftOperand().getExplicitlyConverted()) <=
getComparisonSize(small) and
not getComparisonSize(large.(RShiftExpr).getLeftOperand().getExplicitlyConverted()) <=
getComparisonSize(small) and
// ignore loop-invariant smaller variables
loopVariant(small, l)
select rel,

View File

@@ -59,13 +59,15 @@ where
(
exists(BinaryLogicalOperation blop | blop.getAnOperand() = e1 |
e1.getType().(TypedefType).hasName("HRESULT") and
msg = "Usage of a type " + e1.getType().toString() +
msg =
"Usage of a type " + e1.getType().toString() +
" as an argument of a binary logical operation"
)
or
exists(UnaryLogicalOperation ulop | ulop.getAnOperand() = e1 |
e1.getType().(TypedefType).hasName("HRESULT") and
msg = "Usage of a type " + e1.getType().toString() +
msg =
"Usage of a type " + e1.getType().toString() +
" as an argument of a unary logical operation"
) and
not isHresultBooleanConverted(e1)

View File

@@ -108,7 +108,8 @@ where
exists(Expr source, Expr cmd, QuotedCommandInCreateProcessFunctionConfiguration quotedConfig |
cmd = call.getArgument(call.getCommandLineArgumentId()) and
quotedConfig.hasFlow(DataFlow2::exprNode(source), DataFlow2::exprNode(cmd)) and
msg2 = " and with an unquoted lpCommandLine (" + cmd +
msg2 =
" and with an unquoted lpCommandLine (" + cmd +
") introduces a security vulnerability if the path contains spaces."
)
select call, msg1 + " " + msg2

View File

@@ -90,9 +90,9 @@ class ParameterNullCheck extends ParameterCheck {
va = this.(NotExpr).getOperand() or
va = any(EQExpr eq | eq = this and eq.getAnOperand().getValue() = "0").getAnOperand() or
va = getCheckedFalseCondition(this) or
va = any(NEExpr eq |
eq = getCheckedFalseCondition(this) and eq.getAnOperand().getValue() = "0"
).getAnOperand()
va =
any(NEExpr eq | eq = getCheckedFalseCondition(this) and eq.getAnOperand().getValue() = "0")
.getAnOperand()
)
or
nullSuccessor = getAFalseSuccessor() and
@@ -100,9 +100,9 @@ class ParameterNullCheck extends ParameterCheck {
(
va = this or
va = any(NEExpr eq | eq = this and eq.getAnOperand().getValue() = "0").getAnOperand() or
va = any(EQExpr eq |
eq = getCheckedFalseCondition(this) and eq.getAnOperand().getValue() = "0"
).getAnOperand()
va =
any(EQExpr eq | eq = getCheckedFalseCondition(this) and eq.getAnOperand().getValue() = "0")
.getAnOperand()
)
)
}
@@ -188,7 +188,8 @@ class InitializationFunction extends Function {
isPointerDereferenceAssignmentTarget(this.getParameter(i).getAnAccess()) or
// Field wise assignment to the parameter
any(Assignment e).getLValue() = getAFieldAccess(this.getParameter(i)) or
i = this
i =
this
.(MemberFunction)
.getAnOverridingFunction+()
.(InitializationFunction)
@@ -231,7 +232,8 @@ class InitializationFunction extends Function {
)
)
or
result = any(AssumeExpr e |
result =
any(AssumeExpr e |
e.getEnclosingFunction() = this and e.getAChild().(Literal).getValue() = "0"
)
)
@@ -472,7 +474,8 @@ class ConditionalInitializationCall extends FunctionCall {
a.getLValue() = fa and
fa.getASuccessor+() = result
) and
result = this
result =
this
.getArgument(getTarget(this)
.(ConditionalInitializationFunction)
.conditionallyInitializedParameter(_))
@@ -589,7 +592,8 @@ Expr getAConditionallyInitializedArgument(
* Gets the type signature for the functions parameters.
*/
private string typeSig(Function f) {
result = concat(int i, Type pt |
result =
concat(int i, Type pt |
pt = f.getParameter(i).getType()
|
pt.getUnspecifiedType().toString(), "," order by i

View File

@@ -32,10 +32,12 @@ predicate setWorldWritable(FunctionCall fc, int mode) {
from Expr fc, int mode, string message
where
worldWritableCreation(fc, mode) and
message = "A file may be created here with mode " + octalFileMode(mode) +
message =
"A file may be created here with mode " + octalFileMode(mode) +
", which would make it world-writable."
or
setWorldWritable(fc, mode) and
message = "This sets a file's permissions to " + octalFileMode(mode) +
message =
"This sets a file's permissions to " + octalFileMode(mode) +
", which would make it world-writable."
select fc, message

View File

@@ -15,8 +15,8 @@ bindingset[mode]
string octalFileMode(int mode) {
if mode >= 0 and mode <= 4095
then
/* octal 07777 */ result = "0" + octalDigitOpt(mode, 3) + octalDigit(mode, 2) +
octalDigit(mode, 1) + octalDigit(mode, 0)
/* octal 07777 */ result =
"0" + octalDigitOpt(mode, 3) + octalDigit(mode, 2) + octalDigit(mode, 1) + octalDigit(mode, 0)
else result = "[non-standard mode: decimal " + mode + "]"
}
@@ -128,12 +128,8 @@ class OpenatCreationExpr extends FileCreationExpr {
}
private int fopenMode() {
result = s_irusr()
.bitOr(s_irgrp())
.bitOr(s_iroth())
.bitOr(s_iwusr())
.bitOr(s_iwgrp())
.bitOr(s_iwoth())
result =
s_irusr().bitOr(s_irgrp()).bitOr(s_iroth()).bitOr(s_iwusr()).bitOr(s_iwgrp()).bitOr(s_iwoth())
}
class FopenCreationExpr extends FileCreationExpr {

View File

@@ -52,9 +52,8 @@ class NonNullDaclConfig extends DataFlow2::Configuration {
NonNullDaclConfig() { this = "NonNullDaclConfig" }
override predicate isSource(DataFlow::Node source) {
source.getType().getUnspecifiedType().(PointerType).getBaseType() = any(Type t |
t.getName() = "ACL"
).getUnspecifiedType() and
source.getType().getUnspecifiedType().(PointerType).getBaseType() =
any(Type t | t.getName() = "ACL").getUnspecifiedType() and
(
// If the value comes from a function whose body we can't see, assume
// it's not null.
@@ -79,7 +78,8 @@ class NonNullDaclConfig extends DataFlow2::Configuration {
from SetSecurityDescriptorDaclFunctionCall call, string message
where
exists(NullValue nullExpr |
message = "Setting a DACL to NULL in a SECURITY_DESCRIPTOR will result in an unprotected object."
message =
"Setting a DACL to NULL in a SECURITY_DESCRIPTOR will result in an unprotected object."
|
call.getArgument(1).getValue().toInt() != 0 and
call.getArgument(2) = nullExpr
@@ -89,7 +89,8 @@ where
Expr constassign, VariableAccess var, NullDaclConfig nullDaclConfig,
NonNullDaclConfig nonNullDaclConfig
|
message = "Setting a DACL to NULL in a SECURITY_DESCRIPTOR using variable " + var +
message =
"Setting a DACL to NULL in a SECURITY_DESCRIPTOR using variable " + var +
" that is set to NULL will result in an unprotected object."
|
var = call.getArgument(2) and

View File

@@ -0,0 +1 @@
This directory contains [experimental](../../../../docs/experimental.md) CodeQL queries and libraries.

View File

@@ -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);
}

View File

@@ -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>

View File

@@ -0,0 +1,101 @@
/**
* @name LinuxPrivilegeDroppingOutoforder
* @description A syscall commonly associated with privilege dropping is being called out of order.
* Normally a process drops group ID and sets supplimental groups for the target user
* before setting the target user ID. This can have security impact if the return code
* from these methods is not checked.
* @kind problem
* @problem.severity recommendation
* @id cpp/drop-linux-privileges-outoforder
* @tags security
* external/cwe/cwe-273
* @precision medium
*/
import cpp
predicate argumentMayBeRoot(Expr e) {
e.getValue() = "0" or
e.(VariableAccess).getTarget().getName().toLowerCase().matches("%root%")
}
class SetuidLikeFunctionCall extends FunctionCall {
SetuidLikeFunctionCall() {
(getTarget().hasGlobalName("setuid") or getTarget().hasGlobalName("setresuid")) and
// setuid/setresuid with the root user are false positives.
not argumentMayBeRoot(getArgument(0))
}
}
class SetuidLikeWrapperCall extends FunctionCall {
SetuidLikeFunctionCall baseCall;
SetuidLikeWrapperCall() {
this = baseCall
or
exists(SetuidLikeWrapperCall fc |
this.getTarget() = fc.getEnclosingFunction() and
baseCall = fc.getBaseCall()
)
}
SetuidLikeFunctionCall getBaseCall() { result = baseCall }
}
class CallBeforeSetuidFunctionCall extends FunctionCall {
CallBeforeSetuidFunctionCall() {
(
getTarget().hasGlobalName("setgid") or
getTarget().hasGlobalName("setresgid") or
// Compatibility may require skipping initgroups and setgroups return checks.
// A stricter best practice is to check the result and errnor for EPERM.
getTarget().hasGlobalName("initgroups") or
getTarget().hasGlobalName("setgroups")
) and
// setgid/setresgid/etc with the root group are false positives.
not argumentMayBeRoot(getArgument(0))
}
}
class CallBeforeSetuidWrapperCall extends FunctionCall {
CallBeforeSetuidFunctionCall baseCall;
CallBeforeSetuidWrapperCall() {
this = baseCall
or
exists(CallBeforeSetuidWrapperCall fc |
this.getTarget() = fc.getEnclosingFunction() and
baseCall = fc.getBaseCall()
)
}
CallBeforeSetuidFunctionCall getBaseCall() { result = baseCall }
}
predicate setuidBeforeSetgid(
SetuidLikeWrapperCall setuidWrapper, CallBeforeSetuidWrapperCall setgidWrapper
) {
setgidWrapper.getAPredecessor+() = setuidWrapper
}
predicate isAccessed(FunctionCall fc) {
exists(Variable v | v.getAnAssignedValue() = fc)
or
exists(Operation c | fc = c.getAChild() | c.isCondition())
or
// ignore pattern where result is intentionally ignored by a cast to void.
fc.hasExplicitConversion()
}
from Function func, CallBeforeSetuidFunctionCall fc, SetuidLikeFunctionCall setuid
where
setuidBeforeSetgid(setuid, fc) and
// Require the call return code to be used in a condition or assigned.
// This introduces false negatives where the return is checked but then
// errno == EPERM allows execution to continue.
not isAccessed(fc) and
func = fc.getEnclosingFunction()
select fc,
"This function is called within " + func + ", and potentially after " +
"$@, and may not succeed. Be sure to check the return code and errno, otherwise permissions " +
"may not be dropped.", setuid, setuid.getTarget().getName()

View File

@@ -73,7 +73,8 @@ FunctionDeclarationEntry sourceMethod() {
}
int numberOfSourceMethods(Class c) {
result = count(FunctionDeclarationEntry m |
result =
count(FunctionDeclarationEntry m |
m = sourceMethod() and
m.getFunction().getDeclaringType() = c
)
@@ -126,7 +127,8 @@ predicate similarLines(File f, int line) {
}
private predicate similarLinesPerEquivalenceClass(int equivClass, int lines, File f) {
lines = strictsum(SimilarBlock b, int toSum |
lines =
strictsum(SimilarBlock b, int toSum |
(b.sourceFile() = f and b.getEquivalenceClass() = equivClass) and
toSum = b.sourceLines()
|
@@ -137,7 +139,8 @@ private predicate similarLinesPerEquivalenceClass(int equivClass, int lines, Fil
private predicate similarLinesCoveredFiles(File f, File otherFile) {
exists(int numLines | numLines = f.getMetrics().getNumberOfLines() |
exists(int coveredApprox |
coveredApprox = strictsum(int num |
coveredApprox =
strictsum(int num |
exists(int equivClass |
similarLinesPerEquivalenceClass(equivClass, num, f) and
similarLinesPerEquivalenceClass(equivClass, num, otherFile) and
@@ -153,7 +156,8 @@ predicate similarLinesCovered(File f, int coveredLines, File otherFile) {
exists(int numLines | numLines = f.getMetrics().getNumberOfLines() |
similarLinesCoveredFiles(f, otherFile) and
exists(int notCovered |
notCovered = count(int j |
notCovered =
count(int j |
j in [1 .. numLines] and
not similarLines(f, j)
) and
@@ -169,7 +173,8 @@ predicate duplicateLines(File f, int line) {
}
private predicate duplicateLinesPerEquivalenceClass(int equivClass, int lines, File f) {
lines = strictsum(DuplicateBlock b, int toSum |
lines =
strictsum(DuplicateBlock b, int toSum |
(b.sourceFile() = f and b.getEquivalenceClass() = equivClass) and
toSum = b.sourceLines()
|
@@ -180,7 +185,8 @@ private predicate duplicateLinesPerEquivalenceClass(int equivClass, int lines, F
predicate duplicateLinesCovered(File f, int coveredLines, File otherFile) {
exists(int numLines | numLines = f.getMetrics().getNumberOfLines() |
exists(int coveredApprox |
coveredApprox = strictsum(int num |
coveredApprox =
strictsum(int num |
exists(int equivClass |
duplicateLinesPerEquivalenceClass(equivClass, num, f) and
duplicateLinesPerEquivalenceClass(equivClass, num, otherFile) and
@@ -190,7 +196,8 @@ predicate duplicateLinesCovered(File f, int coveredLines, File otherFile) {
(coveredApprox * 100) / numLines > 75
) and
exists(int notCovered |
notCovered = count(int j |
notCovered =
count(int j |
j in [1 .. numLines] and
not duplicateLines(f, j)
) and
@@ -219,7 +226,8 @@ predicate duplicateFiles(File f, File other, int percent) {
}
predicate mostlyDuplicateClassBase(Class c, Class other, int numDup, int total) {
numDup = strictcount(FunctionDeclarationEntry m1 |
numDup =
strictcount(FunctionDeclarationEntry m1 |
exists(FunctionDeclarationEntry m2 |
duplicateMethod(m1, m2) and
m1 = sourceMethod() and

View File

@@ -27,7 +27,8 @@ class DefectResult extends int {
string getMessage() { defectResults(this, _, _, _, _, _, _, result) }
string getURL() {
result = "file://" + getFile().getAbsolutePath() + ":" + getStartLine() + ":" + getStartColumn()
+ ":" + getEndLine() + ":" + getEndColumn()
result =
"file://" + getFile().getAbsolutePath() + ":" + getStartLine() + ":" + getStartColumn() + ":" +
getEndLine() + ":" + getEndColumn()
}
}

View File

@@ -37,7 +37,8 @@ class MetricResult extends int {
float getValue() { metricResults(this, _, _, _, _, _, _, result) }
string getURL() {
result = "file://" + getFile().getAbsolutePath() + ":" + getStartLine() + ":" + getStartColumn()
+ ":" + getEndLine() + ":" + getEndColumn()
result =
"file://" + getFile().getAbsolutePath() + ":" + getStartLine() + ":" + getStartColumn() + ":" +
getEndLine() + ":" + getEndColumn()
}
}

View File

@@ -73,13 +73,13 @@ string extraDetail(
else
if strictcount(possibleGuard(hf, _)) = 1
then
result = " (" + possibleGuard(hf, _) + " should appear in the #ifndef rather than " + s +
")."
result =
" (" + possibleGuard(hf, _) + " should appear in the #ifndef rather than " + s + ")."
else
if strictcount(possibleGuard(hf, "")) = 1
then
result = " (" + possibleGuard(hf, "") + " should appear in the #ifndef rather than " + s
+ ")."
result =
" (" + possibleGuard(hf, "") + " should appear in the #ifndef rather than " + s + ")."
else result = " (the macro " + s + " is checked for, but is not defined)."
)
}

View File

@@ -30,7 +30,8 @@ predicate canonicalName1(Declaration d, string canonical) {
}
predicate canonicalName2(Declaration d, string canonical) {
canonical = d
canonical =
d
.getName()
.replaceAll("_", "")
.replaceAll("0", "O")

View File

@@ -64,18 +64,22 @@ from ProperClass c, string msg
where
not definesDefaultConstructor(c) and
not c.hasConstructor() and
msg = "AV Rule 68: class " + c.getName() +
msg =
"AV Rule 68: class " + c.getName() +
" does not need a default constructor and should explicitly disallow it."
or
not definesCopyConstructor(c) and
msg = "AV Rule 68: class " + c.getName() +
msg =
"AV Rule 68: class " + c.getName() +
" does not need a copy constructor and should explicitly disallow it."
or
not definesCopyAssignmentOperator(c) and
msg = "AV Rule 68: class " + c.getName() +
msg =
"AV Rule 68: class " + c.getName() +
" does not need a copy assignment operator and should explicitly disallow it."
or
not definesDestructor(c) and
msg = "AV Rule 68: class " + c.getName() +
msg =
"AV Rule 68: class " + c.getName() +
" does not need a destructor and should explicitly disallow it."
select c, msg

View File

@@ -68,24 +68,24 @@ where
(
(
virtualThisCall(call, overridingFunction) and
explanation = "Call to virtual function $@ which is overridden in $@. If you intend to statically call this virtual function, it should be qualified with "
explanation =
"Call to virtual function $@ which is overridden in $@. If you intend to statically call this virtual function, it should be qualified with "
+ virtFunction.getDeclaringType().toString() + "::."
) and
virtFunction = call.getTarget() and
overridingFunction.getDeclaringType().getABaseClass+() = call
.getEnclosingFunction()
.getDeclaringType()
overridingFunction.getDeclaringType().getABaseClass+() =
call.getEnclosingFunction().getDeclaringType()
or
exists(VirtualFunction target |
thisCall(call) and indirectlyCallsVirtualFunction(call.getTarget(), target, _)
|
explanation = "Call to function " + call.getTarget().getName() +
explanation =
"Call to function " + call.getTarget().getName() +
" that calls virtual function $@ (overridden in $@)." and
virtFunction = target and
overridingFunction = target.getAnOverridingFunction() and
overridingFunction.getDeclaringType().getABaseClass+() = call
.getEnclosingFunction()
.getDeclaringType()
overridingFunction.getDeclaringType().getABaseClass+() =
call.getEnclosingFunction().getDeclaringType()
)
)
select call, explanation, virtFunction, virtFunction.getName(), overridingFunction,

View File

@@ -282,15 +282,16 @@ where
(
exists(Function releaseFunction, int releaseLine |
badRelease(r, acquire, releaseFunction, releaseLine) and
message = "Resource " + r.getName() + " is acquired by class " +
r.getDeclaringType().getName() +
message =
"Resource " + r.getName() + " is acquired by class " + r.getDeclaringType().getName() +
" but not released in the destructor. It is released from " + releaseFunction.getName() +
" on line " + releaseLine +
", so this function may need to be called from the destructor."
)
or
not badRelease(r, _, _, _) and
message = "Resource " + r.getName() + " is acquired by class " + r.getDeclaringType().getName() +
message =
"Resource " + r.getName() + " is acquired by class " + r.getDeclaringType().getName() +
" but not released anywhere in this class."
) and
not automaticallyReleased(acquire) and

View File

@@ -62,8 +62,8 @@ class ReferenceCopyAssignmentOperator extends MemberFunction {
/** A call to delete on a member variable */
DeleteExpr getADeleteExpr() {
result.getEnclosingFunction() = this and
result.getExpr().(VariableAccess).getTarget().(MemberVariable).getDeclaringType() = this
.getDeclaringType()
result.getExpr().(VariableAccess).getTarget().(MemberVariable).getDeclaringType() =
this.getDeclaringType()
}
}

View File

@@ -76,7 +76,8 @@ predicate assignOperatorWithWrongType(Operator op, string msg) {
exists(Class c |
c = op.getDeclaringType() and
op.getUnspecifiedType() = c and
msg = "Assignment operator in class " + c.getName() + " should have return type " + c.getName() +
msg =
"Assignment operator in class " + c.getName() + " should have return type " + c.getName() +
"&. Otherwise a copy is created at each call."
)
}
@@ -87,7 +88,8 @@ predicate assignOperatorWithWrongResult(Operator op, string msg) {
exists(op.getBlock()) and
not op.getType() instanceof VoidType and
not assignOperatorWithWrongType(op, _) and
msg = "Assignment operator in class " + op.getDeclaringType().getName() +
msg =
"Assignment operator in class " + op.getDeclaringType().getName() +
" does not return a reference to *this."
}

View File

@@ -45,13 +45,15 @@ class InterfaceImplementor extends Class {
}
int getNumInterfaces() {
result = count(ClassDerivation d |
result =
count(ClassDerivation d |
d.getDerivedClass() = this and d.getBaseClass() instanceof InterfaceClass
)
}
int getNumProtectedImplementations() {
result = count(ClassDerivation d |
result =
count(ClassDerivation d |
d.hasSpecifier("protected") and
d.getDerivedClass() = this and
not d.getBaseClass() instanceof InterfaceClass
@@ -59,7 +61,8 @@ class InterfaceImplementor extends Class {
}
int getNumPrivateImplementations() {
result = count(ClassDerivation d |
result =
count(ClassDerivation d |
d.hasSpecifier("private") and
d.getDerivedClass() = this and
not d.getBaseClass() instanceof InterfaceClass
@@ -67,7 +70,8 @@ class InterfaceImplementor extends Class {
}
int getNumPublicImplementations() {
result = count(ClassDerivation d |
result =
count(ClassDerivation d |
d.hasSpecifier("public") and
d.getDerivedClass() = this and
not d.getBaseClass() instanceof InterfaceClass

View File

@@ -14,7 +14,8 @@ import cpp
// Pick a representative file for a namespace - more than a bit dodgy, but otherwise
// the results don't show up anywhere which is less than helpful
predicate namespaceRepresentative(Namespace ns, File rep) {
rep.getAbsolutePath() = min(File f |
rep.getAbsolutePath() =
min(File f |
exists(Declaration d | d = ns.getADeclaration() | d.getFile() = f)
|
f.getAbsolutePath()

View File

@@ -64,6 +64,7 @@ where
reachable(blame) and
not functionImperfectlyExtracted(f) and
(blame = stmt or blame.(Expr).getEnclosingStmt() = stmt) and
msg = "Function " + f.getName() + " should return a value of type " + f.getType().getName() +
msg =
"Function " + f.getName() + " should return a value of type " + f.getType().getName() +
" but does not return a value here"
select stmt, msg

Some files were not shown because too many files have changed in this diff Show More