Add change note for Spring StringUtils

Approved by esbena

.github/workflows/check-change-note.yml

@@ -0,0 +1,23 @@
+name: Check change note
+
+on:
+  pull_request_target:
+    types: [labeled, unlabeled, opened, synchronize, reopened, ready_for_review]
+    paths:
+      - "*/ql/src/**/*.ql"
+      - "*/ql/src/**/*.qll"
+      - "!**/experimental/**"
+
+jobs:
+  check-change-note:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Fail if no change note found. To fix, either add one, or add the `no-change-note-required` label.
+        if: |
+          github.event.pull_request.draft == false &&
+          !contains(github.event.pull_request.labels.*.name, 'no-change-note-required')
+        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

.github/workflows/codeql-analysis.yml

@@ -2,7 +2,15 @@ name: "Code scanning - action"
 
 on:
   push:
+    branches:
+      - main
+      - 'rc/*'
   pull_request:
+    branches:
+      - main
+      - 'rc/*'
+    paths:
+      - 'csharp/**'
   schedule:
     - cron: '0 9 * * 1'
 
@@ -14,16 +22,7 @@ jobs:
     steps:
     - name: Checkout repository
       uses: actions/checkout@v2
-      with:
-        # We must fetch at least the immediate parents so that if this is
-        # a pull request then we can checkout the head.
-        fetch-depth: 2
 
-    # If this run was triggered by a pull request event, then checkout
-    # the head of the pull request instead of the merge commit.
-    - run: git checkout HEAD^2
-      if: ${{ github.event_name == 'pull_request' }}
-      
     # Initializes the CodeQL tools for scanning.
     - name: Initialize CodeQL
       uses: github/codeql-action/init@v1

.github/workflows/generate-query-help-docs.yml

@@ -1,60 +0,0 @@
-name: Generate CodeQL query help documentation using Sphinx
-
-on:
-  workflow_dispatch:
-      inputs:
-        description:
-          description: A description of the purpose of this job. For human consumption.
-          required: false
-  push:
-    branches:
-     - 'lgtm.com'
-  pull_request:
-    paths:
-      - '.github/workflows/generate-query-help-docs.yml'
-      - 'docs/codeql/query-help/**'
-      
-jobs:
-  build:
-    runs-on: ubuntu-latest
-    steps:
-    - name: Clone github/codeql
-      uses: actions/checkout@v2
-      with:
-        path: codeql
-    - name: Clone github/codeql-go
-      uses: actions/checkout@v2
-      with:
-        repository: 'github/codeql-go'
-        path: codeql-go
-    - 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: Set up query help docs folder  
-      run: |
-        cp -r codeql/docs/codeql/** .
-    - name: Query help to markdown
-      run: |
-        PATH="$PATH:codeql-cli/codeql" python codeql/docs/codeql/query-help-markdown.py
-    - name: Run Sphinx for query help
-      uses: ammaraskar/sphinx-action@8b4f60114d7fd1faeba1a712269168508d4750d2 # v0.4
-      with:
-        docs-folder: "query-help/"
-        pre-build-command: "python -m pip install --upgrade recommonmark"
-        build-command: "sphinx-build -b dirhtml . _build" 
-    - name: Upload HTML artifacts
-      uses: actions/upload-artifact@v2
-      with:
-        name: query-help-html
-        path: query-help/_build
-    

.gitignore

@@ -17,6 +17,9 @@
 # Byte-compiled python files
 *.pyc
 
+# python virtual environment folder 
+.venv/
+
 # It's useful (though not required) to be able to unpack codeql in the ql checkout itself
 /codeql/
 

.vscode/settings.json

@@ -1,3 +1,3 @@
 {
     "omnisharp.autoStart": false
-}
+}

CODEOWNERS

@@ -4,17 +4,16 @@
 /javascript/ @github/codeql-javascript
 /python/ @github/codeql-python
 
-# Assign query help for docs review
-/cpp/**/*.qhelp @hubwriter
-/csharp/**/*.qhelp @jf205
-/java/**/*.qhelp @felicitymay
-/javascript/**/*.qhelp @mchammer01
-/python/**/*.qhelp @felicitymay
-/docs/language/ @shati-patel @jf205
+# Make @xcorail (GitHub Security Lab) a code owner for experimental queries so he gets pinged when we promote a query out of experimental
+/cpp/**/experimental/**/* @github/codeql-c-analysis @xcorail
+/csharp/**/experimental/**/* @github/codeql-csharp @xcorail
+/java/**/experimental/**/* @github/codeql-java @xcorail
+/javascript/**/experimental/**/* @github/codeql-javascript @xcorail
+/python/**/experimental/**/* @github/codeql-python @xcorail
 
-# Exclude help for experimental queries from docs review
-/cpp/**/experimental/**/*.qhelp @github/codeql-c-analysis
-/csharp/**/experimental/**/*.qhelp @github/codeql-csharp
-/java/**/experimental/**/*.qhelp @github/codeql-java
-/javascript/**/experimental/**/*.qhelp @github/codeql-javascript
-/python/**/experimental/**/*.qhelp @github/codeql-python
+# Notify members of codeql-go about PRs to the shared data-flow library files
+/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl.qll @github/codeql-java @github/codeql-go
+/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl2.qll @github/codeql-java @github/codeql-go
+/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll @github/codeql-java @github/codeql-go
+/java/ql/src/semmle/code/java/dataflow/internal/tainttracking1/TaintTrackingImpl.qll @github/codeql-java @github/codeql-go
+/java/ql/src/semmle/code/java/dataflow/internal/tainttracking2/TaintTrackingImpl.qll @github/codeql-java @github/codeql-go

CONTRIBUTING.md

@@ -38,7 +38,7 @@ If you have an idea for a query that you would like to share with other CodeQL u
 
     - The queries and libraries must be autoformatted, for example using the "Format Document" command in [CodeQL for Visual Studio Code](https://help.semmle.com/codeql/codeql-for-vscode/procedures/about-codeql-for-vscode.html).
 
-    If you prefer, you can use this [pre-commit hook](misc/scripts/pre-commit) that automatically checks whether your files are correctly formatted. See the [pre-commit hook installation guide](docs/install-pre-commit-hook.md) for instructions on how to install the hook.
+    If you prefer, you can use this [pre-commit hook](misc/scripts/pre-commit) that automatically checks whether your files are correctly formatted. See the [pre-commit hook installation guide](docs/pre-commit-hook-setup.md) for instructions on how to install the hook.
 
 4. **Compilation**
 
@@ -49,7 +49,11 @@ If you have an idea for a query that you would like to share with other CodeQL u
 
     - The query must have at least one true positive result on some revision of a real project.
 
-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.
+6. **Query help files and unit tests**
+
+	- Query help (`.qhelp`) files and unit tests are optional (but strongly encouraged!) for queries in the `experimental` directories. For more information about contributing query help files and unit tests, see [Supported CodeQL queries and libraries](docs/supported-queries.md).
+
+Experimental queries and libraries may not be actively maintained as the supported 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).
 

config/identical-files.json

@@ -36,6 +36,7 @@
     "cpp/ql/src/semmle/code/cpp/dataflow/internal/tainttracking2/TaintTrackingImpl.qll",
     "cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/tainttracking1/TaintTrackingImpl.qll",
     "cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/tainttracking2/TaintTrackingImpl.qll",
+    "cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/tainttracking3/TaintTrackingImpl.qll",
     "csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking1/TaintTrackingImpl.qll",
     "csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking2/TaintTrackingImpl.qll",
     "csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking3/TaintTrackingImpl.qll",
@@ -356,6 +357,7 @@
   ],
   "Inline Test Expectations": [
     "cpp/ql/test/TestUtilities/InlineExpectationsTest.qll",
+    "java/ql/test/TestUtilities/InlineExpectationsTest.qll",
     "python/ql/test/TestUtilities/InlineExpectationsTest.qll"
   ],
   "C++ ExternalAPIs": [
@@ -373,50 +375,49 @@
     "javascript/ql/src/semmle/javascript/XML.qll",
     "python/ql/src/semmle/python/xml/XML.qll"
   ],
-  "DuplicationProblems.qhelp": [
-    "cpp/ql/src/Metrics/Files/DuplicationProblems.qhelp",
-    "csharp/ql/src/Metrics/Files/DuplicationProblems.qhelp",
-    "javascript/ql/src/Metrics/DuplicationProblems.qhelp",
-    "python/ql/src/Metrics/DuplicationProblems.qhelp"
+  "DuplicationProblems.inc.qhelp": [
+    "cpp/ql/src/Metrics/Files/DuplicationProblems.inc.qhelp",
+    "javascript/ql/src/Metrics/DuplicationProblems.inc.qhelp",
+    "python/ql/src/Metrics/DuplicationProblems.inc.qhelp"
   ],
-  "CommentedOutCodeQuery.qhelp": [
-    "cpp/ql/src/Documentation/CommentedOutCodeQuery.qhelp",
-    "python/ql/src/Lexical/CommentedOutCodeQuery.qhelp",
-    "csharp/ql/src/Bad Practices/Comments/CommentedOutCodeQuery.qhelp",
-    "java/ql/src/Violations of Best Practice/Comments/CommentedOutCodeQuery.qhelp",
-    "javascript/ql/src/Comments/CommentedOutCodeQuery.qhelp"
+  "CommentedOutCodeQuery.inc.qhelp": [
+    "cpp/ql/src/Documentation/CommentedOutCodeQuery.inc.qhelp",
+    "python/ql/src/Lexical/CommentedOutCodeQuery.inc.qhelp",
+    "csharp/ql/src/Bad Practices/Comments/CommentedOutCodeQuery.inc.qhelp",
+    "java/ql/src/Violations of Best Practice/Comments/CommentedOutCodeQuery.inc.qhelp",
+    "javascript/ql/src/Comments/CommentedOutCodeQuery.inc.qhelp"
   ],
-  "FLinesOfCodeReferences.qhelp": [
-    "java/ql/src/Metrics/Files/FLinesOfCodeReferences.qhelp",
-    "javascript/ql/src/Metrics/FLinesOfCodeReferences.qhelp"
+  "FLinesOfCodeReferences.inc.qhelp": [
+    "java/ql/src/Metrics/Files/FLinesOfCodeReferences.inc.qhelp",
+    "javascript/ql/src/Metrics/FLinesOfCodeReferences.inc.qhelp"
   ],
-  "FCommentRatioCommon.qhelp": [
-    "java/ql/src/Metrics/Files/FCommentRatioCommon.qhelp",
-    "javascript/ql/src/Metrics/FCommentRatioCommon.qhelp"
+  "FCommentRatioCommon.inc.qhelp": [
+    "java/ql/src/Metrics/Files/FCommentRatioCommon.inc.qhelp",
+    "javascript/ql/src/Metrics/FCommentRatioCommon.inc.qhelp"
   ],
-  "FLinesOfCodeOverview.qhelp": [
-    "java/ql/src/Metrics/Files/FLinesOfCodeOverview.qhelp",
-    "javascript/ql/src/Metrics/FLinesOfCodeOverview.qhelp"
+  "FLinesOfCodeOverview.inc.qhelp": [
+    "java/ql/src/Metrics/Files/FLinesOfCodeOverview.inc.qhelp",
+    "javascript/ql/src/Metrics/FLinesOfCodeOverview.inc.qhelp"
   ],
-  "CommentedOutCodeMetricOverview.qhelp": [
-    "cpp/ql/src/Metrics/Files/CommentedOutCodeMetricOverview.qhelp",
-    "csharp/ql/src/Metrics/Files/CommentedOutCodeMetricOverview.qhelp",
-    "java/ql/src/Metrics/Files/CommentedOutCodeMetricOverview.qhelp",
-    "javascript/ql/src/Comments/CommentedOutCodeMetricOverview.qhelp",
-    "python/ql/src/Lexical/CommentedOutCodeMetricOverview.qhelp"
+  "CommentedOutCodeMetricOverview.inc.qhelp": [
+    "cpp/ql/src/Metrics/Files/CommentedOutCodeMetricOverview.inc.qhelp",
+    "csharp/ql/src/Metrics/Files/CommentedOutCodeMetricOverview.inc.qhelp",
+    "java/ql/src/Metrics/Files/CommentedOutCodeMetricOverview.inc.qhelp",
+    "javascript/ql/src/Comments/CommentedOutCodeMetricOverview.inc.qhelp",
+    "python/ql/src/Lexical/CommentedOutCodeMetricOverview.inc.qhelp"
   ],
-  "FLinesOfDuplicatedCodeCommon.qhelp": [
-    "cpp/ql/src/Metrics/Files/FLinesOfDuplicatedCodeCommon.qhelp",
-    "java/ql/src/Metrics/Files/FLinesOfDuplicatedCodeCommon.qhelp",
-    "javascript/ql/src/Metrics/FLinesOfDuplicatedCodeCommon.qhelp",
-    "python/ql/src/Metrics/FLinesOfDuplicatedCodeCommon.qhelp"
+  "FLinesOfDuplicatedCodeCommon.inc.qhelp": [
+    "cpp/ql/src/Metrics/Files/FLinesOfDuplicatedCodeCommon.inc.qhelp",
+    "java/ql/src/Metrics/Files/FLinesOfDuplicatedCodeCommon.inc.qhelp",
+    "javascript/ql/src/Metrics/FLinesOfDuplicatedCodeCommon.inc.qhelp",
+    "python/ql/src/Metrics/FLinesOfDuplicatedCodeCommon.inc.qhelp"
   ],
-  "CommentedOutCodeReferences.qhelp": [
-    "cpp/ql/src/Metrics/Files/CommentedOutCodeReferences.qhelp",
-    "csharp/ql/src/Metrics/Files/CommentedOutCodeReferences.qhelp",
-    "java/ql/src/Metrics/Files/CommentedOutCodeReferences.qhelp",
-    "javascript/ql/src/Comments/CommentedOutCodeReferences.qhelp",
-    "python/ql/src/Lexical/CommentedOutCodeReferences.qhelp"
+  "CommentedOutCodeReferences.inc.qhelp": [
+    "cpp/ql/src/Metrics/Files/CommentedOutCodeReferences.inc.qhelp",
+    "csharp/ql/src/Metrics/Files/CommentedOutCodeReferences.inc.qhelp",
+    "java/ql/src/Metrics/Files/CommentedOutCodeReferences.inc.qhelp",
+    "javascript/ql/src/Comments/CommentedOutCodeReferences.inc.qhelp",
+    "python/ql/src/Lexical/CommentedOutCodeReferences.inc.qhelp"
   ],
   "IDE Contextual Queries": [
     "cpp/ql/src/IDEContextual.qll",
@@ -424,5 +425,15 @@
     "java/ql/src/IDEContextual.qll",
     "javascript/ql/src/IDEContextual.qll",
     "python/ql/src/analysis/IDEContextual.qll"
+  ],
+  "SSA C#": [
+    "csharp/ql/src/semmle/code/csharp/dataflow/internal/SsaImplCommon.qll",
+    "csharp/ql/src/semmle/code/csharp/controlflow/internal/pressa/SsaImplCommon.qll",
+    "csharp/ql/src/semmle/code/csharp/dataflow/internal/basessa/SsaImplCommon.qll",
+    "csharp/ql/src/semmle/code/cil/internal/SsaImplCommon.qll"
+  ],
+  "CryptoAlgorithms Python/JS": [
+    "javascript/ql/src/semmle/javascript/security/CryptoAlgorithms.qll",
+    "python/ql/src/semmle/crypto/Crypto.qll"
   ]
-}
+}

cpp/autobuilder/Semmle.Autobuild.Cpp.Tests/BuildScripts.cs

@@ -5,6 +5,7 @@ using System;
 using System.Linq;
 using Microsoft.Build.Construction;
 using System.Xml;
+using System.IO;
 
 namespace Semmle.Autobuild.Cpp.Tests
 {
@@ -43,6 +44,8 @@ namespace Semmle.Autobuild.Cpp.Tests
         public IDictionary<string, int> RunProcess = new Dictionary<string, int>();
         public IDictionary<string, string> RunProcessOut = new Dictionary<string, string>();
         public IDictionary<string, string> RunProcessWorkingDirectory = new Dictionary<string, string>();
+        public HashSet<string> CreateDirectories { get; } = new HashSet<string>();
+        public HashSet<(string, string)> DownloadFiles { get; } = new HashSet<(string, string)>();
 
         int IBuildActions.RunProcess(string cmd, string args, string? workingDirectory, IDictionary<string, string>? env, out IList<string> stdOut)
         {
@@ -135,6 +138,14 @@ namespace Semmle.Autobuild.Cpp.Tests
 
         string IBuildActions.GetFullPath(string path) => path;
 
+        string? IBuildActions.GetFileName(string? path) => Path.GetFileName(path?.Replace('\\', '/'));
+
+        public string? GetDirectoryName(string? path)
+        {
+            var dir = Path.GetDirectoryName(path?.Replace('\\', '/'));
+            return dir is null ? path : path?.Substring(0, dir.Length);
+        }
+
         void IBuildActions.WriteAllText(string filename, string contents)
         {
         }
@@ -153,6 +164,18 @@ namespace Semmle.Autobuild.Cpp.Tests
                 s = s.Replace($"%{kvp.Key}%", kvp.Value);
             return s;
         }
+
+        public void CreateDirectory(string path)
+        {
+            if (!CreateDirectories.Contains(path))
+                throw new ArgumentException($"Missing CreateDirectory, {path}");
+        }
+
+        public void DownloadFile(string address, string fileName)
+        {
+            if (!DownloadFiles.Contains((address, fileName)))
+                throw new ArgumentException($"Missing DownloadFile, {address}, {fileName}");
+        }
     }
 
     /// <summary>
@@ -213,6 +236,7 @@ namespace Semmle.Autobuild.Cpp.Tests
             Actions.GetEnvironmentVariable[$"CODEQL_EXTRACTOR_{codeqlUpperLanguage}_SOURCE_ARCHIVE_DIR"] = "";
             Actions.GetEnvironmentVariable[$"CODEQL_EXTRACTOR_{codeqlUpperLanguage}_ROOT"] = $@"C:\codeql\{codeqlUpperLanguage.ToLowerInvariant()}";
             Actions.GetEnvironmentVariable["CODEQL_JAVA_HOME"] = @"C:\codeql\tools\java";
+            Actions.GetEnvironmentVariable["CODEQL_PLATFORM"] = "win64";
             Actions.GetEnvironmentVariable["SEMMLE_DIST"] = @"C:\odasa";
             Actions.GetEnvironmentVariable["SEMMLE_JAVA_HOME"] = @"C:\odasa\tools\java";
             Actions.GetEnvironmentVariable["SEMMLE_PLATFORM_TOOLS"] = @"C:\odasa\tools";
@@ -273,7 +297,8 @@ namespace Semmle.Autobuild.Cpp.Tests
         [Fact]
         public void TestCppAutobuilderSuccess()
         {
-            Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\csharp\nuget\nuget.exe restore C:\Project\test.sln"] = 1;
+            Actions.RunProcess[@"cmd.exe /C nuget restore C:\Project\test.sln -DisableParallelProcessing"] = 1;
+            Actions.RunProcess[@"cmd.exe /C C:\Project\.nuget\nuget.exe restore C:\Project\test.sln -DisableParallelProcessing"] = 0;
             Actions.RunProcess[@"cmd.exe /C CALL ^""C:\Program Files ^(x86^)\Microsoft Visual Studio 14.0\VC\vcvarsall.bat^"" && set Platform=&& type NUL && C:\odasa\tools\odasa index --auto msbuild C:\Project\test.sln /p:UseSharedCompilation=false /t:rebuild /p:Platform=""x86"" /p:Configuration=""Release"" /p:MvcBuildViews=true"] = 0;
             Actions.RunProcessOut[@"C:\Program Files (x86)\Microsoft Visual Studio\Installer\vswhere.exe -prerelease -legacy -property installationPath"] = "";
             Actions.RunProcess[@"C:\Program Files (x86)\Microsoft Visual Studio\Installer\vswhere.exe -prerelease -legacy -property installationPath"] = 1;
@@ -286,11 +311,13 @@ namespace Semmle.Autobuild.Cpp.Tests
             Actions.FileExists[@"C:\Program Files (x86)\Microsoft Visual Studio\Installer\vswhere.exe"] = true;
             Actions.EnumerateFiles[@"C:\Project"] = "foo.cs\ntest.slx";
             Actions.EnumerateDirectories[@"C:\Project"] = "";
+            Actions.CreateDirectories.Add(@"C:\Project\.nuget");
+            Actions.DownloadFiles.Add(("https://dist.nuget.org/win-x86-commandline/latest/nuget.exe", @"C:\Project\.nuget\nuget.exe"));
 
             var autobuilder = CreateAutoBuilder(true);
             var solution = new TestSolution(@"C:\Project\test.sln");
             autobuilder.ProjectsOrSolutionsToBuild.Add(solution);
-            TestAutobuilderScript(autobuilder, 0, 2);
+            TestAutobuilderScript(autobuilder, 0, 3);
         }
     }
 }

cpp/autobuilder/Semmle.Autobuild.Cpp.Tests/Semmle.Autobuild.Cpp.Tests.csproj

@@ -2,7 +2,7 @@
 
   <PropertyGroup>
     <OutputType>Exe</OutputType>
-    <TargetFramework>netcoreapp3.1</TargetFramework>
+    <TargetFramework>net5.0</TargetFramework>
     <GenerateAssemblyInfo>false</GenerateAssemblyInfo>
     <RuntimeIdentifiers>win-x64;linux-x64;osx-x64</RuntimeIdentifiers>
     <Nullable>enable</Nullable>

cpp/autobuilder/Semmle.Autobuild.Cpp/Semmle.Autobuild.Cpp.csproj

@@ -1,7 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <TargetFramework>netcoreapp3.1</TargetFramework>
+    <TargetFramework>net5.0</TargetFramework>
     <AssemblyName>Semmle.Autobuild.Cpp</AssemblyName>
     <RootNamespace>Semmle.Autobuild.Cpp</RootNamespace>
     <ApplicationIcon />
@@ -17,7 +17,7 @@
   </ItemGroup>
 
   <ItemGroup>
-    <PackageReference Include="Microsoft.Build" Version="16.0.461" />
+    <PackageReference Include="Microsoft.Build" Version="16.9.0" />
   </ItemGroup>
 
   <ItemGroup>

cpp/change-notes/2021-02-04-unsigned-difference-expression-compared-zero.md

@@ -0,0 +1,2 @@
+lgtm
+* A new query (`cpp/unsigned-difference-expression-compared-zero`) is run but not yet displayed on LGTM. The query finds unsigned subtractions used in relational comparisons with the value 0. This query was originally submitted as an experimental query by @ihsinme in https://github.com/github/codeql/pull/4745.

cpp/change-notes/2021-02-24-memset-may-be-deleted.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* A new query (`cpp/memset-may-be-deleted`) is added to the default query suite. The query finds calls to `memset` that may be removed by the compiler. This behavior can make information-leak vulnerabilities easier to exploit. This query was originally [submitted as an experimental query by @ihsinme](https://github.com/github/codeql/pull/4953).

cpp/change-notes/2021-03-01-fluent-interface-data-flow.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* The data-flow library now recognises more side-effects of method chaining (e.g. `someObject.setX(clean).setY(tainted).setZ...` having a side-effect on `someObject`), as well as other related circumstances where a function input is directly passed to its output. All queries that use data-flow analysis, including most security queries, may return more results accordingly.

cpp/change-notes/2021-03-11-failed-extractions.md

@@ -0,0 +1,2 @@
+codescanning
+* Added cpp/diagnostics/failed-extractions. This query gives information about which extractions did not run to completion.

cpp/change-notes/2021-03-17-av-rule-79.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* The 'Resource not released in destructor' (cpp/resource-not-released-in-destructor) query has been improved to recognize more releases of resources.

cpp/config/suites/cpp/correctness

@@ -10,6 +10,7 @@
 + semmlecode-cpp-queries/Likely Bugs/Underspecified Functions/MistypedFunctionArguments.ql: /Correctness/Dangerous Conversions
 + semmlecode-cpp-queries/Security/CWE/CWE-253/HResultBooleanConversion.ql: /Correctness/Dangerous Conversions
 + semmlecode-cpp-queries/Likely Bugs/OO/UnsafeUseOfThis.ql: /Correctness/Dangerous Conversions
++ semmlecode-cpp-queries/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero.ql: /Correctness/Dangerous Conversions
   # Consistent Use
 + semmlecode-cpp-queries/Critical/ReturnValueIgnored.ql: /Correctness/Consistent Use
 + semmlecode-cpp-queries/Likely Bugs/InconsistentCheckReturnNull.ql: /Correctness/Consistent Use

cpp/ql/src/Critical/DeadCodeCondition.qhelp

@@ -9,7 +9,7 @@
 It is likely that these conditions indicate an error in the branching condition. 
 Alternatively, the conditions may have been left behind after debugging.</p>
 
-<include src="aliasAnalysisWarning.qhelp" />
+<include src="aliasAnalysisWarning.inc.qhelp" />
 </overview>
 
 <recommendation>

cpp/ql/src/Critical/DeadCodeFunction.qhelp

@@ -13,7 +13,7 @@ If left in the code base they increase object code size, decrease code comprehen
 This type of function may be part of the program's API and could be used by external programs.
 </p>
 
-<include src="callGraphWarning.qhelp" />
+<include src="callGraphWarning.inc.qhelp" />
 </overview>
 
 <recommendation>

cpp/ql/src/Critical/DescriptorMayNotBeClosed.qhelp

@@ -10,7 +10,7 @@ This query looks at functions that return file or socket descriptors, but may re
 This can occur when an operation performed on the open descriptor fails, and the function returns with an error before it closes the open resource. An improperly handled error could cause the function to leak resource descriptors. Failing to close resources in the function that opened them also makes it more difficult to detect leaks.
 </p> 
 
-<include src="dataFlowWarning.qhelp" />
+<include src="dataFlowWarning.inc.qhelp" />
 </overview>
 
 <recommendation>

cpp/ql/src/Critical/DescriptorNeverClosed.qhelp

@@ -10,7 +10,7 @@ This rule finds calls to <code>socket</code> where there is no corresponding <co
 Leaving descriptors open will cause a resource leak that will persist even after the program terminates.
 </p>
 
-<include src="aliasAnalysisWarning.qhelp" />
+<include src="aliasAnalysisWarning.inc.qhelp" />
 </overview>
 
 <recommendation>

cpp/ql/src/Critical/FileMayNotBeClosed.qhelp

@@ -10,7 +10,7 @@ This rule looks at functions that return a <code>FILE*</code>, but may return an
 This can occur when an operation performed on the open descriptor fails, and the function returns with an error before closing the open resource. An improperly handled error may cause the function to leak file descriptors.
 </p> 
 
-<include src="dataFlowWarning.qhelp" />
+<include src="dataFlowWarning.inc.qhelp" />
 
 </overview>
 <recommendation>

cpp/ql/src/Critical/FileNeverClosed.qhelp

@@ -10,7 +10,7 @@ This rule finds calls to <code>fopen</code> with no corresponding <code>fclose</
 Leaving files open will cause a resource leak that will persist even after the program terminates.
 </p>
 
-<include src="aliasAnalysisWarning.qhelp" />
+<include src="aliasAnalysisWarning.inc.qhelp" />
 
 </overview>
 <recommendation>

cpp/ql/src/Critical/GlobalUseBeforeInit.qhelp

@@ -10,7 +10,7 @@ Not all compilers generate code that zero-out memory, especially when optimizati
 is not compliant with the latest language standards. Accessing uninitialized memory will lead to undefined results.
 </p>
 
-<include src="dataFlowWarning.qhelp" />
+<include src="dataFlowWarning.inc.qhelp" />
 </overview>
 
 <recommendation>

cpp/ql/src/Critical/InconsistentNullnessTesting.qhelp

@@ -12,7 +12,7 @@ Dereferencing a null pointer and attempting to modify its contents can lead to a
 important system data (including the interrupt table in some architectures).
 </p>
 
-<include src="pointsToWarning.qhelp" />
+<include src="pointsToWarning.inc.qhelp" />
 </overview>
 
 <recommendation>

cpp/ql/src/Critical/InitialisationNotRun.qhelp

@@ -11,7 +11,7 @@ Uninitialized variables may contain any value, as not all compilers generate cod
 optimizations are enabled or the compiler is not compliant with the latest language standards.
 </p>
 
-<include src="callGraphWarning.qhelp" />
+<include src="callGraphWarning.inc.qhelp" />
 </overview>
 
 <recommendation>

cpp/ql/src/Critical/LateNegativeTest.qhelp

@@ -13,7 +13,7 @@ after. Otherwise, if the value is negative then the program will have failed
 before performing the test.
 </p>
 
-<include src="dataFlowWarning.qhelp" />
+<include src="dataFlowWarning.inc.qhelp" />
 </overview>
 
 <recommendation>

cpp/ql/src/Critical/MemoryMayNotBeFreed.qhelp

@@ -9,7 +9,7 @@
 This rule looks for functions that allocate memory, but may return without freeing it.  This can occur when an operation performed on the memory block fails, and the function returns with an error before freeing the allocated block.  This causes the function to leak memory and may eventually lead to software failure.
 </p> 
 
-<include src="dataFlowWarning.qhelp" />
+<include src="dataFlowWarning.inc.qhelp" />
 
 </overview>
 <recommendation>

cpp/ql/src/Critical/MemoryNeverFreed.qhelp

@@ -10,7 +10,7 @@ This rule finds calls to the <code>alloc</code> family of functions without a co
 This leads to memory leaks.
 </p>
 
-<include src="aliasAnalysisWarning.qhelp" />
+<include src="aliasAnalysisWarning.inc.qhelp" />
 
 </overview>
 <recommendation>

cpp/ql/src/Critical/MissingNegativityTest.qhelp

@@ -16,7 +16,7 @@ buffer overruns.
 The query looks only at the return values of functions that may return a negative value (not all functions).
 </p>
 
-<include src="dataFlowWarning.qhelp" />
+<include src="dataFlowWarning.inc.qhelp" />
 </overview>
 
 <recommendation>

cpp/ql/src/Critical/NewArrayDeleteMismatch.qhelp

@@ -63,7 +63,7 @@ destructors likely not be called (as previously noted), but the pointer will als
 potentially less of a serious issue than that posed by the first approach, but it should still be avoided.</li>
 </ul>
 
-<include src="pointsToWarning.qhelp" />
+<include src="pointsToWarning.inc.qhelp" />
 
 </overview>
 <recommendation>

cpp/ql/src/Critical/NewDeleteArrayMismatch.qhelp

@@ -18,7 +18,7 @@ an array (which could have header data specifying the length of the array) and w
 element of the 'array', which would likely lead to a segfault due to the invalid header data.
 </p>
 
-<include src="pointsToWarning.qhelp" />
+<include src="pointsToWarning.inc.qhelp" />
 
 </overview>
 <recommendation>

cpp/ql/src/Critical/OverflowCalculated.qhelp

@@ -19,7 +19,7 @@ the data being copied. Buffer overflows can result to anything from a segmentati
 if the array is on stack-allocated memory).
 </p>
 
-<include src="aliasAnalysisWarning.qhelp" />
+<include src="aliasAnalysisWarning.inc.qhelp" />
 </overview>
 
 <recommendation>

cpp/ql/src/Critical/OverflowDestination.qhelp

@@ -14,7 +14,7 @@ Buffer overflows can lead to anything from a segmentation fault to a security vu
 Ensure that the size parameter is derived from the size of the destination buffer, and 
 not the source buffer.</p>
 
-<include src="aliasAnalysisWarning.qhelp" />
+<include src="aliasAnalysisWarning.inc.qhelp" />
 </recommendation>
 
 

cpp/ql/src/Critical/ReturnStackAllocatedObject.qhelp

@@ -12,7 +12,7 @@ the contents of that memory become undefined after that. Clearly, using a pointe
 memory after the function has already returned will have undefined results.
 </p>
 
-<include src="pointsToWarning.qhelp" />
+<include src="pointsToWarning.inc.qhelp" />
 </overview>
 
 <recommendation>

cpp/ql/src/Critical/UseAfterFree.qhelp

@@ -12,7 +12,7 @@ from a segfault to memory corruption that would cause subsequent calls to the dy
 erratically, to a possible security vulnerability.
 </p>
 
-<include src="pointsToWarning.qhelp" />
+<include src="pointsToWarning.inc.qhelp" />
 
 </overview>
 <recommendation>

cpp/ql/src/Diagnostics/ExtractionErrors.ql

@@ -0,0 +1,16 @@
+/**
+ * @name Extraction errors
+ * @description List all extraction errors for files in the source code directory.
+ * @kind diagnostic
+ * @id cpp/diagnostics/extraction-errors
+ */
+
+import cpp
+import ExtractionErrors
+
+from ExtractionError error
+where
+  error instanceof ExtractionUnknownError or
+  exists(error.getFile().getRelativePath())
+select error, "Extraction failed in " + error.getFile() + " with error " + error.getErrorMessage(),
+  error.getSeverity()

cpp/ql/src/Diagnostics/ExtractionErrors.qll

@@ -0,0 +1,137 @@
+/**
+ * Provides a common hierarchy of all types of errors that can occur during extraction.
+ */
+
+import cpp
+
+/*
+ * A note about how the C/C++ extractor emits diagnostics:
+ * When the extractor frontend encounters an error, it emits a diagnostic message,
+ * that includes a message, location and severity.
+ * However, that process is best-effort and may fail (e.g. due to lack of memory).
+ * Thus, if the extractor emitted at least one diagnostic of severity discretionary
+ * error (or higher), it *also* emits a simple "There was an error during this compilation"
+ * error diagnostic, without location information.
+ * In the common case, this means that a compilation during which one or more errors happened also gets
+ * the catch-all diagnostic.
+ * This diagnostic has the empty string as file path.
+ * We filter out these useless diagnostics if there is at least one error-level diagnostic
+ * for the affected compilation in the database.
+ * Otherwise, we show it to indicate that something went wrong and that we
+ * don't know what exactly happened.
+ */
+
+/**
+ * An error that, if present, leads to a file being marked as non-successfully extracted.
+ */
+class ReportableError extends Diagnostic {
+  ReportableError() {
+    (
+      this instanceof CompilerDiscretionaryError or
+      this instanceof CompilerError or
+      this instanceof CompilerCatastrophe
+    ) and
+    // Filter for the catch-all diagnostic, see note above.
+    not this.getFile().getAbsolutePath() = ""
+  }
+}
+
+private newtype TExtractionError =
+  TReportableError(ReportableError err) or
+  TCompilationFailed(Compilation c, File f) {
+    f = c.getAFileCompiled() and not c.normalTermination()
+  } or
+  // Show the catch-all diagnostic (see note above) only if we haven't seen any other error-level diagnostic
+  // for that compilation
+  TUnknownError(CompilerError err) {
+    not exists(ReportableError e | e.getCompilation() = err.getCompilation())
+  }
+
+/**
+ * Superclass for the extraction error hierarchy.
+ */
+class ExtractionError extends TExtractionError {
+  /** Gets the string representation of the error. */
+  string toString() { none() }
+
+  /** Gets the error message for this error. */
+  string getErrorMessage() { none() }
+
+  /** Gets the file this error occured in. */
+  File getFile() { none() }
+
+  /** Gets the location this error occured in. */
+  Location getLocation() { none() }
+
+  /** Gets the SARIF severity of this error. */
+  int getSeverity() {
+    // Unfortunately, we can't distinguish between errors and fatal errors in SARIF,
+    // so all errors have severity 2.
+    result = 2
+  }
+}
+
+/**
+ * An unrecoverable extraction error, where extraction was unable to finish.
+ * This can be caused by a multitude of reasons, for example:
+ *  - hitting a frontend assertion
+ *  - crashing due to dereferencing an invalid pointer
+ *  - stack overflow
+ *  - out of memory
+ */
+class ExtractionUnrecoverableError extends ExtractionError, TCompilationFailed {
+  Compilation c;
+  File f;
+
+  ExtractionUnrecoverableError() { this = TCompilationFailed(c, f) }
+
+  override string toString() {
+    result = "Unrecoverable extraction error while compiling " + f.toString()
+  }
+
+  override string getErrorMessage() { result = "unrecoverable compilation failure." }
+
+  override File getFile() { result = f }
+
+  override Location getLocation() { result = f.getLocation() }
+}
+
+/**
+ * A recoverable extraction error.
+ * These are compiler errors from the frontend.
+ * Upon encountering one of these, we still continue extraction, but the
+ * database will be incomplete for that file.
+ */
+class ExtractionRecoverableError extends ExtractionError, TReportableError {
+  ReportableError err;
+
+  ExtractionRecoverableError() { this = TReportableError(err) }
+
+  override string toString() { result = "Recoverable extraction error: " + err }
+
+  override string getErrorMessage() { result = err.getFullMessage() }
+
+  override File getFile() { result = err.getFile() }
+
+  override Location getLocation() { result = err.getLocation() }
+}
+
+/**
+ * An unknown error happened during extraction.
+ * These are only displayed if we know that we encountered an error during extraction,
+ * but, for some reason, failed to emit a proper diagnostic with location information
+ * and error message.
+ */
+class ExtractionUnknownError extends ExtractionError, TUnknownError {
+  CompilerError err;
+
+  ExtractionUnknownError() { this = TUnknownError(err) }
+
+  override string toString() { result = "Unknown extraction error: " + err }
+
+  override string getErrorMessage() { result = err.getFullMessage() }
+
+  override File getFile() { result = err.getFile() }
+
+  override Location getLocation() { result = err.getLocation() }
+}

cpp/ql/src/Diagnostics/FailedExtractorInvocations.ql

@@ -0,0 +1,22 @@
+/**
+ * @name Failed extractor invocations
+ * @description Gives the command line of compilations for which extraction did not run to completion.
+ * @kind diagnostic
+ * @id cpp/diagnostics/failed-extractor-invocations
+ */
+
+import cpp
+
+class AnonymousCompilation extends Compilation {
+  override string toString() { result = "<compilation>" }
+}
+
+string describe(Compilation c) {
+  if c.getArgument(1) = "--mimic"
+  then result = "compiler invocation " + concat(int i | i > 1 | c.getArgument(i), " " order by i)
+  else result = "extractor invocation " + concat(int i | | c.getArgument(i), " " order by i)
+}
+
+from Compilation c
+where not c.normalTermination()
+select c, "Extraction aborted for " + describe(c), 2

cpp/ql/src/Diagnostics/SuccessfullyExtractedFiles.ql

@@ -0,0 +1,15 @@
+/**
+ * @name Successfully extracted files
+ * @description Lists all files in the source code directory that were extracted without encountering an error in the file.
+ * @kind diagnostic
+ * @id cpp/diagnostics/successfully-extracted-files
+ */
+
+import cpp
+import ExtractionErrors
+
+from File f
+where
+  not exists(ExtractionError e | e.getFile() = f) and
+  exists(f.getRelativePath())
+select f, ""

cpp/ql/src/Documentation/CommentedOutCode.qhelp

@@ -2,6 +2,6 @@
   "-//Semmle//qhelp//EN"
   "qhelp.dtd">
 <qhelp>
-  <include src="CommentedOutCodeQuery.qhelp" />
-  <include src="../Metrics/Files/CommentedOutCodeReferences.qhelp" />
+  <include src="CommentedOutCodeQuery.inc.qhelp" />
+  <include src="../Metrics/Files/CommentedOutCodeReferences.inc.qhelp" />
 </qhelp>

cpp/ql/src/Likely Bugs/Leap Year/Adding365DaysPerYear.qhelp

@@ -3,7 +3,7 @@
   "qhelp.dtd">
 <qhelp>
 <overview>
-  <include src="LeapYear.qhelp" />
+  <include src="LeapYear.inc.qhelp" />
 
 <p>When performing arithmetic operations on a variable that represents a date, leap years must be taken into account.
 It is not safe to assume that a year is 365 days long.</p>

cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.qhelp

@@ -3,7 +3,7 @@
   "qhelp.dtd">
 <qhelp>
 <overview>
-  <include src="LeapYear.qhelp" />
+  <include src="LeapYear.inc.qhelp" />
   
 <p>When performing arithmetic operations on a variable that represents a year, it is important to consider that the resulting value may not be a valid date.</p>
 <p>The typical example is doing simple year arithmetic (i.e. <code>date.year++</code>) without considering if the resulting value will be a valid date or not.</p>

cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.qhelp

@@ -3,7 +3,7 @@
   "qhelp.dtd">
 <qhelp>
 <overview>
-  <include src="LeapYear.qhelp" />
+  <include src="LeapYear.inc.qhelp" />
   
 <p>When using a function that transforms a date structure, and the year on the input argument for the API has been manipulated, it is important to check for the return value of the function to make sure it succeeded.</p>
 <p>Otherwise, the function may have failed, and the output parameter may contain invalid data that can cause any number of problems on the affected system.</p>

cpp/ql/src/Likely Bugs/Leap Year/UnsafeArrayForDaysOfYear.qhelp

@@ -3,7 +3,7 @@
   "qhelp.dtd">
 <qhelp>
 <overview>
-  <include src="LeapYear.qhelp" />
+  <include src="LeapYear.inc.qhelp" />
 
 <p>This query helps to detect when a developer allocates an array or other fixed-length data structure such as <code>std::vector</code> with 365 elements – one for each day of the year.</p>
 <p>Since leap years have 366 days, there will be no allocated element on December 31st at the end of a leap year; which will lead to a buffer overflow on a leap year.</p>

cpp/ql/src/Metrics/Dependencies/ExternalDependencies.ql

@@ -5,7 +5,6 @@
  * @kind treemap
  * @treemap.warnOn highValues
  * @metricType externalDependency
- * @precision medium
  * @id cpp/external-dependencies
  * @tags modularity
  */

cpp/ql/src/Metrics/Files/FLinesOfCode.ql

@@ -7,7 +7,6 @@
  * @treemap.warnOn highValues
  * @metricType file
  * @metricAggregate avg sum max
- * @precision very-high
  * @id cpp/lines-of-code-in-files
  * @tags maintainability
  *       complexity

cpp/ql/src/Metrics/Files/FLinesOfCommentedOutCode.qhelp

@@ -2,6 +2,6 @@
   "-//Semmle//qhelp//EN"
   "qhelp.dtd">
 <qhelp>
-  <include src="CommentedOutCodeMetricOverview.qhelp" />
-  <include src="CommentedOutCodeReferences.qhelp" />
+  <include src="CommentedOutCodeMetricOverview.inc.qhelp" />
+  <include src="CommentedOutCodeReferences.inc.qhelp" />
 </qhelp>

cpp/ql/src/Metrics/Files/FLinesOfCommentedOutCode.ql

@@ -5,7 +5,6 @@
  * @treemap.warnOn highValues
  * @metricType file
  * @metricAggregate avg sum max
- * @precision high
  * @id cpp/lines-of-commented-out-code-in-files
  * @tags documentation
  */

cpp/ql/src/Metrics/Files/FLinesOfComments.ql

@@ -7,7 +7,6 @@
  * @treemap.warnOn lowValues
  * @metricType file
  * @metricAggregate avg sum max
- * @precision very-high
  * @id cpp/lines-of-comments-in-files
  * @tags maintainability
  *       documentation

cpp/ql/src/Metrics/Files/FLinesOfDuplicatedCode.qhelp

@@ -2,5 +2,5 @@
   "-//Semmle//qhelp//EN"
   "qhelp.dtd">
 <qhelp>
-<include src="FLinesOfDuplicatedCodeCommon.qhelp" />
+<include src="FLinesOfDuplicatedCodeCommon.inc.qhelp" />
 </qhelp>

cpp/ql/src/Metrics/Files/FLinesOfDuplicatedCode.ql

@@ -8,7 +8,6 @@
  * @treemap.warnOn highValues
  * @metricType file
  * @metricAggregate avg sum max
- * @precision high
  * @id cpp/duplicated-lines-in-files
  * @tags testability
  *       modularity

cpp/ql/src/Metrics/Files/FLinesOfDuplicatedCodeCommon.inc.qhelp

@@ -14,7 +14,7 @@ for a number of reasons.
 </p>
 
 </overview>
-<include src="DuplicationProblems.qhelp" />
+<include src="DuplicationProblems.inc.qhelp" />
 
 <recommendation>
 

cpp/ql/src/Metrics/Files/FNumberOfTests.ql

@@ -5,7 +5,6 @@
  * @treemap.warnOn lowValues
  * @metricType file
  * @metricAggregate avg sum max
- * @precision medium
  * @id cpp/tests-in-files
  * @tags maintainability
  */

cpp/ql/src/Security/CWE/CWE-014/MemsetMayBeDeleted-bad.c

@@ -0,0 +1,3 @@
+char password[MAX_PASSWORD_LENGTH];
+// read and verify password
+memset(password, 0, MAX_PASSWORD_LENGTH);

cpp/ql/src/Security/CWE/CWE-014/MemsetMayBeDeleted-good.c

@@ -0,0 +1,3 @@
+char password[MAX_PASSWORD_LENGTH];
+// read and verify password
+memset_s(password, MAX_PASSWORD_LENGTH, 0, MAX_PASSWORD_LENGTH);

cpp/ql/src/Security/CWE/CWE-014/MemsetMayBeDeleted.qhelp

@@ -0,0 +1,45 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Calling <code>memset</code> or <code>bzero</code> on a buffer to clear its contents may get optimized
+away by the compiler if the buffer is not subsequently used. This is not desirable behavior if the buffer
+contains sensitive data that could somehow be retrieved by an attacker.</p>
+
+</overview>
+<recommendation>
+
+<p>Use alternative platform-supplied functions that will not get optimized away. Examples of such
+functions include <code>memset_s</code>, <code>SecureZeroMemory</code>, and <code>bzero_explicit</code>.
+Alternatively, passing the <code>-fno-builtin-memset</code> option to the GCC/Clang compiler usually
+also prevents the optimization. Finally, you can use the public-domain <code>secure_memzero</code> function
+(see references below). This function, however, is not guaranteed to work on all platforms and compilers.</p>
+
+</recommendation>
+<example>
+<p>The following program fragment uses <code>memset</code> to erase sensitive information after it is no
+longer needed:</p>
+<sample src="MemsetMayBeDeleted-bad.c" />
+<p>Because of dead store elimination, the call to <code>memset</code> may be removed by the compiler
+(since the buffer is not subsequently used), resulting in potentially sensitive data remaining in memory.
+</p>
+
+<p>The best solution to this problem is to use the <code>memset_s</code> function instead of
+<code>memset</code>:</p>
+<sample src="MemsetMayBeDeleted-good.c" />
+
+</example>
+<references>
+
+<li>
+CERT C Coding Standard:
+<a href="https://wiki.sei.cmu.edu/confluence/display/c/MSC06-C.+Beware+of+compiler+optimizations">MSC06-C. Beware of compiler optimizations</a>.
+</li>
+<li>
+USENIX: The Advanced Computing Systems Association:
+<a href="https://www.usenix.org/system/files/conference/usenixsecurity17/sec17-yang.pdf">Dead Store Elimination (Still) Considered Harmfuls</a>
+</li>
+
+</references>
+</qhelp>

cpp/ql/src/Security/CWE/CWE-014/MemsetMayBeDeleted.ql

@@ -0,0 +1,81 @@
+/**
+ * @name Call to `memset` may be deleted
+ * @description Using the `memset` function to clear private data in a variable that has no subsequent use
+ *              can make information-leak vulnerabilities easier to exploit because the compiler can remove the call.
+ * @kind problem
+ * @id cpp/memset-may-be-deleted
+ * @problem.severity warning
+ * @precision high
+ * @tags security
+ *       external/cwe/cwe-14
+ */
+
+import cpp
+import semmle.code.cpp.dataflow.EscapesTree
+import semmle.code.cpp.commons.Exclusions
+import semmle.code.cpp.models.interfaces.Alias
+
+class MemsetFunction extends Function {
+  MemsetFunction() {
+    this.hasGlobalOrStdOrBslName("memset")
+    or
+    this.hasGlobalOrStdName("wmemset")
+    or
+    this.hasGlobalName(["bzero", "__builtin_memset"])
+  }
+}
+
+predicate isNonEscapingArgument(Expr escaped) {
+  exists(Call call, AliasFunction aliasFunction, int i |
+    aliasFunction = call.getTarget() and
+    call.getArgument(i) = escaped.getUnconverted() and
+    (
+      aliasFunction.parameterNeverEscapes(i)
+      or
+      aliasFunction.parameterEscapesOnlyViaReturn(i) and
+      (call instanceof ExprInVoidContext or call.getConversion*() instanceof BoolConversion)
+    )
+  )
+}
+
+pragma[noinline]
+predicate callToMemsetWithRelevantVariable(
+  LocalVariable v, VariableAccess acc, FunctionCall call, MemsetFunction memset
+) {
+  not v.isStatic() and
+  // Reference-typed variables get special treatment in `variableAddressEscapesTree` so we leave them
+  // out of this query.
+  not v.getUnspecifiedType() instanceof ReferenceType and
+  call.getTarget() = memset and
+  acc = v.getAnAccess() and
+  // `v` escapes as the argument to `memset`
+  variableAddressEscapesTree(acc, call.getArgument(0).getFullyConverted())
+}
+
+pragma[noinline]
+predicate relevantVariable(LocalVariable v, FunctionCall call, MemsetFunction memset) {
+  exists(VariableAccess acc, VariableAccess anotherAcc |
+    callToMemsetWithRelevantVariable(v, acc, call, memset) and
+    // `v` is not only just used in the call to `memset`.
+    anotherAcc = v.getAnAccess() and
+    acc != anotherAcc and
+    not anotherAcc.isUnevaluated()
+  )
+}
+
+from FunctionCall call, LocalVariable v, MemsetFunction memset
+where
+  relevantVariable(v, call, memset) and
+  not isFromMacroDefinition(call) and
+  // `v` doesn't escape anywhere else.
+  forall(Expr escape | variableAddressEscapesTree(v.getAnAccess(), escape) |
+    isNonEscapingArgument(escape)
+  ) and
+  // There is no later use of `v`.
+  not v.getAnAccess() = call.getASuccessor*() and
+  // Not using the `-fno-builtin-memset` flag
+  exists(Compilation c |
+    c.getAFileCompiled() = call.getFile() and
+    not c.getAnArgument() = "-fno-builtin-memset"
+  )
+select call, "Call to " + memset.getName() + " may be deleted by the compiler."

cpp/ql/src/Security/CWE/CWE-020/ExternalAPIsSpecific.qll

@@ -46,7 +46,7 @@ class UntrustedDataToExternalAPIConfig extends TaintTracking::Configuration {
   UntrustedDataToExternalAPIConfig() { this = "UntrustedDataToExternalAPIConfig" }
 
   override predicate isSource(DataFlow::Node source) {
-    exists(RemoteFlowFunction remoteFlow |
+    exists(RemoteFlowSourceFunction remoteFlow |
       remoteFlow = source.asExpr().(Call).getTarget() and
       remoteFlow.hasRemoteFlowSource(_, _)
     )

cpp/ql/src/Security/CWE/CWE-079/CgiXss.ql

@@ -34,6 +34,10 @@ class Configuration extends TaintTrackingConfiguration {
   override predicate isSink(Element tainted) {
     exists(PrintStdoutCall call | call.getAnArgument() = tainted)
   }
+
+  override predicate isBarrier(Expr e) {
+    super.isBarrier(e) or e.getUnspecifiedType() instanceof IntegralType
+  }
 }
 
 from QueryString query, Element printedArg, PathNode sourceNode, PathNode sinkNode

cpp/ql/src/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero.c

@@ -0,0 +1,5 @@
+unsigned limit = get_limit();
+unsigned total = 0;
+while (limit - total > 0) { // wrong: if `total` is greater than `limit` this will underflow and continue executing the loop.
+  total += get_data();
+}

cpp/ql/src/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero.qhelp

@@ -0,0 +1,31 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>
+This rule finds relational comparisons between the result of an unsigned subtraction and the value <code>0</code>.
+Such comparisons are likely to be wrong as the value of an unsigned subtraction can never be negative. So the
+relational comparison ends up checking whether the result of the subtraction is equal to <code>0</code>.
+This is probably not what the programmer intended.
+</p>
+</overview>
+<recommendation>
+
+<p>If a relational comparison is intended, consider casting the result of the subtraction to a signed type.
+   If the intention was to test for equality, consider replacing the relational comparison with an equality test.
+</p>
+
+</recommendation>
+<example>
+<sample src="UnsignedDifferenceExpressionComparedZero.c" />
+
+</example>
+<references>
+
+<li>SEI CERT C Coding Standard:
+<a href="https://wiki.sei.cmu.edu/confluence/display/c/INT02-C.+Understand+integer+conversion+rules">INT02-C. Understand integer conversion rules</a>.
+</li>
+
+</references>
+</qhelp>

cpp/ql/src/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero.ql

@@ -0,0 +1,49 @@
+/**
+ * @name Unsigned difference expression compared to zero
+ * @description A subtraction with an unsigned result can never be negative. Using such an expression in a relational comparison with `0` is likely to be wrong.
+ * @kind problem
+ * @id cpp/unsigned-difference-expression-compared-zero
+ * @problem.severity warning
+ * @precision medium
+ * @tags security
+ *       correctness
+ *       external/cwe/cwe-191
+ */
+
+import cpp
+import semmle.code.cpp.commons.Exclusions
+import semmle.code.cpp.valuenumbering.GlobalValueNumbering
+import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
+import semmle.code.cpp.controlflow.Guards
+
+/** Holds if `sub` is guarded by a condition which ensures that `left >= right`. */
+pragma[noinline]
+predicate isGuarded(SubExpr sub, Expr left, Expr right) {
+  exists(GuardCondition guard |
+    guard.controls(sub.getBasicBlock(), true) and
+    guard.ensuresLt(left, right, 0, sub.getBasicBlock(), false)
+  )
+}
+
+/** Holds if `sub` will never be negative. */
+predicate nonNegative(SubExpr sub) {
+  not exprMightOverflowNegatively(sub.getFullyConverted())
+  or
+  // The subtraction is guarded by a check of the form `left >= right`.
+  exists(GVN left, GVN right |
+    // This is basically a poor man's version of a directional unbind operator.
+    strictcount([left, globalValueNumber(sub.getLeftOperand())]) = 1 and
+    strictcount([right, globalValueNumber(sub.getRightOperand())]) = 1 and
+    isGuarded(sub, left.getAnExpr(), right.getAnExpr())
+  )
+}
+
+from RelationalOperation ro, SubExpr sub
+where
+  not isFromMacroDefinition(ro) and
+  not isFromMacroDefinition(sub) and
+  ro.getLesserOperand().getValue().toInt() = 0 and
+  ro.getGreaterOperand() = sub and
+  sub.getFullyConverted().getUnspecifiedType().(IntegralType).isUnsigned() and
+  not nonNegative(sub)
+select ro, "Unsigned subtraction can never be negative."

cpp/ql/src/Security/CWE/CWE-311/CleartextBufferWrite.qhelp

@@ -2,4 +2,4 @@
   "-//Semmle//qhelp//EN"
   "qhelp.dtd">
 <qhelp>
-<include src="CleartextStorage.qhelp" /></qhelp>
+<include src="CleartextStorage.inc.qhelp" /></qhelp>

cpp/ql/src/Security/CWE/CWE-311/CleartextFileWrite.qhelp

@@ -2,4 +2,4 @@
   "-//Semmle//qhelp//EN"
   "qhelp.dtd">
 <qhelp>
-<include src="CleartextStorage.qhelp" /></qhelp>
+<include src="CleartextStorage.inc.qhelp" /></qhelp>

cpp/ql/src/Security/CWE/CWE-457/InitializationFunctions.qll

@@ -353,7 +353,9 @@ class InitializationFunction extends Function {
           // Destination range is zeroed out on failure, assuming first two parameters are valid
           "memcpy_s",
           // This zeroes the memory unconditionally
-          "SeCreateAccessState"
+          "SeCreateAccessState",
+          // Argument initialization is optional, but always succeeds
+          "KeGetCurrentProcessorNumberEx"
         ]
     )
   }

cpp/ql/src/Security/CWE/CWE-676/DangerousFunctionOverflow.ql

@@ -8,6 +8,7 @@
  * @tags reliability
  *       security
  *       external/cwe/cwe-242
+ *       external/cwe/cwe-676
  */
 
 import cpp

cpp/ql/src/Summary/LinesOfCode.ql

@@ -0,0 +1,11 @@
+/**
+ * @id cpp/summary/lines-of-code
+ * @name Total lines of C/C++ code in the database
+ * @description The total number of lines of C/C++ code across all files, including system headers, libraries, and auto-generated files. This is a useful metric of the size of a database. For all files that were seen during the build, this query counts the lines of code, excluding whitespace or comments.
+ * @kind metric
+ * @tags summary
+ */
+
+import cpp
+
+select sum(File f | f.fromSource() | f.getMetrics().getNumberOfLinesOfCode())

cpp/ql/src/experimental/Security/CWE/CWE-020/LateCheckOfFunctionArgument.c

@@ -0,0 +1,7 @@
+if(len<0) return 1;
+memset(dest, source, len); // GOOD: variable `len` checked before call
+
+...
+
+memset(dest, source, len); // BAD: variable `len` checked after call
+if(len<0) return 1;

cpp/ql/src/experimental/Security/CWE/CWE-020/LateCheckOfFunctionArgument.qhelp

@@ -0,0 +1,28 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Checking the function argument after calling the function itself. This situation looks suspicious and requires the attention of the developer. It may be necessary to add validation before calling the function</p>
+
+
+</overview>
+<recommendation>
+
+<p>We recommend checking before calling the function.</p>
+
+</recommendation>
+<example>
+<p>The following example demonstrates an erroneous and fixed use of function argument validation.</p>
+<sample src="LateCheckOfFunctionArgument.c" />
+
+</example>
+<references>
+
+<li>
+  CWE Common Weakness Enumeration:
+  <a href="https://cwe.mitre.org/data/definitions/20.html"> CWE-20: Improper Input Validation</a>.
+</li>
+
+</references>
+</qhelp>

cpp/ql/src/experimental/Security/CWE/CWE-020/LateCheckOfFunctionArgument.ql

@@ -0,0 +1,66 @@
+/**
+ * @name Late Check Of Function Argument
+ * @description --Checking the function argument after calling the function itself.
+ *              --This situation looks suspicious and requires the attention of the developer.
+ *              --It may be necessary to add validation before calling the function.
+ * @kind problem
+ * @id cpp/late-check-of-function-argument
+ * @problem.severity warning
+ * @precision medium
+ * @tags correctness
+ *       security
+ *       external/cwe/cwe-20
+ */
+
+import cpp
+import semmle.code.cpp.valuenumbering.GlobalValueNumbering
+
+/** Holds for a function `f` that has an argument at index `apos` used for positioning in a buffer. */
+predicate numberArgument(Function f, int apos) {
+  f.hasGlobalOrStdName("write") and apos = 2
+  or
+  f.hasGlobalOrStdName("read") and apos = 2
+  or
+  f.hasGlobalOrStdName("lseek") and apos = 1
+  or
+  f.hasGlobalOrStdName("memmove") and apos = 2
+  or
+  f.hasGlobalOrStdName("memset") and apos = 2
+  or
+  f.hasGlobalOrStdName("memcpy") and apos = 2
+  or
+  f.hasGlobalOrStdName("memcmp") and apos = 2
+  or
+  f.hasGlobalOrStdName("strncat") and apos = 2
+  or
+  f.hasGlobalOrStdName("strncpy") and apos = 2
+  or
+  f.hasGlobalOrStdName("strncmp") and apos = 2
+  or
+  f.hasGlobalOrStdName("snprintf") and apos = 1
+  or
+  f.hasGlobalOrStdName("strndup") and apos = 2
+}
+
+class IfCompareWithZero extends IfStmt {
+  IfCompareWithZero() { this.getCondition().(RelationalOperation).getAChild().getValue() = "0" }
+
+  Expr noZerroOperand() {
+    if this.getCondition().(RelationalOperation).getGreaterOperand().getValue() = "0"
+    then result = this.getCondition().(RelationalOperation).getLesserOperand()
+    else result = this.getCondition().(RelationalOperation).getGreaterOperand()
+  }
+}
+
+from FunctionCall fc, IfCompareWithZero ifc, int na
+where
+  numberArgument(fc.getTarget(), na) and
+  globalValueNumber(fc.getArgument(na)) = globalValueNumber(ifc.noZerroOperand()) and
+  dominates(fc, ifc) and
+  not exists(IfStmt ifc1 |
+    dominates(ifc1, fc) and
+    globalValueNumber(fc.getArgument(na)) = globalValueNumber(ifc1.getCondition().getAChild*())
+  )
+select fc,
+  "The value of argument '$@' appears to be checked after the call, rather than before it.",
+  fc.getArgument(na), fc.getArgument(na).toString()

cpp/ql/src/experimental/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero.c

@@ -1,11 +0,0 @@
-unsigned long sizeArray;
-
-// BAD: let's consider several values, taking ULONG_MAX =18446744073709551615
-// sizeArray = 60; (sizeArray - 10) = 50; true
-// sizeArray = 10; (sizeArray - 10) = 0; false
-// sizeArray = 1; (sizeArray - 10) = 18446744073709551607; true
-// sizeArray = 0; (sizeArray - 10) = 18446744073709551606; true
-if (sizeArray - 10 > 0)
-
-// GOOD: Prevent overflow by checking the input
-if (sizeArray > 10)

cpp/ql/src/experimental/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero.qhelp

@@ -1,33 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-<overview>
-<p>The code compares the unsigned difference with zero. 
-It is highly probable that the condition is wrong if the difference expression has the unsigned type.  
-The condition holds in all the cases when difference is not equal to zero. 
-It means that we may use condition not equal. But the programmer probably wanted to compare the difference of elements.</p>
-
-<p>False positives include code in which the first difference element is always greater than or equal to the second. 
-For comparison, ">" such conditions are equivalent to "! =", And are recommended for replacement. 
-For comparison "> =", the conditions are always true and are recommended to be excluded.</p>
-
-</overview>
-<recommendation>
-
-<p>Use a simple comparison of two elements, instead of comparing their difference to zero.</p>
-
-</recommendation>
-<example>
-<p>The following example demonstrates an erroneous and corrected use of comparison.</p>
-<sample src="UnsignedDifferenceExpressionComparedZero.c" />
-
-</example>
-<references>
-
-<li>CERT C Coding Standard:
-<a href="https://wiki.sei.cmu.edu/confluence/display/c/INT02-C.+Understand+integer+conversion+rules">INT02-C. Understand integer conversion rules</a>.
-</li>
-
-</references>
-</qhelp>

cpp/ql/src/experimental/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero.ql

@@ -1,23 +0,0 @@
-/**
- * @name Unsigned difference expression compared to zero
- * @description It is highly probable that the condition is wrong if the difference expression has the unsigned type.
- *              The condition holds in all the cases when difference is not equal to zero. It means that we may use condition not equal.
- *              But the programmer probably wanted to compare the difference of elements.
- * @kind problem
- * @id cpp/unsigned-difference-expression-compared-zero
- * @problem.severity warning
- * @precision medium
- * @tags security
- *       external/cwe/cwe-191
- */
-
-import cpp
-import semmle.code.cpp.commons.Exclusions
-
-from RelationalOperation ro, SubExpr sub
-where
-  not isFromMacroDefinition(ro) and
-  ro.getLesserOperand().getValue().toInt() = 0 and
-  ro.getGreaterOperand() = sub and
-  sub.getFullyConverted().getUnspecifiedType().(IntegralType).isUnsigned()
-select ro, "Difference in condition is always greater than or equal to zero"

cpp/ql/src/experimental/Security/CWE/CWE-359/PrivateCleartextWrite.ql

@@ -16,6 +16,6 @@ import DataFlow::PathGraph
 
 from WriteConfig b, DataFlow::PathNode source, DataFlow::PathNode sink
 where b.hasFlowPath(source, sink)
-select sink.getNode(),
-  "This write into the external location '" + sink + "' may contain unencrypted data from $@",
-  source, "this source."
+select sink.getNode(), source, sink,
+  "This write into the external location '" + sink.getNode() +
+    "' may contain unencrypted data from $@", source, "this source."

cpp/ql/src/experimental/Security/CWE/CWE-401/MemoryLeakOnFailedCallToRealloc.ql

@@ -12,6 +12,21 @@
  */
 
 import cpp
+import semmle.code.cpp.controlflow.Guards
+import semmle.code.cpp.valuenumbering.HashCons
+
+/**
+ * A function call that potentially does not return (such as `exit`).
+ */
+class CallMayNotReturn extends FunctionCall {
+  CallMayNotReturn() {
+    // call that is known to not return
+    not exists(this.(ControlFlowNode).getASuccessor())
+    or
+    // call to another function that may not return
+    exists(CallMayNotReturn exit | getTarget() = exit.getEnclosingFunction())
+  }
+}
 
 /**
  * A call to `realloc` of the form `v = realloc(v, size)`, for some variable `v`.
@@ -20,50 +35,27 @@ class ReallocCallLeak extends FunctionCall {
   Variable v;
 
   ReallocCallLeak() {
-    exists(AssignExpr ex, VariableAccess va1, VariableAccess va2 |
-      this.getTarget().hasName("realloc") and
+    exists(AssignExpr ex |
+      this.getTarget().hasGlobalOrStdName("realloc") and
       this = ex.getRValue() and
-      va1 = ex.getLValue() and
-      va2 = this.getArgument(0) and
-      va1 = v.getAnAccess() and
-      va2 = v.getAnAccess()
+      hashCons(ex.getLValue()) = hashCons(this.getArgument(0)) and
+      v.getAnAccess() = this.getArgument(0)
     )
   }
 
-  predicate isExistsIfWithExitCall() {
-    exists(IfStmt ifc |
-      this.getArgument(0) = v.getAnAccess() and
-      ifc.getCondition().getAChild*() = v.getAnAccess() and
-      ifc.getEnclosingFunction() = this.getEnclosingFunction() and
-      ifc.getLocation().getStartLine() >= this.getArgument(0).getLocation().getStartLine() and
-      exists(FunctionCall fc |
-        fc.getTarget().hasName("exit") and
-        fc.getEnclosingFunction() = this.getEnclosingFunction() and
-        (ifc.getThen().getAChild*() = fc or ifc.getElse().getAChild*() = fc)
-      )
-      or
-      exists(FunctionCall fc, FunctionCall ftmp1, FunctionCall ftmp2 |
-        ftmp1.getTarget().hasName("exit") and
-        ftmp2.(ControlFlowNode).getASuccessor*() = ftmp1 and
-        fc = ftmp2.getEnclosingFunction().getACallToThisFunction() and
-        fc.getEnclosingFunction() = this.getEnclosingFunction() and
-        (ifc.getThen().getAChild*() = fc or ifc.getElse().getAChild*() = fc)
-      )
-    )
-  }
-
-  predicate isExistsAssertWithArgumentCall() {
-    exists(FunctionCall fc |
-      fc.getTarget().hasName("__assert_fail") and
-      this.getEnclosingFunction() = fc.getEnclosingFunction() and
-      fc.getLocation().getStartLine() > this.getArgument(0).getLocation().getEndLine() and
-      fc.getArgument(0).toString().matches("%" + this.getArgument(0).toString() + "%")
+  /**
+   * Holds if failure of this allocation may be handled by termination, for
+   * example a call to `exit()`.
+   */
+  predicate mayHandleByTermination() {
+    exists(GuardCondition guard, CallMayNotReturn exit |
+      this.(ControlFlowNode).getASuccessor*() = guard and
+      guard.getAChild*() = v.getAnAccess() and
+      guard.controls(exit.getBasicBlock(), _)
     )
   }
 }
 
 from ReallocCallLeak rcl
-where
-  not rcl.isExistsIfWithExitCall() and
-  not rcl.isExistsAssertWithArgumentCall()
+where not rcl.mayHandleByTermination()
 select rcl, "possible loss of original pointer on unsuccessful call realloc"

cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.cpp

@@ -0,0 +1,35 @@
+// BAD: on memory allocation error, the program terminates.
+void badFunction(const int *source, std::size_t length) noexcept {
+  int * dest = new int[length];
+  std::memset(dest, 0, length);
+// ..
+}
+// GOOD: memory allocation error will be handled.
+void goodFunction(const int *source, std::size_t length) noexcept {
+  try {
+       int * dest = new int[length];
+  } catch(std::bad_alloc) {
+    // ...
+  }
+  std::memset(dest, 0, length);
+// ..
+}
+// BAD: memory allocation error will not be handled.
+void badFunction(const int *source, std::size_t length) noexcept {
+  try {
+       int * dest = new (std::nothrow) int[length];
+  } catch(std::bad_alloc) {
+    // ...
+  }
+  std::memset(dest, 0, length);
+// ..
+}
+// GOOD: memory allocation error will be handled.
+void goodFunction(const int *source, std::size_t length) noexcept {
+  int * dest = new (std::nothrow) int[length];
+  if (!dest) {
+      return;
+  }
+  std::memset(dest, 0, length);
+// ..
+}

cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.qhelp

@@ -0,0 +1,27 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>When using the <code>new</code> operator to allocate memory, you need to pay attention to the different ways of detecting errors. <code>::operator new(std::size_t)</code> throws an exception on error, whereas <code>::operator new(std::size_t, const std::nothrow_t &amp;)</code> 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.</p>
+
+</overview>
+<recommendation>
+
+<p>Use the correct error detection method corresponding with the memory allocation.</p>
+
+</recommendation>
+<example>
+<p>The following example demonstrates various approaches to detecting memory allocation errors using the <code>new</code> operator.</p>
+<sample src="WrongInDetectingAndHandlingMemoryAllocationErrors.cpp" />
+
+</example>
+<references>
+
+<li>
+  CERT C++ Coding Standard:
+<a href="https://wiki.sei.cmu.edu/confluence/display/cplusplus/MEM52-CPP.+Detect+and+handle+memory+allocation+errors">MEM52-CPP. Detect and handle memory allocation errors</a>.
+</li>
+
+</references>
+</qhelp>

cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql

@@ -0,0 +1,87 @@
+/**
+ * @name Detect And Handle Memory Allocation Errors
+ * @description --::operator new(std::size_t) throws an exception on error, and ::operator new(std::size_t, const std::nothrow_t &) returns zero on error.
+ *              --the programmer can get confused when check the error that occurs when allocating memory incorrectly.
+ * @kind problem
+ * @id cpp/detect-and-handle-memory-allocation-errors
+ * @problem.severity warning
+ * @precision medium
+ * @tags correctness
+ *       security
+ *       external/cwe/cwe-570
+ */
+
+import cpp
+
+/**
+ * Lookup if condition compare with 0
+ */
+class IfCompareWithZero extends IfStmt {
+  IfCompareWithZero() {
+    this.getCondition().(EQExpr).getAChild().getValue() = "0"
+    or
+    this.getCondition().(NEExpr).getAChild().getValue() = "0" and
+    this.hasElse()
+    or
+    this.getCondition().(NEExpr).getAChild().getValue() = "0" and
+    this.getThen().getAChild*() instanceof ReturnStmt
+  }
+}
+
+/**
+ * lookup for calls to `operator new`, with incorrect error handling.
+ */
+class WrongCheckErrorOperatorNew extends FunctionCall {
+  Expr exp;
+
+  WrongCheckErrorOperatorNew() {
+    this = exp.(NewOrNewArrayExpr).getAChild().(FunctionCall) and
+    (
+      this.getTarget().hasGlobalOrStdName("operator new")
+      or
+      this.getTarget().hasGlobalOrStdName("operator new[]")
+    )
+  }
+
+  /**
+   * Holds if handler `try ... catch` exists.
+   */
+  predicate isExistsTryCatchBlock() {
+    exists(TryStmt ts | this.getEnclosingStmt() = ts.getStmt().getAChild*())
+  }
+
+  /**
+   * Holds if results call `operator new` check in `operator if`.
+   */
+  predicate isExistsIfCondition() {
+    exists(IfCompareWithZero ifc, AssignExpr aex, Initializer it |
+      // call `operator new` directly from the condition of `operator if`.
+      this = ifc.getCondition().getAChild*()
+      or
+      // check results call `operator new` with variable appropriation
+      postDominates(ifc, this) and
+      aex.getAChild() = exp and
+      ifc.getCondition().getAChild().(VariableAccess).getTarget() =
+        aex.getLValue().(VariableAccess).getTarget()
+      or
+      // check results call `operator new` with declaration variable
+      postDominates(ifc, this) and
+      exp = it.getExpr() and
+      it.getDeclaration() = ifc.getCondition().getAChild().(VariableAccess).getTarget()
+    )
+  }
+
+  /**
+   * Holds if `(std::nothrow)` or `(std::noexcept)` exists in call `operator new`.
+   */
+  predicate isExistsNothrow() { getTarget().isNoExcept() or getTarget().isNoThrow() }
+}
+
+from WrongCheckErrorOperatorNew op
+where
+  // use call `operator new` with `(std::nothrow)` and checking error using `try ... catch` block and not `operator if`
+  op.isExistsNothrow() and not op.isExistsIfCondition() and op.isExistsTryCatchBlock()
+  or
+  // use call `operator new` without `(std::nothrow)` and checking error using `operator if` and not  `try ... catch` block
+  not op.isExistsNothrow() and not op.isExistsTryCatchBlock() and op.isExistsIfCondition()
+select op, "memory allocation error check is incorrect or missing"

cpp/ql/src/experimental/Security/CWE/CWE-788/AccessOfMemoryLocationAfterEndOfBufferUsingStrlen.c

@@ -0,0 +1,9 @@
+// BAD: if buffer does not have a terminal zero, then access outside the allocated memory is possible.
+
+buffer[strlen(buffer)] = 0;
+
+
+// GOOD: we will eliminate dangerous behavior if we use a different method of calculating the length. 
+size_t len;
+...
+buffer[len] = 0

cpp/ql/src/experimental/Security/CWE/CWE-788/AccessOfMemoryLocationAfterEndOfBufferUsingStrlen.qhelp

@@ -0,0 +1,31 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Potentially dangerous use of the strlen function to calculate the length of a string.
+The expression <code>buffer[strlen(buffer)] = 0</code> is potentially dangerous, if the variable buffer does not have a terminal zero, then access beyond the bounds of the allocated memory is possible, which will lead to undefined behavior.
+If terminal zero is present, then the specified expression is meaningless.</p>
+
+<p>False positives include heavily nested strlen. This situation is unlikely.</p>
+
+</overview>
+<recommendation>
+
+<p>We recommend using another method for calculating the string length</p>
+
+</recommendation>
+<example>
+<p>The following example demonstrates an erroneous and corrected use of the strlen function.</p>
+<sample src="AccessOfMemoryLocationAfterEndOfBufferUsingStrlen.c" />
+
+</example>
+<references>
+
+<li>
+  CERT C Coding Standard:
+  <a href="https://wiki.sei.cmu.edu/confluence/display/c/STR32-C.+Do+not+pass+a+non-null-terminated+character+sequence+to+a+library+function+that+expects+a+string">STR32-C. Do not pass a non-null-terminated character sequence to a library function that expects a string</a>.
+</li>
+
+</references>
+</qhelp>

cpp/ql/src/experimental/Security/CWE/CWE-788/AccessOfMemoryLocationAfterEndOfBufferUsingStrlen.ql

@@ -0,0 +1,34 @@
+/**
+ * @name Access Of Memory Location After End Of Buffer
+ * @description The expression `buffer [strlen (buffer)] = 0` is potentially dangerous, if the variable `buffer` does not have a terminal zero, then access beyond the bounds of the allocated memory is possible, which will lead to undefined behavior.
+ *              If terminal zero is present, then the specified expression is meaningless.
+ * @kind problem
+ * @id cpp/access-memory-location-after-end-buffer
+ * @problem.severity warning
+ * @precision medium
+ * @tags correctness
+ *       security
+ *       external/cwe/cwe-788
+ */
+
+import cpp
+import semmle.code.cpp.valuenumbering.GlobalValueNumbering
+import semmle.code.cpp.dataflow.DataFlow
+
+from StrlenCall fc, AssignExpr expr, ArrayExpr exprarr
+where
+  exprarr = expr.getLValue() and
+  expr.getRValue().getValue().toInt() = 0 and
+  globalValueNumber(exprarr.getArrayOffset()) = globalValueNumber(fc) and
+  not exists(Expr exptmp |
+    (
+      DataFlow::localExprFlow(fc, exptmp) or
+      exptmp.getAChild*() = fc.getArgument(0).(VariableAccess).getTarget().getAnAccess()
+    ) and
+    dominates(exptmp, expr) and
+    postDominates(exptmp, fc) and
+    not exptmp.getEnclosingStmt() = fc.getEnclosingStmt() and
+    not exptmp.getEnclosingStmt() = expr.getEnclosingStmt()
+  ) and
+  globalValueNumber(fc.getArgument(0)) = globalValueNumber(exprarr.getArrayBase())
+select expr, "potential unsafe or redundant assignment."

cpp/ql/src/experimental/Security/CWE/CWE-788/AccessOfMemoryLocationAfterEndOfBufferUsingStrncat.c

@@ -0,0 +1,4 @@
+ 
+strncat(dest, source, sizeof(dest) - strlen(dest)); // BAD: writes a zero byte past the `dest` buffer.
+
+strncat(dest, source, sizeof(dest) - strlen(dest) -1); // GOOD: Reserves space for the zero byte.

cpp/ql/src/experimental/Security/CWE/CWE-788/AccessOfMemoryLocationAfterEndOfBufferUsingStrncat.qhelp

@@ -0,0 +1,32 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>The standard library function <code>strncat(dest, source, count)</code> appends the <code>source</code> string to the <code>dest</code> string. <code>count</code> specifies the maximum number of characters to append and must be less than the remaining space in the target buffer. Calls of the form <code> strncat (dest, source, sizeof (dest) - strlen (dest)) </code> set the third argument to one more than possible. So when the <code>dest</code> is full, the expression <code> sizeof (dest) - strlen (dest) </code> 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 <code>dest</code> buffer.</p>
+
+
+</overview>
+<recommendation>
+
+<p>We recommend subtracting one from the third argument. For example, replace <code>strncat(dest, source, sizeof(dest)-strlen(dest))</code> with <code>strncat(dest, source, sizeof(dest)-strlen(dest)-1)</code>.</p>
+
+</recommendation>
+<example>
+<p>The following example demonstrates an erroneous and corrected use of the <code>strncat</code> function.</p>
+<sample src="AccessOfMemoryLocationAfterEndOfBufferUsingStrncat.c" />
+
+</example>
+<references>
+
+<li>
+  CERT C Coding Standard:
+<a href="https://wiki.sei.cmu.edu/confluence/display/c/STR31-C.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator">STR31-C. Guarantee that storage for strings has sufficient space for character data and the null terminator</a>.
+</li>
+<li>
+  CERT C Coding Standard:
+  <a href="https://wiki.sei.cmu.edu/confluence/display/c/ARR30-C.+Do+not+form+or+use+out-of-bounds+pointers+or+array+subscripts">ARR30-C. Do not form or use out-of-bounds pointers or array subscripts</a>.
+</li>
+
+</references>
+</qhelp>

cpp/ql/src/experimental/Security/CWE/CWE-788/AccessOfMemoryLocationAfterEndOfBufferUsingStrncat.ql

@@ -0,0 +1,64 @@
+/**
+ * @name Access Of Memory Location After The End Of A Buffer Using Strncat
+ * @description Calls of the form `strncat(dest, source, sizeof (dest) - strlen (dest))` set the third argument to one more than possible. So when `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.
+ * @kind problem
+ * @id cpp/access-memory-location-after-end-buffer
+ * @problem.severity warning
+ * @precision medium
+ * @tags correctness
+ *       security
+ *       external/cwe/cwe-788
+ */
+
+import cpp
+import semmle.code.cpp.valuenumbering.GlobalValueNumbering
+
+/**
+ * A call to `strncat` of the form `strncat(buff, str, someExpr - strlen(buf))`, for some expression `someExpr` equal to `sizeof(buff)`.
+ */
+class WrongCallStrncat extends FunctionCall {
+  Expr leftsomeExpr;
+
+  WrongCallStrncat() {
+    this.getTarget().hasGlobalOrStdName("strncat") and
+    // the expression of the first argument in `strncat` and `strnlen` is identical
+    globalValueNumber(this.getArgument(0)) =
+      globalValueNumber(this.getArgument(2).(SubExpr).getRightOperand().(StrlenCall).getStringExpr()) and
+    // using a string constant often speaks of manually calculating the length of the required buffer.
+    (
+      not this.getArgument(1) instanceof StringLiteral and
+      not this.getArgument(1) instanceof CharLiteral
+    ) and
+    // for use in predicates
+    leftsomeExpr = this.getArgument(2).(SubExpr).getLeftOperand()
+  }
+
+  /**
+   * Holds if the left side of the expression `someExpr` equal to `sizeof(buf)`.
+   */
+  predicate isExpressionEqualSizeof() {
+    // the left side of the expression `someExpr` is `sizeof(buf)`.
+    globalValueNumber(this.getArgument(0)) =
+      globalValueNumber(leftsomeExpr.(SizeofExprOperator).getExprOperand())
+    or
+    // value of the left side of the expression `someExpr` equal  `sizeof(buf)` value, and `buf` is array.
+    leftsomeExpr.getValue().toInt() = this.getArgument(0).getType().getSize()
+  }
+
+  /**
+   * Holds if the left side of the expression `someExpr` equal to variable containing the length of the memory allocated for the buffer.
+   */
+  predicate isVariableEqualValueSizegBuffer() {
+    // the left side of expression `someExpr` is the variable that was used in the function of allocating memory for the buffer`.
+    exists(AllocationExpr alc |
+      leftsomeExpr.(VariableAccess).getTarget() =
+        alc.(FunctionCall).getArgument(0).(VariableAccess).getTarget()
+    )
+  }
+}
+
+from WrongCallStrncat sc
+where
+  sc.isExpressionEqualSizeof() or
+  sc.isVariableEqualValueSizegBuffer()
+select sc, "if the used buffer is full, writing out of the buffer is possible"

cpp/ql/src/external/tests/DefectFilter.ql

@@ -1,12 +0,0 @@
-/**
- * @name Defect filter
- * @description Only include results in large files (200) lines of code, and change the message.
- * @tags filter
- */
-
-import cpp
-import external.DefectFilter
-
-from DefectResult res
-where res.getFile().getMetrics().getNumberOfLinesOfCode() > 200
-select res, "Large files: " + res.getMessage()

cpp/ql/src/external/tests/DefectFromExternalData.ql

@@ -1,18 +0,0 @@
-/**
- * @name Defect from external data
- * @description Insert description here...
- * @kind problem
- * @problem.severity warning
- * @tags external-data
- */
-
-import cpp
-import external.ExternalArtifact
-
-from ExternalData d, File u
-where
-  d.getQueryPath() = "external-data.ql" and
-  u.getShortName() = d.getField(0)
-select u,
-  d.getField(5) + ", " + d.getFieldAsDate(1) + ", " + d.getField(2) + ", " + d.getFieldAsFloat(3) +
-    ", " + d.getFieldAsInt(4) + ": " + d.getNumFields()

cpp/ql/src/external/tests/MetricFilter.ql

@@ -1,12 +0,0 @@
-/**
- * @name Metric filter
- * @description Only include results in large files (200) lines of code.
- * @tags filter
- */
-
-import cpp
-import external.MetricFilter
-
-from MetricResult res
-where res.getFile().getMetrics().getNumberOfLinesOfCode() > 200
-select res, res.getValue()

cpp/ql/src/filters/FilterAutogenerated.ql

@@ -1,16 +0,0 @@
-/**
- * @name Filter: exclude results from files that are autogenerated
- * @description Use this filter to return results only if they are
- *              located in files that are maintained manually.
- * @kind problem
- * @id cpp/autogenerated-filter
- * @tags filter
- */
-
-import cpp
-import semmle.code.cpp.AutogeneratedFile
-import external.DefectFilter
-
-from DefectResult res
-where not res.getFile() instanceof AutogeneratedFile
-select res, res.getMessage()

cpp/ql/src/filters/FilterAutogeneratedForMetric.ql

@@ -1,16 +0,0 @@
-/**
- * @name Metric filter: exclude results from files that are autogenerated
- * @description Use this filter to return results only if they are
- *              located in files that are maintained manually.
- * @kind treemap
- * @id cpp/autogenerated-for-metric-filter
- * @tags filter
- */
-
-import cpp
-import semmle.code.cpp.AutogeneratedFile
-import external.MetricFilter
-
-from MetricResult res
-where not res.getFile() instanceof AutogeneratedFile
-select res, res.getValue()