diff --git a/.codeqlmanifest.json b/.codeqlmanifest.json index 30efc842b00..81d370e793d 100644 --- a/.codeqlmanifest.json +++ b/.codeqlmanifest.json @@ -1,5 +1,6 @@ { "provide": [ "*/ql/src/qlpack.yml", "*/ql/test/qlpack.yml", + "cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/tainted/qlpack.yml", "*/ql/examples/qlpack.yml", "*/upgrades/qlpack.yml", "misc/legacy-support/*/qlpack.yml", diff --git a/.github/workflows/check-change-note.yml b/.github/workflows/check-change-note.yml index 0b90ffbc668..8c036c2f1b3 100644 --- a/.github/workflows/check-change-note.yml +++ b/.github/workflows/check-change-note.yml @@ -19,5 +19,5 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | - gh api 'repos/${{github.repository}}/pulls/${{github.event.number}}/files' --paginate | - jq 'any(.[].filename ; test("/change-notes/.*[.]md$"))' --exit-status + gh api 'repos/${{github.repository}}/pulls/${{github.event.number}}/files' --paginate --jq 'any(.[].filename ; test("/change-notes/.*[.]md$"))' | + grep true -c diff --git a/.github/workflows/close-stale.yml b/.github/workflows/close-stale.yml new file mode 100644 index 00000000000..c63a6be690c --- /dev/null +++ b/.github/workflows/close-stale.yml @@ -0,0 +1,30 @@ +name: Mark stale issues + +on: + workflow_dispatch: + schedule: + - cron: "30 1 * * *" + +jobs: + stale: + if: github.repository == 'github/codeql' + + runs-on: ubuntu-latest + + steps: + - uses: actions/stale@v3 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + stale-issue-message: 'This issue is stale because it has been open 14 days with no activity. Comment or remove the `Stale` label in order to avoid having this issue closed in 7 days.' + close-issue-message: 'This issue was closed because it has been inactive for 7 days.' + days-before-stale: 14 + days-before-close: 7 + only-labels: awaiting-response + + # do not mark PRs as stale + days-before-pr-stale: -1 + days-before-pr-close: -1 + + # Uncomment for dry-run + # debug-only: true + # operations-per-run: 1000 diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index b86009ef6da..87d6632d03e 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -19,13 +19,18 @@ jobs: runs-on: ubuntu-latest + permissions: + contents: read + security-events: write + pull-requests: read + steps: - name: Checkout repository uses: actions/checkout@v2 # Initializes the CodeQL tools for scanning. - name: Initialize CodeQL - uses: github/codeql-action/init@v1 + uses: github/codeql-action/init@main # Override language selection by uncommenting this and choosing your languages with: languages: csharp @@ -34,7 +39,7 @@ jobs: # Autobuild attempts to build any compiled languages (C/C++, C#, or Java). # If this step fails, then you should remove it and run the build manually (see below) - name: Autobuild - uses: github/codeql-action/autobuild@v1 + uses: github/codeql-action/autobuild@main # ℹ️ Command-line programs to run using the OS shell. # 📚 https://git.io/JvXDl @@ -48,4 +53,4 @@ jobs: # make release - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v1 + uses: github/codeql-action/analyze@main diff --git a/.github/workflows/csv-coverage.yml b/.github/workflows/csv-coverage.yml new file mode 100644 index 00000000000..54c172d66d3 --- /dev/null +++ b/.github/workflows/csv-coverage.yml @@ -0,0 +1,77 @@ +name: Build/check CSV flow coverage report + +on: + workflow_dispatch: + inputs: + qlModelShaOverride: + description: 'github/codeql repo SHA used for looking up the CSV models' + required: false + push: + branches: + - main + - 'rc/**' + pull_request: + paths: + - '.github/workflows/csv-coverage.yml' + - '*/ql/src/**/*.ql' + - '*/ql/src/**/*.qll' + - 'misc/scripts/library-coverage/*.py' + # input data files + - '*/documentation/library-coverage/cwe-sink.csv' + - '*/documentation/library-coverage/frameworks.csv' + # coverage report files + - '*/documentation/library-coverage/flow-model-coverage.csv' + - '*/documentation/library-coverage/flow-model-coverage.rst' + +jobs: + build: + + runs-on: ubuntu-latest + + steps: + - name: Clone self (github/codeql) + uses: actions/checkout@v2 + with: + path: script + - name: Clone self (github/codeql) at a given SHA for analysis + if: github.event.inputs.qlModelShaOverride != '' + uses: actions/checkout@v2 + with: + path: codeqlModels + ref: github.event.inputs.qlModelShaOverride + - name: Clone self (github/codeql) for analysis + if: github.event.inputs.qlModelShaOverride == '' + uses: actions/checkout@v2 + with: + path: codeqlModels + - name: Set up Python 3.8 + uses: actions/setup-python@v2 + with: + python-version: 3.8 + - name: Download CodeQL CLI + uses: dsaltares/fetch-gh-release-asset@aa37ae5c44d3c9820bc12fe675e8670ecd93bd1c + with: + repo: "github/codeql-cli-binaries" + version: "latest" + file: "codeql-linux64.zip" + token: ${{ secrets.GITHUB_TOKEN }} + - name: Unzip CodeQL CLI + run: unzip -d codeql-cli codeql-linux64.zip + - name: Build modeled package list + run: | + PATH="$PATH:codeql-cli/codeql" python script/misc/scripts/library-coverage/generate-report.py ci codeqlModels script + - name: Upload CSV package list + uses: actions/upload-artifact@v2 + with: + name: csv-flow-model-coverage + path: flow-model-coverage-*.csv + - name: Upload RST package list + uses: actions/upload-artifact@v2 + with: + name: rst-flow-model-coverage + path: flow-model-coverage-*.rst + # - name: Check coverage files + # if: github.event.pull_request + # run: | + # python script/misc/scripts/library-coverage/compare-files.py codeqlModels + diff --git a/config/identical-files.json b/config/identical-files.json index 6c1c0c7409d..582e4c7b6dc 100644 --- a/config/identical-files.json +++ b/config/identical-files.json @@ -5,6 +5,7 @@ "java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl3.qll", "java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl4.qll", "java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl5.qll", + "java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl6.qll", "cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl.qll", "cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll", "cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll", @@ -56,6 +57,10 @@ "csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImplConsistency.qll", "python/ql/src/semmle/python/dataflow/new/internal/DataFlowImplConsistency.qll" ], + "DataFlow Java/C# Flow Summaries": [ + "java/ql/src/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll", + "csharp/ql/src/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll" + ], "SsaReadPosition Java/C#": [ "java/ql/src/semmle/code/java/dataflow/internal/rangeanalysis/SsaReadPositionCommon.qll", "csharp/ql/src/semmle/code/csharp/dataflow/internal/rangeanalysis/SsaReadPositionCommon.qll" @@ -245,6 +250,10 @@ "cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysis.qll", "csharp/ql/src/experimental/ir/implementation/unaliased_ssa/internal/AliasAnalysis.qll" ], + "SSA PrintAliasAnalysis": [ + "cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/PrintAliasAnalysis.qll", + "cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/PrintAliasAnalysis.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" @@ -434,6 +443,10 @@ ], "CryptoAlgorithms Python/JS": [ "javascript/ql/src/semmle/javascript/security/CryptoAlgorithms.qll", - "python/ql/src/semmle/crypto/Crypto.qll" + "python/ql/src/semmle/python/concepts/CryptoAlgorithms.qll" + ], + "SensitiveDataHeuristics Python/JS": [ + "javascript/ql/src/semmle/javascript/security/internal/SensitiveDataHeuristics.qll", + "python/ql/src/semmle/python/security/internal/SensitiveDataHeuristics.qll" ] -} \ No newline at end of file +} diff --git a/cpp/change-notes/2021-03-11-overflow-abs.md b/cpp/change-notes/2021-03-11-overflow-abs.md new file mode 100644 index 00000000000..66854412f72 --- /dev/null +++ b/cpp/change-notes/2021-03-11-overflow-abs.md @@ -0,0 +1,2 @@ +lgtm +* The `cpp/tainted-arithmetic`, `cpp/arithmetic-with-extreme-values`, and `cpp/uncontrolled-arithmetic` queries now recognize more functions as returning the absolute value of their input. As a result, they produce fewer false positives. diff --git a/cpp/change-notes/2021-04-06-assign-where-compare-meant.md b/cpp/change-notes/2021-04-06-assign-where-compare-meant.md new file mode 100644 index 00000000000..e540593b15a --- /dev/null +++ b/cpp/change-notes/2021-04-06-assign-where-compare-meant.md @@ -0,0 +1,2 @@ +lgtm,codescanning +* The 'Assignment where comparison was intended' (cpp/assign-where-compare-meant) query has been improved to flag fewer benign assignments in conditionals. diff --git a/cpp/change-notes/2021-04-09-unsigned-difference-expression-compared-zero.md b/cpp/change-notes/2021-04-09-unsigned-difference-expression-compared-zero.md new file mode 100644 index 00000000000..3297dfce9a9 --- /dev/null +++ b/cpp/change-notes/2021-04-09-unsigned-difference-expression-compared-zero.md @@ -0,0 +1,2 @@ +lgtm,codescanning +* The 'Unsigned difference expression compared to zero' (cpp/unsigned-difference-expression-compared-zero) query has been improved to produce fewer false positive results. \ No newline at end of file diff --git a/cpp/change-notes/2021-04-13-arithmetic-queries.md b/cpp/change-notes/2021-04-13-arithmetic-queries.md new file mode 100644 index 00000000000..4d0f8833adc --- /dev/null +++ b/cpp/change-notes/2021-04-13-arithmetic-queries.md @@ -0,0 +1,2 @@ +lgtm +* The queries cpp/tainted-arithmetic, cpp/uncontrolled-arithmetic, and cpp/arithmetic-with-extreme-values have been improved to produce fewer false positives. diff --git a/cpp/change-notes/2021-04-21-return-stack-allocated-object.md b/cpp/change-notes/2021-04-21-return-stack-allocated-object.md new file mode 100644 index 00000000000..1876f4cf5f7 --- /dev/null +++ b/cpp/change-notes/2021-04-21-return-stack-allocated-object.md @@ -0,0 +1,2 @@ +codescanning +* The 'Pointer to stack object used as return value' (cpp/return-stack-allocated-object) query has been deprecated, and any uses should be replaced with `Returning stack-allocated memory` (cpp/return-stack-allocated-memory). \ No newline at end of file diff --git a/cpp/change-notes/2021-04-26-more-sound-expr-might-overflow.md b/cpp/change-notes/2021-04-26-more-sound-expr-might-overflow.md new file mode 100644 index 00000000000..5a7b8414fad --- /dev/null +++ b/cpp/change-notes/2021-04-26-more-sound-expr-might-overflow.md @@ -0,0 +1,2 @@ +lgtm,codescanning +* The `exprMightOverflowPositively` and `exprMightOverflowNegatively` predicates from the `SimpleRangeAnalysis` library now recognize more expressions that might overflow. diff --git a/cpp/change-notes/2021-05-10-comparison-with-wider-type.md b/cpp/change-notes/2021-05-10-comparison-with-wider-type.md new file mode 100644 index 00000000000..f06895a3c1a --- /dev/null +++ b/cpp/change-notes/2021-05-10-comparison-with-wider-type.md @@ -0,0 +1,2 @@ +lgtm,codescanning +* The 'Comparison with wider type' (cpp/comparison-with-wider-type) query has been improved to produce fewer false positives. \ No newline at end of file diff --git a/cpp/change-notes/2021-05-12-uncontrolled-arithmetic.md b/cpp/change-notes/2021-05-12-uncontrolled-arithmetic.md new file mode 100644 index 00000000000..56fbc9a44ce --- /dev/null +++ b/cpp/change-notes/2021-05-12-uncontrolled-arithmetic.md @@ -0,0 +1,2 @@ +lgtm,codescanning +* The query "Uncontrolled arithmetic" (`cpp/uncontrolled-arithmetic`) has been improved to produce fewer false positives. diff --git a/cpp/change-notes/2021-05-14-uncontrolled-allocation-size.md b/cpp/change-notes/2021-05-14-uncontrolled-allocation-size.md new file mode 100644 index 00000000000..6f0c9d6fa98 --- /dev/null +++ b/cpp/change-notes/2021-05-14-uncontrolled-allocation-size.md @@ -0,0 +1,2 @@ +lgtm +* The "Tainted allocation size" query (cpp/uncontrolled-allocation-size) has been improved to produce fewer false positives. diff --git a/cpp/change-notes/2021-05-18-static-buffer-overflow.md b/cpp/change-notes/2021-05-18-static-buffer-overflow.md new file mode 100644 index 00000000000..f56040fe8aa --- /dev/null +++ b/cpp/change-notes/2021-05-18-static-buffer-overflow.md @@ -0,0 +1,2 @@ +lgtm +* The "Static buffer overflow" query (cpp/static-buffer-overflow) has been improved to produce fewer false positives. diff --git a/cpp/change-notes/2021-05-19-weak-cryptographic-algorithm.md b/cpp/change-notes/2021-05-19-weak-cryptographic-algorithm.md new file mode 100644 index 00000000000..5e48ba166b7 --- /dev/null +++ b/cpp/change-notes/2021-05-19-weak-cryptographic-algorithm.md @@ -0,0 +1,2 @@ +lgtm,codescanning +* The "Use of a broken or risky cryptographic algorithm" (`cpp/weak-cryptographic-algorithm`) query has been enhanced to reduce false positive results, and (rarely) find more true positive results. diff --git a/cpp/change-notes/2021-05-20-incorrect-allocation-error-handling.md b/cpp/change-notes/2021-05-20-incorrect-allocation-error-handling.md new file mode 100644 index 00000000000..f00856ed711 --- /dev/null +++ b/cpp/change-notes/2021-05-20-incorrect-allocation-error-handling.md @@ -0,0 +1,2 @@ +lgtm +* A new query (`cpp/incorrect-allocation-error-handling`) has been added. The query finds incorrect error-handling of calls to `operator new`. This query was originally [submitted as an experimental query by @ihsinme](https://github.com/github/codeql/pull/5010). diff --git a/cpp/change-notes/2021-05-20-ref-qualifiers.md b/cpp/change-notes/2021-05-20-ref-qualifiers.md new file mode 100644 index 00000000000..e4b394a2173 --- /dev/null +++ b/cpp/change-notes/2021-05-20-ref-qualifiers.md @@ -0,0 +1,2 @@ +lgtm,codescanning +* lvalue/rvalue ref qualifiers are now accessible via the new predicates on `MemberFunction`(`.isLValueRefQualified`, `.isRValueRefQualified`, and `isRefQualified`). diff --git a/cpp/change-notes/2021-05-21-unsafe-strncat.md b/cpp/change-notes/2021-05-21-unsafe-strncat.md new file mode 100644 index 00000000000..135b5278df4 --- /dev/null +++ b/cpp/change-notes/2021-05-21-unsafe-strncat.md @@ -0,0 +1,2 @@ +lgtm +* The "Potentially unsafe call to strncat" query (cpp/unsafe-strncat) query has been improved to detect more cases of unsafe calls to `strncat`. diff --git a/cpp/change-notes/2021-06-10-std-types.md b/cpp/change-notes/2021-06-10-std-types.md new file mode 100644 index 00000000000..86577ce14a0 --- /dev/null +++ b/cpp/change-notes/2021-06-10-std-types.md @@ -0,0 +1,4 @@ +lgtm,codescanning +* Added definitions for types found in `cstdint`. Added types `FixedWidthIntegralType`, `MinimumWidthIntegralType`, `FastestMinimumWidthIntegralType`, and `MaximumWidthIntegralType` to describe types such as `int8_t`, `int_least8_t`, `int_fast8_t`, and `intmax_t` respectively. +* Changed definition of `Intmax_t` and `Uintmax_t` to be part of the new type structure. +* Added a type `FixedWidthEnumType` which describes enums based on a fixed-width integer type. For instance, `enum e: uint8_t = { a, b };`. diff --git a/cpp/ql/src/Best Practices/Likely Errors/OffsetUseBeforeRangeCheck.ql b/cpp/ql/src/Best Practices/Likely Errors/OffsetUseBeforeRangeCheck.ql index 36024ddbc70..ecf739b91be 100644 --- a/cpp/ql/src/Best Practices/Likely Errors/OffsetUseBeforeRangeCheck.ql +++ b/cpp/ql/src/Best Practices/Likely Errors/OffsetUseBeforeRangeCheck.ql @@ -5,6 +5,7 @@ * @kind problem * @id cpp/offset-use-before-range-check * @problem.severity warning + * @security-severity 5.9 * @precision medium * @tags reliability * security diff --git a/cpp/ql/src/Best Practices/Magic Constants/MagicConstantsNumbers.qhelp b/cpp/ql/src/Best Practices/Magic Constants/MagicConstantsNumbers.qhelp index 16fc75fc7ad..aa97965996f 100644 --- a/cpp/ql/src/Best Practices/Magic Constants/MagicConstantsNumbers.qhelp +++ b/cpp/ql/src/Best Practices/Magic Constants/MagicConstantsNumbers.qhelp @@ -39,7 +39,7 @@ then replace all the relevant occurrences in the code.
This rule finds calls to a function that ignore the return value. A function call is only marked
-as a violation if at least 80% of the total calls to that function check the return value. Not
+as a violation if at least 90% of the total calls to that function check the return value. Not
checking a return value is a common source of defects from standard library functions like The standard library function malloc or fread.
These functions return the status information and the return values should always be checked
to see if the operation succeeded before operating on any data modified or resources allocated by these functions.
@@ -32,7 +32,7 @@ Check the return value of functions that return status information.
switch statemen
MSDN Library: switch statement (C++)
strncat appends a source string to a target string.
-The third argument defines the maximum number of characters to append and should be less than or equal to the remaining space in the destination buffer. Calls of the form strncat(dest, src, strlen(dest)) or strncat(dest, src, sizeof(dest)) set the third argument to the entire size of the destination buffer. Executing a call of this type may cause a buffer overflow unless the buffer is known to be empty. Buffer overflows can lead to anything from a segmentation fault to a security vulnerability.
Calls of the form strncat(dest, src, strlen(dest)) or strncat(dest, src, sizeof(dest)) set
+the third argument to the entire size of the destination buffer.
+Executing a call of this type may cause a buffer overflow unless the buffer is known to be empty.
Similarly, calls of the form strncat(dest, src, sizeof (dest) - strlen (dest)) allow one
+byte to be written ouside the dest buffer.
Buffer overflows can lead to anything from a segmentation fault to a security vulnerability.
Different overloads of the new operator handle allocation failures in different ways.
+If new T fails for some type T, it throws a std::bad_alloc exception,
+but new(std::nothrow) T returns a null pointer. If the programmer does not use the corresponding
+method of error handling, allocation failure may go unhandled and could cause the program to behave in
+unexpected ways.
Make sure that exceptions are handled appropriately if new T is used. On the other hand,
+make sure to handle the possibility of null pointers if new(std::nothrow) T is used.
The code passes user input as part of a SQL query without escaping special elements.
+It generates a SQL query to Postgres using sprintf,
+with the user-supplied data directly passed as an argument
+to sprintf. This leaves the code vulnerable to attack by SQL Injection.
Use a library routine to escape characters in the user-supplied
+string before converting it to SQL. Use esc and quote pqxx library functions.
Using variables with the same name is dangerous. However, such a situation inside the while loop can create an infinite loop exhausting resources. Requires the attention of developers.
+ +We recommend not to use local variables inside a loop if their names are the same as the variables in the condition of this loop.
+ +The following example demonstrates an erroneous and corrected use of a local variable within a loop.
+Freeing a previously allocated resource twice can lead to various vulnerabilities in the program.
+ +We recommend that you exclude situations of possible double release. For example, use the assignment NULL to a freed variable.
+ +The following example demonstrates an erroneous and corrected use of freeing a pointer.
+When using the new operator to allocate memory, you need to pay attention to the different ways of detecting errors. ::operator new(std::size_t) throws an exception on error, whereas ::operator new(std::size_t, const std::nothrow_t &) returns zero on error. The programmer can get confused and check the error that occurs when allocating memory incorrectly. That can lead to an unhandled program termination or to a violation of the program logic.
Use the correct error detection method corresponding with the memory allocation.
- -The following example demonstrates various approaches to detecting memory allocation errors using the new operator.
In some situations, after code refactoring, parts of the old constructs may remain. They are correctly accepted by the compiler, but can critically affect program execution. For example, if you switch from `do {...} while ();` to `while () {...}` forgetting to remove the old construct completely, you get `while(){...}while();` which may be vulnerable. These code snippets look suspicious and require the developer's attention.
+ + +We recommend that you use more explicit code transformations.
+ +The following example demonstrates the erroneous and corrected sections of the code.
+Using bitwise operations can be a mistake in some situations. For example, if parameters are evaluated in an expression and the function should be called only upon certain test results. These bitwise operations look suspicious and require developer attention.
+ + +We recommend that you evaluate the correctness of using the specified bit operations.
+ +The following example demonstrates the erroneous and fixed use of bit and logical operations.
+Finding places of confusing use of boolean type. For example, a unary minus does not work before a boolean type and an increment always gives true.
+ + +we recommend making the code simpler.
+ +The following example demonstrates erroneous and fixed methods for using a boolean data type.
+The standard library function strncat(dest, source, count) appends the source string to the dest string. count specifies the maximum number of characters to append and must be less than the remaining space in the target buffer. Calls of the form strncat (dest, source, sizeof (dest) - strlen (dest)) set the third argument to one more than possible. So when the dest is full, the expression sizeof (dest) - strlen (dest) will be equal to one, and not zero as the programmer might think. Making a call of this type may result in a zero byte being written just outside the dest buffer.
We recommend subtracting one from the third argument. For example, replace strncat(dest, source, sizeof(dest)-strlen(dest)) with strncat(dest, source, sizeof(dest)-strlen(dest)-1).
The following example demonstrates an erroneous and corrected use of the strncat function.