Merge pull request #7667 from github/release-prep/2.7.6

Release preparation for version 2.7.6

.codeqlmanifest.json

@@ -11,7 +11,9 @@
         "misc/legacy-support/*/qlpack.yml",
         "misc/suite-helpers/qlpack.yml",
         "ruby/extractor-pack/codeql-extractor.yml",
-        "ruby/ql/consistency-queries/qlpack.yml"
+        "ruby/ql/consistency-queries/qlpack.yml",
+        "ql/ql/consistency-queries/qlpack.yml",
+        "ql/extractor-pack/codeql-extractor.yml"
     ],
     "versionPolicies": {
       "default": {

.github/labeler.yml

@@ -26,3 +26,6 @@ documentation:
   - "**/*.qhelp"
   - "**/*.md"
   - docs/**/*
+
+"QL-for-QL": 
+  - ql/**/*

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

@@ -7,6 +7,7 @@ on:
       - "*/ql/src/**/*.ql"
       - "*/ql/src/**/*.qll"
       - "!**/experimental/**"
+      - "!ql/**"
 
 jobs:
   check-change-note:

.github/workflows/ql-for-ql-build.yml

@@ -0,0 +1,192 @@
+name: Run QL for QL
+
+on:
+  push:
+    branches: [main]
+  pull_request:
+    branches: [main]
+
+env:
+  CARGO_TERM_COLOR: always
+
+jobs:
+  queries:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - name: Find codeql
+        id: find-codeql
+        uses: github/codeql-action/init@erik-krogh/ql
+        with:
+          languages: javascript # does not matter
+      - name: Get CodeQL version
+        id: get-codeql-version
+        run: |
+          echo "::set-output name=version::$("${CODEQL}" --version | head -n 1 | rev | cut -d " " -f 1 | rev)"
+        shell: bash
+        env:
+          CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}
+      - name: Cache queries
+        id: cache-queries
+        uses: actions/cache@v2
+        with:
+          path: ${{ runner.temp }}/query-pack.zip
+          key: queries-${{ hashFiles('ql/**/*.ql*') }}-${{ hashFiles('ql/ql/src/ql.dbscheme*') }}-${{ steps.get-codeql-version.outputs.version }}
+      - name: Build query pack
+        if: steps.cache-queries.outputs.cache-hit != 'true'
+        run: |
+          cd ql/ql/src
+          "${CODEQL}" pack create
+          cd .codeql/pack/codeql/ql-all/0.0.0
+          zip "${PACKZIP}" -r .
+        env:
+          CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}
+          PACKZIP: ${{ runner.temp }}/query-pack.zip
+      - name: Upload query pack
+        uses: actions/upload-artifact@v2
+        with:
+          name: query-pack-zip
+          path: ${{ runner.temp }}/query-pack.zip
+
+  extractors:
+    strategy:
+      fail-fast: false
+
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v2
+      - name: Cache entire extractor
+        id: cache-extractor
+        uses: actions/cache@v2
+        with:
+          path: |
+            ql/target/release/ql-autobuilder
+            ql/target/release/ql-autobuilder.exe
+            ql/target/release/ql-extractor
+            ql/target/release/ql-extractor.exe
+          key: ${{ runner.os }}-extractor-${{ hashFiles('ql/**/Cargo.lock') }}-${{ hashFiles('ql/**/*.rs') }}
+      - name: Cache cargo
+        if: steps.cache-extractor.outputs.cache-hit != 'true'
+        uses: actions/cache@v2
+        with:
+          path: |
+            ~/.cargo/registry
+            ~/.cargo/git
+            ql/target
+          key: ${{ runner.os }}-rust-cargo-${{ hashFiles('ql/**/Cargo.lock') }}
+      - name: Check formatting
+        if: steps.cache-extractor.outputs.cache-hit != 'true'
+        run: cd ql; cargo fmt --all -- --check
+      - name: Build
+        if: steps.cache-extractor.outputs.cache-hit != 'true'
+        run: cd ql; cargo build --verbose
+      - name: Run tests
+        if: steps.cache-extractor.outputs.cache-hit != 'true'
+        run: cd ql; cargo test --verbose
+      - name: Release build
+        if: steps.cache-extractor.outputs.cache-hit != 'true'
+        run: cd ql; cargo build --release
+      - name: Generate dbscheme
+        if: steps.cache-extractor.outputs.cache-hit != 'true'
+        run: ql/target/release/ql-generator --dbscheme ql/ql/src/ql.dbscheme --library ql/ql/src/codeql_ql/ast/internal/TreeSitter.qll
+      - uses: actions/upload-artifact@v2
+        with:
+          name: extractor-ubuntu-latest
+          path: |
+            ql/target/release/ql-autobuilder
+            ql/target/release/ql-autobuilder.exe
+            ql/target/release/ql-extractor
+            ql/target/release/ql-extractor.exe
+          retention-days: 1
+  package:
+    runs-on: ubuntu-latest
+
+    needs:
+      - extractors
+      - queries
+
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions/download-artifact@v2
+        with:
+          name: query-pack-zip
+          path: query-pack-zip
+      - uses: actions/download-artifact@v2
+        with:
+          name: extractor-ubuntu-latest
+          path: linux64
+      - run: |
+          unzip query-pack-zip/*.zip -d pack
+          cp -r ql/codeql-extractor.yml ql/tools ql/ql/src/ql.dbscheme.stats pack/
+          mkdir -p pack/tools/linux64
+          if [[ -f linux64/ql-autobuilder ]]; then
+            cp linux64/ql-autobuilder pack/tools/linux64/autobuilder
+            chmod +x pack/tools/linux64/autobuilder
+          fi
+          if [[ -f linux64/ql-extractor ]]; then
+            cp linux64/ql-extractor pack/tools/linux64/extractor
+            chmod +x pack/tools/linux64/extractor
+          fi
+          cd pack
+          zip -rq ../codeql-ql.zip .
+      - uses: actions/upload-artifact@v2
+        with:
+          name: codeql-ql-pack
+          path: codeql-ql.zip
+          retention-days: 1
+  analyze:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        folder: [cpp, csharp, java, javascript, python, ql, ruby]
+
+    needs:
+      - package
+
+    steps:
+      - name: Download pack
+        uses: actions/download-artifact@v2
+        with:
+          name: codeql-ql-pack
+          path: ${{ runner.temp }}/codeql-ql-pack-artifact
+
+      - name: Prepare pack
+        run: |
+          unzip "${PACK_ARTIFACT}/*.zip" -d "${PACK}"
+        env:
+          PACK_ARTIFACT: ${{ runner.temp }}/codeql-ql-pack-artifact
+          PACK: ${{ runner.temp }}/pack
+      - name: Hack codeql-action options
+        run: |
+          JSON=$(jq -nc --arg pack "${PACK}" '.resolve.queries=["--search-path", $pack] | .resolve.extractor=["--search-path", $pack] | .database.init=["--search-path", $pack]')
+          echo "CODEQL_ACTION_EXTRA_OPTIONS=${JSON}" >> ${GITHUB_ENV}
+        env:
+          PACK: ${{ runner.temp }}/pack
+
+      - name: Checkout repository
+        uses: actions/checkout@v2
+      - name: Create CodeQL config file
+        run: |
+          echo "paths:" > ${CONF}
+          echo "  - ${FOLDER}" >> ${CONF}
+          echo "paths-ignore:" >> ${CONF}
+          echo "  - ql/ql/test" >> ${CONF}
+          echo "Config file: "
+          cat ${CONF}
+        env: 
+          CONF: ./ql-for-ql-config.yml
+          FOLDER: ${{ matrix.folder }}
+
+      - name: Initialize CodeQL
+        uses: github/codeql-action/init@erik-krogh/ql
+        with:
+          languages: ql
+          db-location: ${{ runner.temp }}/db
+          config-file: ./ql-for-ql-config.yml
+
+      - name: Perform CodeQL Analysis
+        uses: github/codeql-action/analyze@erik-krogh/ql
+        with: 
+          category: "ql-for-ql-${{ matrix.folder }}"
+

.github/workflows/ql-for-ql-dataset_measure.yml

@@ -0,0 +1,84 @@
+name: Collect database stats for QL for QL
+
+on:
+  push:
+    branches: [main]
+    paths:
+      - ql/ql/src/ql.dbscheme
+  pull_request:
+    branches: [main]
+    paths:
+      - ql/ql/src/ql.dbscheme
+  workflow_dispatch:
+
+jobs:
+  measure:
+    env:
+      CODEQL_THREADS: 4 # TODO: remove this once it's set by the CLI
+    strategy:
+      matrix:
+        repo: 
+          - github/codeql
+          - github/codeql-go
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+
+      - name: Find codeql
+        id: find-codeql
+        uses: github/codeql-action/init@erik-krogh/ql
+        with:
+          languages: javascript # does not matter
+      - uses: actions/cache@v2
+        with:
+          path: |
+            ~/.cargo/registry
+            ~/.cargo/git
+            ql/target
+          key: ${{ runner.os }}-qltest-cargo-${{ hashFiles('**/Cargo.lock') }}
+      - name: Build Extractor
+        run: cd ql; env "PATH=$PATH:`dirname ${CODEQL}`" ./create-extractor-pack.sh
+        env:
+          CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}
+      - name: Checkout ${{ matrix.repo }}
+        uses: actions/checkout@v2
+        with:
+          repository: ${{ matrix.repo }}
+          path: ${{ github.workspace }}/repo
+      - name: Create database
+        run: |
+          "${CODEQL}" database create \
+            --search-path "ql/extractor-pack" \
+            --threads 4 \
+            --language ql --source-root "${{ github.workspace }}/repo" \
+            "${{ runner.temp }}/database"
+        env:
+          CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}
+      - name: Measure database
+        run: |
+          mkdir -p "stats/${{ matrix.repo }}"
+          "${CODEQL}" dataset measure --threads 4 --output "stats/${{ matrix.repo }}/stats.xml" "${{ runner.temp }}/database/db-ql"
+        env:
+          CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}
+      - uses: actions/upload-artifact@v2
+        with:
+          name: measurements
+          path: stats
+          retention-days: 1
+
+  merge:
+    runs-on: ubuntu-latest
+    needs: measure
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions/download-artifact@v2
+        with:
+          name: measurements
+          path: stats
+      - run: |
+          python -m pip install --user lxml
+          find stats -name 'stats.xml' -print0 | sort -z | xargs -0 python ql/scripts/merge_stats.py --output ql/ql/src/ql.dbscheme.stats --normalise ql_tokeninfo
+      - uses: actions/upload-artifact@v2
+        with:
+          name: ql.dbscheme.stats
+          path: ql/ql/src/ql.dbscheme.stats

.github/workflows/ql-for-ql-tests.yml

@@ -0,0 +1,52 @@
+name: Run QL for QL Tests
+
+on:
+  push:
+    branches: [main]
+    paths:
+      - "ql/**"
+  pull_request:
+    branches: [main]
+    paths:
+      - "ql/**"
+
+env:
+  CARGO_TERM_COLOR: always
+
+jobs:
+  qltest:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - name: Find codeql
+        id: find-codeql
+        uses: github/codeql-action/init@erik-krogh/ql
+        with:
+          languages: javascript # does not matter
+      - uses: actions/cache@v2
+        with:
+          path: |
+            ~/.cargo/registry
+            ~/.cargo/git
+            ql/target
+          key: ${{ runner.os }}-qltest-cargo-${{ hashFiles('**/Cargo.lock') }}
+      - name: Build extractor
+        run: |
+          cd ql;
+          codeqlpath=$(dirname ${{ steps.find-codeql.outputs.codeql-path }});
+          env "PATH=$PATH:$codeqlpath" ./create-extractor-pack.sh
+      - name: Run QL tests
+        run: | 
+          "${CODEQL}" test run --check-databases --check-unused-labels --check-repeated-labels --check-redefined-labels --check-use-before-definition --search-path "${{ github.workspace }}/ql/extractor-pack" --consistency-queries ql/ql/consistency-queries ql/ql/test
+        env:
+          CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}
+      - name: Check QL formatting
+        run: | 
+          find ql/ql "(" -name "*.ql" -or -name "*.qll" ")" -print0 | xargs -0 "${CODEQL}" query format --check-only
+        env:
+          CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}
+      - name: Check QL compilation
+        run: | 
+          "${CODEQL}" query compile --check-only --threads=4 --warnings=error --search-path "${{ github.workspace }}/ql/extractor-pack" "ql/ql/src" "ql/ql/examples"
+        env:
+          CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}

CODEOWNERS

@@ -25,3 +25,6 @@
 /docs/codeql-for-visual-studio-code/ @github/codeql-vscode-reviewers
 /docs/ql-language-reference/ @github/codeql-frontend-reviewers
 /docs/query-*-style-guide.md @github/codeql-analysis-reviewers
+
+# QL for QL reviewers
+/ql/ @erik-krogh @tausbn

config/identical-files.json

@@ -452,9 +452,15 @@
     "ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImplCommon.qll",
     "cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImplCommon.qll"
   ],
-  "CryptoAlgorithms Python/JS": [
+  "CryptoAlgorithms Python/JS/Ruby": [
     "javascript/ql/lib/semmle/javascript/security/CryptoAlgorithms.qll",
-    "python/ql/lib/semmle/python/concepts/CryptoAlgorithms.qll"
+    "python/ql/lib/semmle/python/concepts/CryptoAlgorithms.qll",
+    "ruby/ql/lib/codeql/ruby/security/CryptoAlgorithms.qll"
+  ],
+  "CryptoAlgorithmNames Python/JS/Ruby": [
+    "javascript/ql/lib/semmle/javascript/security/internal/CryptoAlgorithmNames.qll",
+    "python/ql/lib/semmle/python/concepts/internal/CryptoAlgorithmNames.qll",
+    "ruby/ql/lib/codeql/ruby/security/internal/CryptoAlgorithmNames.qll"
   ],
   "SensitiveDataHeuristics Python/JS": [
     "javascript/ql/lib/semmle/javascript/security/internal/SensitiveDataHeuristics.qll",

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

@@ -17,7 +17,7 @@
   </ItemGroup>
 
   <ItemGroup>
-    <PackageReference Include="Microsoft.Build" Version="16.9.0" />
+    <PackageReference Include="Microsoft.Build" Version="16.11.0" />
   </ItemGroup>
 
   <ItemGroup>

cpp/change-notes/2021-11-01-isFromSystemMacroDefinition.md

@@ -1,4 +0,0 @@
-lgtm,codescanning
-* The QL library `semmle.code.cpp.commons.Exclusions` now contains a predicate
-  `isFromSystemMacroDefinition` for identifying code that originates from a
-  macro outside the project being analyzed.

cpp/change-notes/2021-11-09-use-of-http.md

@@ -1,2 +0,0 @@
-lgtm,codescanning
-* A new query `cpp/non-https-url` has been added for C/C++. The query flags uses of `http` URLs that might be better replaced with `https`.

cpp/change-notes/2021-11-25-certificate-not-checked.md

@@ -1,2 +0,0 @@
-lgtm,codescanning
-* A new query `cpp/certificate-not-checked` has been added for C/C++. The query flags unsafe use of OpenSSL and similar libraries.

cpp/change-notes/2021-11-25-certificate-result-conflation.md

@@ -1,2 +0,0 @@
-lgtm,codescanning
-* A new query `cpp/certificate-result-conflation` has been added for C/C++. The query flags unsafe use of OpenSSL and similar libraries.

cpp/old-change-notes/2021-10-07-cleartext-transmission.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* The "Cleartext transmission of sensitive information" (`cpp/cleartext-transmission`) query has been improved, reducing the number of false positive results when encryption is present.

cpp/ql/lib/CHANGELOG.md

@@ -1,3 +1,9 @@
+## 0.0.7
+
+## 0.0.6
+
+## 0.0.5
+
 ## 0.0.4
 
 ### New Features

cpp/ql/lib/DefaultOptions.qll

@@ -73,7 +73,7 @@ class Options extends string {
    *   __assume(0);
    * ```
    * (note that in this case if the hint is wrong and the expression is reached at
-   * runtime, the program's behaviour is undefined)
+   * runtime, the program's behavior is undefined)
    */
   predicate exprExits(Expr e) {
     e.(AssumeExpr).getChild(0).(CompileTimeConstantInt).getIntValue() = 0 or

cpp/ql/lib/Options.qll

@@ -50,7 +50,7 @@ class CustomOptions extends Options {
    *   __assume(0);
    * ```
    * (note that in this case if the hint is wrong and the expression is reached at
-   * runtime, the program's behaviour is undefined)
+   * runtime, the program's behavior is undefined)
    */
   override predicate exprExits(Expr e) { Options.super.exprExits(e) }
 

cpp/ql/lib/change-notes/released/0.0.5.md

@@ -0,0 +1 @@
+## 0.0.5

cpp/ql/lib/change-notes/released/0.0.6.md

@@ -0,0 +1 @@
+## 0.0.6

cpp/ql/lib/change-notes/released/0.0.7.md

@@ -0,0 +1 @@
+## 0.0.7

cpp/ql/lib/codeql-pack.release.yml

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.0.4
+lastReleaseVersion: 0.0.7

cpp/ql/lib/experimental/semmle/code/cpp/models/interfaces/SimpleRangeAnalysisDefinition.qll

@@ -37,7 +37,7 @@ abstract class SimpleRangeAnalysisDefinition extends RangeSsaDefinition {
    * dependencies. Without this information, range analysis might work for
    * simple cases but will go into infinite loops on complex code.
    *
-   * For example, when modelling the definition by reference in a call to an
+   * For example, when modeling the definition by reference in a call to an
    * overloaded `operator=`, written as `v = e`, the definition of `(this, v)`
    * depends on `e`.
    */

cpp/ql/lib/experimental/semmle/code/cpp/rangeanalysis/InBoundsPointerDeref.qll

@@ -5,7 +5,7 @@
  * `Instruction` level), and then using the array length analysis and the range
  * analysis together to prove that some of these pointer dereferences are safe.
  *
- * The analysis is soundy, i.e. it is sound if no undefined behaviour is present
+ * The analysis is soundy, i.e. it is sound if no undefined behavior is present
  * in the program.
  * Furthermore, it crucially depends on the soundiness of the range analysis and
  * the array length analysis.

cpp/ql/lib/qlpack.yml

@@ -1,8 +1,8 @@
 name: codeql/cpp-all
-version: 0.0.5-dev
+version: 0.0.7
 groups: cpp
 dbscheme: semmlecode.cpp.dbscheme
 extractor: cpp
 library: true
 dependencies:
-  codeql/cpp-upgrades: 0.0.3
+  codeql/cpp-upgrades: ^0.0.3

cpp/ql/lib/semmle/code/cpp/commons/Printf.qll

@@ -9,6 +9,83 @@ import semmle.code.cpp.models.interfaces.FormattingFunction
 private import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
 private import semmle.code.cpp.rangeanalysis.RangeAnalysisUtils
 
+private newtype TBufferWriteEstimationReason =
+  TNoSpecifiedEstimateReason() or
+  TTypeBoundsAnalysis() or
+  TValueFlowAnalysis()
+
+/**
+ * A reason for a specific buffer write size estimate.
+ */
+abstract class BufferWriteEstimationReason extends TBufferWriteEstimationReason {
+  /**
+   * Returns the name of the concrete class.
+   */
+  abstract string toString();
+
+  /**
+   * Returns a human readable representation of this reason.
+   */
+  abstract string getDescription();
+
+  /**
+   * Combine estimate reasons. Used to give a reason for the size of a format string
+   * conversion given reasons coming from its individual specifiers.
+   */
+  abstract BufferWriteEstimationReason combineWith(BufferWriteEstimationReason other);
+}
+
+/**
+ * No particular reason given. This is currently used for backward compatibility so that
+ * classes derived from BufferWrite and overriding `getMaxData/0` still work with the
+ * queries as intended.
+ */
+class NoSpecifiedEstimateReason extends BufferWriteEstimationReason, TNoSpecifiedEstimateReason {
+  override string toString() { result = "NoSpecifiedEstimateReason" }
+
+  override string getDescription() { result = "no reason specified" }
+
+  override BufferWriteEstimationReason combineWith(BufferWriteEstimationReason other) {
+    // this reason should not be used in format specifiers, so it should not be combined
+    // with other reasons
+    none()
+  }
+}
+
+/**
+ * The estimation comes from rough bounds just based on the type (e.g.
+ * `0 <= x < 2^32` for an unsigned 32 bit integer).
+ */
+class TypeBoundsAnalysis extends BufferWriteEstimationReason, TTypeBoundsAnalysis {
+  override string toString() { result = "TypeBoundsAnalysis" }
+
+  override string getDescription() { result = "based on type bounds" }
+
+  override BufferWriteEstimationReason combineWith(BufferWriteEstimationReason other) {
+    other != TNoSpecifiedEstimateReason() and result = TTypeBoundsAnalysis()
+  }
+}
+
+/**
+ * The estimation comes from non trivial bounds found via actual flow analysis.
+ * For example
+ * ```
+ * unsigned u = x;
+ * if (u < 1000) {
+ *    //...  <- estimation done here based on u
+ * }
+ * ```
+ */
+class ValueFlowAnalysis extends BufferWriteEstimationReason, TValueFlowAnalysis {
+  override string toString() { result = "ValueFlowAnalysis" }
+
+  override string getDescription() { result = "based on flow analysis of value bounds" }
+
+  override BufferWriteEstimationReason combineWith(BufferWriteEstimationReason other) {
+    other != TNoSpecifiedEstimateReason() and result = other
+  }
+}
+
 class PrintfFormatAttribute extends FormatAttribute {
   PrintfFormatAttribute() { this.getArchetype() = ["printf", "__printf__"] }
 }
@@ -990,7 +1067,14 @@ class FormatLiteral extends Literal {
    * conversion specifier of this format string; has no result if this cannot
    * be determined.
    */
-  int getMaxConvertedLength(int n) {
+  int getMaxConvertedLength(int n) { result = max(getMaxConvertedLength(n, _)) }
+
+  /**
+   * Gets the maximum length of the string that can be produced by the nth
+   * conversion specifier of this format string, specifying the estimation reason;
+   * has no result if this cannot be determined.
+   */
+  int getMaxConvertedLength(int n, BufferWriteEstimationReason reason) {
     exists(int len |
       (
         (
@@ -1002,10 +1086,12 @@ class FormatLiteral extends Literal {
       ) and
       (
         this.getConversionChar(n) = "%" and
-        len = 1
+        len = 1 and
+        reason = TValueFlowAnalysis()
         or
         this.getConversionChar(n).toLowerCase() = "c" and
-        len = 1 // e.g. 'a'
+        len = 1 and
+        reason = TValueFlowAnalysis() // e.g. 'a'
         or
         this.getConversionChar(n).toLowerCase() = "f" and
         exists(int dot, int afterdot |
@@ -1019,7 +1105,8 @@ class FormatLiteral extends Literal {
             afterdot = 6
           ) and
           len = 1 + 309 + dot + afterdot
-        ) // e.g. -1e308="-100000"...
+        ) and
+        reason = TTypeBoundsAnalysis() // e.g. -1e308="-100000"...
         or
         this.getConversionChar(n).toLowerCase() = "e" and
         exists(int dot, int afterdot |
@@ -1033,7 +1120,8 @@ class FormatLiteral extends Literal {
             afterdot = 6
           ) and
           len = 1 + 1 + dot + afterdot + 1 + 1 + 3
-        ) // -1e308="-1.000000e+308"
+        ) and
+        reason = TTypeBoundsAnalysis() // -1e308="-1.000000e+308"
         or
         this.getConversionChar(n).toLowerCase() = "g" and
         exists(int dot, int afterdot |
@@ -1056,67 +1144,80 @@ class FormatLiteral extends Literal {
           //       (e.g. 123456, 0.000123456 are just OK)
           //       so case %f can be at most P characters + 4 zeroes, sign, dot = P + 6
           len = (afterdot.maximum(1) + 6).maximum(1 + 1 + dot + afterdot + 1 + 1 + 3)
-        ) // (e.g. "-1.59203e-319")
+        ) and
+        reason = TTypeBoundsAnalysis() // (e.g. "-1.59203e-319")
         or
         this.getConversionChar(n).toLowerCase() = ["d", "i"] and
         // e.g. -2^31 = "-2147483648"
-        len =
-          min(float cand |
-            // The first case handles length sub-specifiers
-            // Subtract one in the exponent because one bit is for the sign.
-            // Add 1 to account for the possible sign in the output.
-            cand = 1 + lengthInBase10(2.pow(this.getIntegralDisplayType(n).getSize() * 8 - 1))
-            or
-            // The second case uses range analysis to deduce a length that's shorter than the length
-            // of the number -2^31.
-            exists(Expr arg, float lower, float upper |
-              arg = this.getUse().getConversionArgument(n) and
-              lower = lowerBound(arg.getFullyConverted()) and
-              upper = upperBound(arg.getFullyConverted())
-            |
-              cand =
-                max(int cand0 |
-                  // Include the sign bit in the length if it can be negative
-                  (
-                    if lower < 0
-                    then cand0 = 1 + lengthInBase10(lower.abs())
-                    else cand0 = lengthInBase10(lower)
-                  )
-                  or
-                  (
-                    if upper < 0
-                    then cand0 = 1 + lengthInBase10(upper.abs())
-                    else cand0 = lengthInBase10(upper)
-                  )
+        exists(float typeBasedBound, float valueBasedBound |
+          // The first case handles length sub-specifiers
+          // Subtract one in the exponent because one bit is for the sign.
+          // Add 1 to account for the possible sign in the output.
+          typeBasedBound =
+            1 + lengthInBase10(2.pow(this.getIntegralDisplayType(n).getSize() * 8 - 1)) and
+          // The second case uses range analysis to deduce a length that's shorter than the length
+          // of the number -2^31.
+          exists(Expr arg, float lower, float upper, float typeLower, float typeUpper |
+            arg = this.getUse().getConversionArgument(n) and
+            lower = lowerBound(arg.getFullyConverted()) and
+            upper = upperBound(arg.getFullyConverted()) and
+            typeLower = exprMinVal(arg.getFullyConverted()) and
+            typeUpper = exprMaxVal(arg.getFullyConverted())
+          |
+            valueBasedBound =
+              max(int cand |
+                // Include the sign bit in the length if it can be negative
+                (
+                  if lower < 0
+                  then cand = 1 + lengthInBase10(lower.abs())
+                  else cand = lengthInBase10(lower)
                 )
+                or
+                (
+                  if upper < 0
+                  then cand = 1 + lengthInBase10(upper.abs())
+                  else cand = lengthInBase10(upper)
+                )
+              ) and
+            (
+              if lower > typeLower or upper < typeUpper
+              then reason = TValueFlowAnalysis()
+              else reason = TTypeBoundsAnalysis()
             )
-          )
+          ) and
+          len = valueBasedBound.minimum(typeBasedBound)
+        )
         or
         this.getConversionChar(n).toLowerCase() = "u" and
         // e.g. 2^32 - 1 = "4294967295"
-        len =
-          min(float cand |
-            // The first case handles length sub-specifiers
-            cand = 2.pow(this.getIntegralDisplayType(n).getSize() * 8)
-            or
-            // The second case uses range analysis to deduce a length that's shorter than
-            // the length of the number 2^31 - 1.
-            exists(Expr arg, float lower |
-              arg = this.getUse().getConversionArgument(n) and
-              lower = lowerBound(arg.getFullyConverted())
-            |
-              cand =
-                max(float cand0 |
+        exists(float typeBasedBound, float valueBasedBound |
+          // The first case handles length sub-specifiers
+          typeBasedBound = lengthInBase10(2.pow(this.getIntegralDisplayType(n).getSize() * 8) - 1) and
+          // The second case uses range analysis to deduce a length that's shorter than
+          // the length of the number 2^31 - 1.
+          exists(Expr arg, float lower, float upper, float typeLower, float typeUpper |
+            arg = this.getUse().getConversionArgument(n) and
+            lower = lowerBound(arg.getFullyConverted()) and
+            upper = upperBound(arg.getFullyConverted()) and
+            typeLower = exprMinVal(arg.getFullyConverted()) and
+            typeUpper = exprMaxVal(arg.getFullyConverted())
+          |
+            valueBasedBound =
+              lengthInBase10(max(float cand |
                   // If lower can be negative we use `(unsigned)-1` as the candidate value.
                   lower < 0 and
-                  cand0 = 2.pow(any(IntType t | t.isUnsigned()).getSize() * 8)
+                  cand = 2.pow(any(IntType t | t.isUnsigned()).getSize() * 8)
                   or
-                  cand0 = upperBound(arg.getFullyConverted())
-                )
+                  cand = upper
+                )) and
+            (
+              if lower > typeLower or upper < typeUpper
+              then reason = TValueFlowAnalysis()
+              else reason = TTypeBoundsAnalysis()
             )
-          |
-            lengthInBase10(cand)
-          )
+          ) and
+          len = valueBasedBound.minimum(typeBasedBound)
+        )
         or
         this.getConversionChar(n).toLowerCase() = "x" and
         // e.g. "12345678"
@@ -1135,7 +1236,8 @@ class FormatLiteral extends Literal {
           (
             if this.hasAlternateFlag(n) then len = 2 + baseLen else len = baseLen // "0x"
           )
-        )
+        ) and
+        reason = TTypeBoundsAnalysis()
         or
         this.getConversionChar(n).toLowerCase() = "p" and
         exists(PointerType ptrType, int baseLen |
@@ -1144,7 +1246,8 @@ class FormatLiteral extends Literal {
           (
             if this.hasAlternateFlag(n) then len = 2 + baseLen else len = baseLen // "0x"
           )
-        )
+        ) and
+        reason = TValueFlowAnalysis()
         or
         this.getConversionChar(n).toLowerCase() = "o" and
         // e.g. 2^32 - 1 = "37777777777"
@@ -1163,14 +1266,16 @@ class FormatLiteral extends Literal {
           (
             if this.hasAlternateFlag(n) then len = 1 + baseLen else len = baseLen // "0"
           )
-        )
+        ) and
+        reason = TTypeBoundsAnalysis()
         or
         this.getConversionChar(n).toLowerCase() = "s" and
         len =
           min(int v |
             v = this.getPrecision(n) or
             v = this.getUse().getFormatArgument(n).(AnalysedString).getMaxLength() - 1 // (don't count null terminator)
-          )
+          ) and
+        reason = TValueFlowAnalysis()
       )
     )
   }
@@ -1182,10 +1287,19 @@ class FormatLiteral extends Literal {
    * determining whether a buffer overflow is caused by long float to string
    * conversions.
    */
-  int getMaxConvertedLengthLimited(int n) {
+  int getMaxConvertedLengthLimited(int n) { result = max(getMaxConvertedLengthLimited(n, _)) }
+
+  /**
+   * Gets the maximum length of the string that can be produced by the nth
+   * conversion specifier of this format string, specifying the reason for the
+   * estimation, except that float to string conversions are assumed to be 8
+   * characters.  This is helpful for determining whether a buffer overflow is
+   * caused by long float to string conversions.
+   */
+  int getMaxConvertedLengthLimited(int n, BufferWriteEstimationReason reason) {
     if this.getConversionChar(n).toLowerCase() = "f"
-    then result = this.getMaxConvertedLength(n).minimum(8)
-    else result = this.getMaxConvertedLength(n)
+    then result = this.getMaxConvertedLength(n, reason).minimum(8)
+    else result = this.getMaxConvertedLength(n, reason)
   }
 
   /**
@@ -1225,29 +1339,35 @@ class FormatLiteral extends Literal {
     )
   }
 
-  private int getMaxConvertedLengthAfter(int n) {
+  private int getMaxConvertedLengthAfter(int n, BufferWriteEstimationReason reason) {
     if n = this.getNumConvSpec()
-    then result = this.getConstantSuffix().length() + 1
+    then result = this.getConstantSuffix().length() + 1 and reason = TValueFlowAnalysis()
     else
-      result =
-        this.getConstantPart(n).length() + this.getMaxConvertedLength(n) +
-          this.getMaxConvertedLengthAfter(n + 1)
+      exists(BufferWriteEstimationReason headReason, BufferWriteEstimationReason tailReason |
+        result =
+          this.getConstantPart(n).length() + this.getMaxConvertedLength(n, headReason) +
+            this.getMaxConvertedLengthAfter(n + 1, tailReason) and
+        reason = headReason.combineWith(tailReason)
+      )
   }
 
-  private int getMaxConvertedLengthAfterLimited(int n) {
+  private int getMaxConvertedLengthAfterLimited(int n, BufferWriteEstimationReason reason) {
     if n = this.getNumConvSpec()
-    then result = this.getConstantSuffix().length() + 1
+    then result = this.getConstantSuffix().length() + 1 and reason = TValueFlowAnalysis()
     else
-      result =
-        this.getConstantPart(n).length() + this.getMaxConvertedLengthLimited(n) +
-          this.getMaxConvertedLengthAfterLimited(n + 1)
+      exists(BufferWriteEstimationReason headReason, BufferWriteEstimationReason tailReason |
+        result =
+          this.getConstantPart(n).length() + this.getMaxConvertedLengthLimited(n, headReason) +
+            this.getMaxConvertedLengthAfterLimited(n + 1, tailReason) and
+        reason = headReason.combineWith(tailReason)
+      )
   }
 
   /**
    * Gets the maximum length of the string that can be produced by this format
    * string.  Has no result if this cannot be determined.
    */
-  int getMaxConvertedLength() { result = this.getMaxConvertedLengthAfter(0) }
+  int getMaxConvertedLength() { result = this.getMaxConvertedLengthAfter(0, _) }
 
   /**
    * Gets the maximum length of the string that can be produced by this format
@@ -1255,5 +1375,24 @@ class FormatLiteral extends Literal {
    * characters.  This is helpful for determining whether a buffer overflow
    * is caused by long float to string conversions.
    */
-  int getMaxConvertedLengthLimited() { result = this.getMaxConvertedLengthAfterLimited(0) }
+  int getMaxConvertedLengthLimited() { result = this.getMaxConvertedLengthAfterLimited(0, _) }
+
+  /**
+   * Gets the maximum length of the string that can be produced by this format
+   * string, specifying the reason for the estimate. Has no result if no estimate
+   * can be found.
+   */
+  int getMaxConvertedLengthWithReason(BufferWriteEstimationReason reason) {
+    result = this.getMaxConvertedLengthAfter(0, reason)
+  }
+
+  /**
+   * Gets the maximum length of the string that can be produced by this format
+   * string, specifying the reason for the estimate, except that float to string
+   * conversions are assumed to be 8 characters.  This is helpful for determining
+   * whether a buffer overflow is caused by long float to string conversions.
+   */
+  int getMaxConvertedLengthLimitedWithReason(BufferWriteEstimationReason reason) {
+    result = this.getMaxConvertedLengthAfterLimited(0, reason)
+  }
 }

cpp/ql/lib/semmle/code/cpp/controlflow/SSAUtils.qll

@@ -153,9 +153,11 @@ library class SSAHelper extends int {
    * Modern Compiler Implementation by Andrew Appel.
    */
   private predicate frontier_phi_node(StackVariable v, BasicBlock b) {
-    exists(BasicBlock x | dominanceFrontier(x, b) and ssa_defn_rec(v, x)) and
+    exists(BasicBlock x |
+      dominanceFrontier(x, b) and ssa_defn_rec(pragma[only_bind_into](v), pragma[only_bind_into](x))
+    ) and
     /* We can also eliminate those nodes where the variable is not live on any incoming edge */
-    live_at_start_of_bb(v, b)
+    live_at_start_of_bb(pragma[only_bind_into](v), b)
   }
 
   private predicate ssa_defn_rec(StackVariable v, BasicBlock b) {

cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplConsistency.qll

@@ -15,11 +15,23 @@ module Consistency {
   class ConsistencyConfiguration extends TConsistencyConfiguration {
     string toString() { none() }
 
+    /** Holds if `n` should be excluded from the consistency test `uniqueEnclosingCallable`. */
+    predicate uniqueEnclosingCallableExclude(Node n) { none() }
+
+    /** Holds if `n` should be excluded from the consistency test `uniqueNodeLocation`. */
+    predicate uniqueNodeLocationExclude(Node n) { none() }
+
+    /** Holds if `n` should be excluded from the consistency test `missingLocation`. */
+    predicate missingLocationExclude(Node n) { none() }
+
     /** Holds if `n` should be excluded from the consistency test `postWithInFlow`. */
     predicate postWithInFlowExclude(Node n) { none() }
 
     /** Holds if `n` should be excluded from the consistency test `argHasPostUpdate`. */
     predicate argHasPostUpdateExclude(ArgumentNode n) { none() }
+
+    /** Holds if `n` should be excluded from the consistency test `reverseRead`. */
+    predicate reverseReadExclude(Node n) { none() }
   }
 
   private class RelevantNode extends Node {
@@ -46,6 +58,7 @@ module Consistency {
       n instanceof RelevantNode and
       c = count(nodeGetEnclosingCallable(n)) and
       c != 1 and
+      not any(ConsistencyConfiguration conf).uniqueEnclosingCallableExclude(n) and
       msg = "Node should have one enclosing callable but has " + c + "."
     )
   }
@@ -66,6 +79,7 @@ module Consistency {
           n.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
         ) and
       c != 1 and
+      not any(ConsistencyConfiguration conf).uniqueNodeLocationExclude(n) and
       msg = "Node should have one location but has " + c + "."
     )
   }
@@ -76,7 +90,8 @@ module Consistency {
         strictcount(Node n |
           not exists(string filepath, int startline, int startcolumn, int endline, int endcolumn |
             n.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-          )
+          ) and
+          not any(ConsistencyConfiguration conf).missingLocationExclude(n)
         ) and
       msg = "Nodes without location: " + c
     )
@@ -172,6 +187,7 @@ module Consistency {
 
   query predicate reverseRead(Node n, string msg) {
     exists(Node n2 | readStep(n, _, n2) and hasPost(n2) and not hasPost(n)) and
+    not any(ConsistencyConfiguration conf).reverseReadExclude(n) and
     msg = "Origin of readStep is missing a PostUpdateNode."
   }
 

cpp/ql/lib/semmle/code/cpp/exprs/Lambda.qll

@@ -118,7 +118,7 @@ class LambdaCapture extends Locatable, @lambdacapture {
    * An identifier is captured by reference if:
    *   - It is explicitly captured by reference.
    *   - It is implicitly captured, and the lambda's default capture mode is by-reference.
-   *   - The identifier is "this". [Said behaviour is dictated by the C++11 standard, but it
+   *   - The identifier is "this". [Said behavior is dictated by the C++11 standard, but it
    *                                is actually "*this" being captured rather than "this".]
    */
   predicate isCapturedByReference() { lambda_capture(this, _, _, _, true, _, _) }

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplConsistency.qll

@@ -15,11 +15,23 @@ module Consistency {
   class ConsistencyConfiguration extends TConsistencyConfiguration {
     string toString() { none() }
 
+    /** Holds if `n` should be excluded from the consistency test `uniqueEnclosingCallable`. */
+    predicate uniqueEnclosingCallableExclude(Node n) { none() }
+
+    /** Holds if `n` should be excluded from the consistency test `uniqueNodeLocation`. */
+    predicate uniqueNodeLocationExclude(Node n) { none() }
+
+    /** Holds if `n` should be excluded from the consistency test `missingLocation`. */
+    predicate missingLocationExclude(Node n) { none() }
+
     /** Holds if `n` should be excluded from the consistency test `postWithInFlow`. */
     predicate postWithInFlowExclude(Node n) { none() }
 
     /** Holds if `n` should be excluded from the consistency test `argHasPostUpdate`. */
     predicate argHasPostUpdateExclude(ArgumentNode n) { none() }
+
+    /** Holds if `n` should be excluded from the consistency test `reverseRead`. */
+    predicate reverseReadExclude(Node n) { none() }
   }
 
   private class RelevantNode extends Node {
@@ -46,6 +58,7 @@ module Consistency {
       n instanceof RelevantNode and
       c = count(nodeGetEnclosingCallable(n)) and
       c != 1 and
+      not any(ConsistencyConfiguration conf).uniqueEnclosingCallableExclude(n) and
       msg = "Node should have one enclosing callable but has " + c + "."
     )
   }
@@ -66,6 +79,7 @@ module Consistency {
           n.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
         ) and
       c != 1 and
+      not any(ConsistencyConfiguration conf).uniqueNodeLocationExclude(n) and
       msg = "Node should have one location but has " + c + "."
     )
   }
@@ -76,7 +90,8 @@ module Consistency {
         strictcount(Node n |
           not exists(string filepath, int startline, int startcolumn, int endline, int endcolumn |
             n.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-          )
+          ) and
+          not any(ConsistencyConfiguration conf).missingLocationExclude(n)
         ) and
       msg = "Nodes without location: " + c
     )
@@ -172,6 +187,7 @@ module Consistency {
 
   query predicate reverseRead(Node n, string msg) {
     exists(Node n2 | readStep(n, _, n2) and hasPost(n2) and not hasPost(n)) and
+    not any(ConsistencyConfiguration conf).reverseReadExclude(n) and
     msg = "Origin of readStep is missing a PostUpdateNode."
   }
 

cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysis.qll

@@ -266,6 +266,20 @@ private predicate operandReturned(Operand operand, IntValue bitOffset) {
   bitOffset = Ints::unknown()
 }
 
+pragma[nomagic]
+private predicate initializeParameterInstructionHasVariable(
+  IRVariable var, InitializeParameterInstruction init
+) {
+  init.getIRVariable() = var
+}
+
+private predicate instructionInitializesThisInFunction(
+  Language::Function f, InitializeParameterInstruction init
+) {
+  initializeParameterInstructionHasVariable(any(IRThisVariable var), pragma[only_bind_into](init)) and
+  init.getEnclosingFunction() = f
+}
+
 private predicate isArgumentForParameter(
   CallInstruction ci, Operand operand, InitializeParameterInstruction init
 ) {
@@ -275,8 +289,7 @@ private predicate isArgumentForParameter(
     (
       init.getParameter() = f.getParameter(operand.(PositionalArgumentOperand).getIndex())
       or
-      init.getIRVariable() instanceof IRThisVariable and
-      unique( | | init.getEnclosingFunction()) = f and
+      instructionInitializesThisInFunction(f, init) and
       operand instanceof ThisArgumentOperand
     ) and
     not Language::isFunctionVirtual(f) and

cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasAnalysis.qll

@@ -266,6 +266,20 @@ private predicate operandReturned(Operand operand, IntValue bitOffset) {
   bitOffset = Ints::unknown()
 }
 
+pragma[nomagic]
+private predicate initializeParameterInstructionHasVariable(
+  IRVariable var, InitializeParameterInstruction init
+) {
+  init.getIRVariable() = var
+}
+
+private predicate instructionInitializesThisInFunction(
+  Language::Function f, InitializeParameterInstruction init
+) {
+  initializeParameterInstructionHasVariable(any(IRThisVariable var), pragma[only_bind_into](init)) and
+  init.getEnclosingFunction() = f
+}
+
 private predicate isArgumentForParameter(
   CallInstruction ci, Operand operand, InitializeParameterInstruction init
 ) {
@@ -275,8 +289,7 @@ private predicate isArgumentForParameter(
     (
       init.getParameter() = f.getParameter(operand.(PositionalArgumentOperand).getIndex())
       or
-      init.getIRVariable() instanceof IRThisVariable and
-      unique( | | init.getEnclosingFunction()) = f and
+      instructionInitializesThisInFunction(f, init) and
       operand instanceof ThisArgumentOperand
     ) and
     not Language::isFunctionVirtual(f) and

cpp/ql/lib/semmle/code/cpp/security/BufferWrite.qll

@@ -71,13 +71,30 @@ abstract class BufferWrite extends Expr {
    */
   int getMaxData() { none() }
 
+  /**
+   * Gets an upper bound to the amount of data that's being written (if one
+   * can be found), specifying the reason for the estimation.
+   */
+  int getMaxData(BufferWriteEstimationReason reason) {
+    reason instanceof NoSpecifiedEstimateReason and result = getMaxData()
+  }
+
   /**
    * Gets an upper bound to the amount of data that's being written (if one
    * can be found), except that float to string conversions are assumed to be
    * much smaller (8 bytes) than their true maximum length.  This can be
    * helpful in determining the cause of a buffer overflow issue.
    */
-  int getMaxDataLimited() { result = this.getMaxData() }
+  int getMaxDataLimited() { result = getMaxData() }
+
+  /**
+   * Gets an upper bound to the amount of data that's being written (if one
+   * can be found), specifying the reason for the estimation, except that
+   * float to string conversions are assumed to be much smaller (8 bytes)
+   * than their true maximum length.  This can be helpful in determining the
+   * cause of a buffer overflow issue.
+   */
+  int getMaxDataLimited(BufferWriteEstimationReason reason) { result = getMaxData(reason) }
 
   /**
    * Gets the size of a single character of the type this
@@ -135,10 +152,16 @@ class StrCopyBW extends BufferWriteCall {
     result = this.getArgument(this.getParamSize()).getValue().toInt() * this.getCharSize()
   }
 
-  override int getMaxData() {
+  private int getMaxDataImpl(BufferWriteEstimationReason reason) {
+    // when result exists, it is an exact flow analysis
+    reason instanceof ValueFlowAnalysis and
     result =
       this.getArgument(this.getParamSrc()).(AnalysedString).getMaxLength() * this.getCharSize()
   }
+
+  override int getMaxData(BufferWriteEstimationReason reason) { result = getMaxDataImpl(reason) }
+
+  override int getMaxData() { result = max(getMaxDataImpl(_)) }
 }
 
 /**
@@ -173,10 +196,16 @@ class StrCatBW extends BufferWriteCall {
     result = this.getArgument(this.getParamSize()).getValue().toInt() * this.getCharSize()
   }
 
-  override int getMaxData() {
+  private int getMaxDataImpl(BufferWriteEstimationReason reason) {
+    // when result exists, it is an exact flow analysis
+    reason instanceof ValueFlowAnalysis and
     result =
       this.getArgument(this.getParamSrc()).(AnalysedString).getMaxLength() * this.getCharSize()
   }
+
+  override int getMaxData(BufferWriteEstimationReason reason) { result = getMaxDataImpl(reason) }
+
+  override int getMaxData() { result = max(getMaxDataImpl(_)) }
 }
 
 /**
@@ -233,19 +262,29 @@ class SprintfBW extends BufferWriteCall {
 
   override Expr getDest() { result = this.getArgument(f.getOutputParameterIndex(false)) }
 
-  override int getMaxData() {
+  private int getMaxDataImpl(BufferWriteEstimationReason reason) {
     exists(FormatLiteral fl |
       fl = this.(FormattingFunctionCall).getFormat() and
-      result = fl.getMaxConvertedLength() * this.getCharSize()
+      result = fl.getMaxConvertedLengthWithReason(reason) * this.getCharSize()
     )
   }
 
-  override int getMaxDataLimited() {
+  override int getMaxData(BufferWriteEstimationReason reason) { result = getMaxDataImpl(reason) }
+
+  override int getMaxData() { result = max(getMaxDataImpl(_)) }
+
+  private int getMaxDataLimitedImpl(BufferWriteEstimationReason reason) {
     exists(FormatLiteral fl |
       fl = this.(FormattingFunctionCall).getFormat() and
-      result = fl.getMaxConvertedLengthLimited() * this.getCharSize()
+      result = fl.getMaxConvertedLengthLimitedWithReason(reason) * this.getCharSize()
     )
   }
+
+  override int getMaxDataLimited(BufferWriteEstimationReason reason) {
+    result = getMaxDataLimitedImpl(reason)
+  }
+
+  override int getMaxDataLimited() { result = max(getMaxDataLimitedImpl(_)) }
 }
 
 /**
@@ -336,19 +375,29 @@ class SnprintfBW extends BufferWriteCall {
     result = this.getArgument(this.getParamSize()).getValue().toInt() * this.getCharSize()
   }
 
-  override int getMaxData() {
+  private int getMaxDataImpl(BufferWriteEstimationReason reason) {
     exists(FormatLiteral fl |
       fl = this.(FormattingFunctionCall).getFormat() and
-      result = fl.getMaxConvertedLength() * this.getCharSize()
+      result = fl.getMaxConvertedLengthWithReason(reason) * this.getCharSize()
     )
   }
 
-  override int getMaxDataLimited() {
+  override int getMaxData(BufferWriteEstimationReason reason) { result = getMaxDataImpl(reason) }
+
+  override int getMaxData() { result = max(getMaxDataImpl(_)) }
+
+  private int getMaxDataLimitedImpl(BufferWriteEstimationReason reason) {
     exists(FormatLiteral fl |
       fl = this.(FormattingFunctionCall).getFormat() and
-      result = fl.getMaxConvertedLengthLimited() * this.getCharSize()
+      result = fl.getMaxConvertedLengthLimitedWithReason(reason) * this.getCharSize()
     )
   }
+
+  override int getMaxDataLimited(BufferWriteEstimationReason reason) {
+    result = getMaxDataLimitedImpl(reason)
+  }
+
+  override int getMaxDataLimited() { result = max(getMaxDataLimitedImpl(_)) }
 }
 
 /**
@@ -436,7 +485,9 @@ class ScanfBW extends BufferWrite {
 
   override Expr getDest() { result = this }
 
-  override int getMaxData() {
+  private int getMaxDataImpl(BufferWriteEstimationReason reason) {
+    // when this returns, it is based on exact flow analysis
+    reason instanceof ValueFlowAnalysis and
     exists(ScanfFunctionCall fc, ScanfFormatLiteral fl, int arg |
       this = fc.getArgument(arg) and
       fl = fc.getFormat() and
@@ -444,6 +495,10 @@ class ScanfBW extends BufferWrite {
     )
   }
 
+  override int getMaxData(BufferWriteEstimationReason reason) { result = getMaxDataImpl(reason) }
+
+  override int getMaxData() { result = max(getMaxDataImpl(_)) }
+
   override string getBWDesc() {
     exists(FunctionCall fc |
       this = fc.getArgument(_) and
@@ -474,8 +529,14 @@ class RealpathBW extends BufferWriteCall {
 
   override Expr getASource() { result = this.getArgument(0) }
 
-  override int getMaxData() {
+  private int getMaxDataImpl(BufferWriteEstimationReason reason) {
+    // although there may be some unknown invariants guaranteeing that a real path is shorter than PATH_MAX, we can consider providing less than PATH_MAX a problem with high precision
+    reason instanceof ValueFlowAnalysis and
     result = path_max() and
     this = this // Suppress a compiler warning
   }
+
+  override int getMaxData(BufferWriteEstimationReason reason) { result = getMaxDataImpl(reason) }
+
+  override int getMaxData() { result = max(getMaxDataImpl(_)) }
 }

cpp/ql/src/CHANGELOG.md

@@ -1,3 +1,14 @@
+## 0.0.7
+
+## 0.0.6
+
+## 0.0.5
+
+### New Queries
+
+* A new query `cpp/certificate-not-checked` has been added for C/C++. The query flags unsafe use of OpenSSL and similar libraries.
+* A new query `cpp/certificate-result-conflation` has been added for C/C++. The query flags unsafe use of OpenSSL and similar libraries.
+
 ## 0.0.4
 
 ### New Queries

cpp/ql/src/Likely Bugs/Arithmetic/ComparisonWithCancelingSubExpr.ql

@@ -85,7 +85,8 @@ private predicate cancelingSubExprs(ComparisonOperation cmp, VariableAccess a1,
   exists(Variable v |
     exists(float m | m < 0 and cmpLinearSubVariable(cmp, v, a1, m)) and
     exists(float m | m > 0 and cmpLinearSubVariable(cmp, v, a2, m))
-  )
+  ) and
+  not any(ClassTemplateInstantiation inst).getATemplateArgument() = cmp.getParent*()
 }
 
 from ComparisonOperation cmp, VariableAccess a1, VariableAccess a2

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

@@ -29,7 +29,9 @@ predicate pointlessSelfComparison(ComparisonOperation cmp) {
     not exists(lhs.getQualifier()) and // Avoid structure fields
     not exists(rhs.getQualifier()) and // Avoid structure fields
     not convertedExprMightOverflow(lhs) and
-    not convertedExprMightOverflow(rhs)
+    not convertedExprMightOverflow(rhs) and
+    // Don't warn if the comparison is part of a template argument.
+    not any(ClassTemplateInstantiation inst).getATemplateArgument() = cmp.getParent*()
   )
 }
 

cpp/ql/src/Likely Bugs/Format/SnprintfOverflow.qhelp

@@ -3,7 +3,7 @@
   "qhelp.dtd">
 <qhelp>
 <overview>
-<p>The return value of a call to <code>snprintf</code> is the number of characters that <i>would have</i> been written to the buffer assuming there was sufficient space.  In the event that the operation reaches the end of the buffer and more than one character is discarded, the return value will be greater than the buffer size.  This can cause incorrect behaviour, for example:
+<p>The return value of a call to <code>snprintf</code> is the number of characters that <i>would have</i> been written to the buffer assuming there was sufficient space.  In the event that the operation reaches the end of the buffer and more than one character is discarded, the return value will be greater than the buffer size.  This can cause incorrect behavior, for example:
 </p>
 </overview>
 

cpp/ql/src/Security/CWE/CWE-120/OverrunWrite.ql

@@ -21,14 +21,15 @@ import semmle.code.cpp.commons.Alloc
  * See CWE-120/UnboundedWrite.ql for a summary of CWE-120 alert cases.
  */
 
-from BufferWrite bw, Expr dest, int destSize
+from BufferWrite bw, Expr dest, int destSize, int estimated
 where
   not bw.hasExplicitLimit() and // has no explicit size limit
   dest = bw.getDest() and
   destSize = getBufferSize(dest, _) and
+  estimated = bw.getMaxDataLimited(_) and
   // we can deduce that too much data may be copied (even without
   // long '%f' conversions)
-  bw.getMaxDataLimited() > destSize
+  estimated > destSize
 select bw,
-  "This '" + bw.getBWDesc() + "' operation requires " + bw.getMaxData() +
+  "This '" + bw.getBWDesc() + "' operation requires " + estimated +
     " bytes but the destination is only " + destSize + " bytes."

cpp/ql/src/Security/CWE/CWE-120/OverrunWriteFloat.ql

@@ -21,14 +21,15 @@ import semmle.code.cpp.security.BufferWrite
  * See CWE-120/UnboundedWrite.ql for a summary of CWE-120 alert cases.
  */
 
-from BufferWrite bw, int destSize
+from BufferWrite bw, int destSize, int estimated, BufferWriteEstimationReason reason
 where
   not bw.hasExplicitLimit() and
   // has no explicit size limit
   destSize = getBufferSize(bw.getDest(), _) and
-  bw.getMaxData() > destSize and
+  estimated = bw.getMaxData(reason) and
+  estimated > destSize and
   // and we can deduce that too much data may be copied
-  bw.getMaxDataLimited() <= destSize // but it would fit without long '%f' conversions
+  bw.getMaxDataLimited(reason) <= destSize // but it would fit without long '%f' conversions
 select bw,
-  "This '" + bw.getBWDesc() + "' operation may require " + bw.getMaxData() +
+  "This '" + bw.getBWDesc() + "' operation may require " + estimated +
     " bytes because of float conversions, but the target is only " + destSize + " bytes."

cpp/ql/src/Security/CWE/CWE-120/UnboundedWrite.ql

@@ -44,7 +44,7 @@ import TaintedWithPath
 
 predicate isUnboundedWrite(BufferWrite bw) {
   not bw.hasExplicitLimit() and // has no explicit size limit
-  not exists(bw.getMaxData()) // and we can't deduce an upper bound to the amount copied
+  not exists(bw.getMaxData(_)) // and we can't deduce an upper bound to the amount copied
 }
 
 /*

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

@@ -22,11 +22,13 @@ import semmle.code.cpp.dataflow.DataFlow
  *  Holds if `sub` is guarded by a condition which ensures that
  * `left >= right`.
  */
-pragma[noinline]
+pragma[nomagic]
 predicate isGuarded(SubExpr sub, Expr left, Expr right) {
-  exists(GuardCondition guard, int k |
-    guard.controls(sub.getBasicBlock(), _) and
-    guard.ensuresLt(left, right, k, sub.getBasicBlock(), false) and
+  exprIsSubLeftOrLess(pragma[only_bind_into](sub), _) and // Manual magic
+  exists(GuardCondition guard, int k, BasicBlock bb |
+    pragma[only_bind_into](bb) = sub.getBasicBlock() and
+    guard.controls(pragma[only_bind_into](bb), _) and
+    guard.ensuresLt(left, right, k, bb, false) and
     k >= 0
   )
 }
@@ -36,47 +38,56 @@ predicate isGuarded(SubExpr sub, Expr left, Expr right) {
  * `sub.getLeftOperand()`.
  */
 predicate exprIsSubLeftOrLess(SubExpr sub, DataFlow::Node n) {
-  n.asExpr() = sub.getLeftOperand()
-  or
-  exists(DataFlow::Node other |
-    // dataflow
-    exprIsSubLeftOrLess(sub, other) and
-    (
-      DataFlow::localFlowStep(n, other) or
-      DataFlow::localFlowStep(other, n)
+  interestingSubExpr(sub, _) and // Manual magic
+  (
+    n.asExpr() = sub.getLeftOperand()
+    or
+    exists(DataFlow::Node other |
+      // dataflow
+      exprIsSubLeftOrLess(sub, other) and
+      (
+        DataFlow::localFlowStep(n, other) or
+        DataFlow::localFlowStep(other, n)
+      )
+    )
+    or
+    exists(DataFlow::Node other |
+      // guard constraining `sub`
+      exprIsSubLeftOrLess(sub, other) and
+      isGuarded(sub, other.asExpr(), n.asExpr()) // other >= n
+    )
+    or
+    exists(DataFlow::Node other, float p, float q |
+      // linear access of `other`
+      exprIsSubLeftOrLess(sub, other) and
+      linearAccess(n.asExpr(), other.asExpr(), p, q) and // n = p * other + q
+      p <= 1 and
+      q <= 0
+    )
+    or
+    exists(DataFlow::Node other, float p, float q |
+      // linear access of `n`
+      exprIsSubLeftOrLess(sub, other) and
+      linearAccess(other.asExpr(), n.asExpr(), p, q) and // other = p * n + q
+      p >= 1 and
+      q >= 0
     )
-  )
-  or
-  exists(DataFlow::Node other |
-    // guard constraining `sub`
-    exprIsSubLeftOrLess(sub, other) and
-    isGuarded(sub, other.asExpr(), n.asExpr()) // other >= n
-  )
-  or
-  exists(DataFlow::Node other, float p, float q |
-    // linear access of `other`
-    exprIsSubLeftOrLess(sub, other) and
-    linearAccess(n.asExpr(), other.asExpr(), p, q) and // n = p * other + q
-    p <= 1 and
-    q <= 0
-  )
-  or
-  exists(DataFlow::Node other, float p, float q |
-    // linear access of `n`
-    exprIsSubLeftOrLess(sub, other) and
-    linearAccess(other.asExpr(), n.asExpr(), p, q) and // other = p * n + q
-    p >= 1 and
-    q >= 0
   )
 }
 
-from RelationalOperation ro, SubExpr sub
-where
-  not isFromMacroDefinition(ro) and
+predicate interestingSubExpr(SubExpr sub, RelationalOperation ro) {
   not isFromMacroDefinition(sub) and
   ro.getLesserOperand().getValue().toInt() = 0 and
   ro.getGreaterOperand() = sub and
   sub.getFullyConverted().getUnspecifiedType().(IntegralType).isUnsigned() and
-  exprMightOverflowNegatively(sub.getFullyConverted()) and // generally catches false positives involving constants
-  not exprIsSubLeftOrLess(sub, DataFlow::exprNode(sub.getRightOperand())) // generally catches false positives where there's a relation between the left and right operands
+  // generally catches false positives involving constants
+  exprMightOverflowNegatively(sub.getFullyConverted())
+}
+
+from RelationalOperation ro, SubExpr sub
+where
+  interestingSubExpr(sub, ro) and
+  not isFromMacroDefinition(ro) and
+  // generally catches false positives where there's a relation between the left and right operands
+  not exprIsSubLeftOrLess(sub, DataFlow::exprNode(sub.getRightOperand()))
 select ro, "Unsigned subtraction can never be negative."

cpp/ql/src/Security/CWE/CWE-311/CleartextTransmission.ql

@@ -14,105 +14,188 @@
 import cpp
 import semmle.code.cpp.security.SensitiveExprs
 import semmle.code.cpp.dataflow.TaintTracking
+import semmle.code.cpp.valuenumbering.GlobalValueNumbering
 import semmle.code.cpp.models.interfaces.FlowSource
 import DataFlow::PathGraph
 
 /**
- * A function call that sends or receives data over a network.
+ * A DataFlow node corresponding to a variable or function call that
+ * might contain or return a password or other sensitive information.
  */
-abstract class NetworkSendRecv extends FunctionCall {
+class SensitiveNode extends DataFlow::Node {
+  SensitiveNode() {
+    this.asExpr() = any(SensitiveVariable sv).getInitializer().getExpr() or
+    this.asExpr().(VariableAccess).getTarget() =
+      any(SensitiveVariable sv).(GlobalOrNamespaceVariable) or
+    this.asUninitialized() instanceof SensitiveVariable or
+    this.asParameter() instanceof SensitiveVariable or
+    this.asExpr().(FunctionCall).getTarget() instanceof SensitiveFunction
+  }
+}
+
+/**
+ * A function that sends or receives data over a network.
+ */
+abstract class SendRecv extends Function {
   /**
    * Gets the expression for the socket or similar object used for sending or
-   * receiving data (if any).
+   * receiving data through the function call `call` (if any).
    */
-  abstract Expr getSocketExpr();
+  abstract Expr getSocketExpr(Call call);
 
   /**
-   * Gets the expression for the buffer to be sent from / received into.
+   * Gets the expression for the buffer to be sent from / received into through
+   * the function call `call`.
    */
-  abstract Expr getDataExpr();
+  abstract Expr getDataExpr(Call call);
+}
+
+/**
+ * A function that sends data over a network.
+ */
+class Send extends SendRecv instanceof RemoteFlowSinkFunction {
+  override Expr getSocketExpr(Call call) {
+    call.getTarget() = this and
+    exists(FunctionInput input, int arg |
+      super.hasSocketInput(input) and
+      input.isParameter(arg) and
+      result = call.getArgument(arg)
+    )
+  }
+
+  override Expr getDataExpr(Call call) {
+    call.getTarget() = this and
+    exists(FunctionInput input, int arg |
+      super.hasRemoteFlowSink(input, _) and
+      input.isParameterDeref(arg) and
+      result = call.getArgument(arg)
+    )
+  }
+}
+
+/**
+ * A function that receives data over a network.
+ */
+class Recv extends SendRecv instanceof RemoteFlowSourceFunction {
+  override Expr getSocketExpr(Call call) {
+    call.getTarget() = this and
+    exists(FunctionInput input, int arg |
+      super.hasSocketInput(input) and
+      input.isParameter(arg) and
+      result = call.getArgument(arg)
+    )
+  }
+
+  override Expr getDataExpr(Call call) {
+    call.getTarget() = this and
+    exists(FunctionOutput output, int arg |
+      super.hasRemoteFlowSource(output, _) and
+      output.isParameterDeref(arg) and
+      result = call.getArgument(arg)
+    )
+  }
+}
+
+/**
+ * A function call that sends or receives data over a network.
+ *
+ * note: function calls such as `write` may be writing to a network source
+ * or a file. We could attempt to determine which, and sort results into
+ * `cpp/cleartext-transmission` and perhaps `cpp/cleartext-storage-file`. In
+ * practice it usually isn't very important which query reports a result as
+ * long as its reported exactly once.
+ *
+ * We do exclude function calls that specify a constant socket, which is
+ * likely to mean standard input, standard output or a similar channel.
+ */
+abstract class NetworkSendRecv extends FunctionCall {
+  SendRecv target;
+
+  NetworkSendRecv() {
+    this.getTarget() = target and
+    // exclude calls based on the socket...
+    not exists(GVN g |
+      g = globalValueNumber(target.getSocketExpr(this)) and
+      (
+        // literal constant
+        globalValueNumber(any(Literal l)) = g
+        or
+        // variable (such as a global) initialized to a literal constant
+        exists(Variable v |
+          v.getInitializer().getExpr() instanceof Literal and
+          g = globalValueNumber(v.getAnAccess())
+        )
+      )
+    )
+  }
+
+  final Expr getDataExpr() { result = target.getDataExpr(this) }
 }
 
 /**
  * A function call that sends data over a network.
- *
- * note: functions such as `write` may be writing to a network source or a file. We could attempt to determine which, and sort results into `cpp/cleartext-transmission` and perhaps `cpp/cleartext-storage-file`. In practice it usually isn't very important which query reports a result as long as its reported exactly once.
  */
 class NetworkSend extends NetworkSendRecv {
-  RemoteFlowSinkFunction target;
-
-  NetworkSend() { target = this.getTarget() }
-
-  override Expr getSocketExpr() {
-    exists(FunctionInput input, int arg |
-      target.hasSocketInput(input) and
-      input.isParameter(arg) and
-      result = this.getArgument(arg)
-    )
-  }
-
-  override Expr getDataExpr() {
-    exists(FunctionInput input, int arg |
-      target.hasRemoteFlowSink(input, _) and
-      input.isParameterDeref(arg) and
-      result = this.getArgument(arg)
-    )
-  }
+  override Send target;
 }
 
 /**
  * A function call that receives data over a network.
  */
 class NetworkRecv extends NetworkSendRecv {
-  RemoteFlowSourceFunction target;
-
-  NetworkRecv() { target = this.getTarget() }
-
-  override Expr getSocketExpr() {
-    exists(FunctionInput input, int arg |
-      target.hasSocketInput(input) and
-      input.isParameter(arg) and
-      result = this.getArgument(arg)
-    )
-  }
-
-  override Expr getDataExpr() {
-    exists(FunctionOutput output, int arg |
-      target.hasRemoteFlowSource(output, _) and
-      output.isParameterDeref(arg) and
-      result = this.getArgument(arg)
-    )
-  }
+  override Recv target;
 }
 
 /**
- * Taint flow from a sensitive expression to a network operation with data
- * tainted by that expression.
+ * An expression that is an argument or return value from an encryption or
+ * decryption call.
  */
-class SensitiveSendRecvConfiguration extends TaintTracking::Configuration {
-  SensitiveSendRecvConfiguration() { this = "SensitiveSendRecvConfiguration" }
-
-  override predicate isSource(DataFlow::Node source) { source.asExpr() instanceof SensitiveExpr }
-
-  override predicate isSink(DataFlow::Node sink) {
-    exists(NetworkSendRecv transmission |
-      sink.asExpr() = transmission.getDataExpr() and
-      // a zero socket descriptor is standard input, which is not interesting for this query.
-      not exists(Zero zero |
-        DataFlow::localFlow(DataFlow::exprNode(zero),
-          DataFlow::exprNode(transmission.getSocketExpr()))
+class Encrypted extends Expr {
+  Encrypted() {
+    exists(FunctionCall fc |
+      fc.getTarget().getName().toLowerCase().regexpMatch(".*(crypt|encode|decode).*") and
+      (
+        this = fc or
+        this = fc.getAnArgument()
       )
     )
   }
 }
 
+/**
+ * Taint flow from a sensitive expression.
+ */
+class FromSensitiveConfiguration extends TaintTracking::Configuration {
+  FromSensitiveConfiguration() { this = "FromSensitiveConfiguration" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof SensitiveNode }
+
+  override predicate isSink(DataFlow::Node sink) {
+    sink.asExpr() = any(NetworkSendRecv nsr).getDataExpr()
+    or
+    sink.asExpr() instanceof Encrypted
+  }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
+    // flow through encryption functions to the return value (in case we can reach other sinks)
+    node2.asExpr().(Encrypted).(FunctionCall).getAnArgument() = node1.asExpr()
+  }
+}
+
 from
-  SensitiveSendRecvConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink,
-  NetworkSendRecv transmission, string msg
+  FromSensitiveConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink,
+  NetworkSendRecv networkSendRecv, string msg
 where
+  // flow from sensitive -> network data
   config.hasFlowPath(source, sink) and
-  sink.getNode().asExpr() = transmission.getDataExpr() and
-  if transmission instanceof NetworkSend
+  sink.getNode().asExpr() = networkSendRecv.getDataExpr() and
+  // no flow from sensitive -> evidence of encryption
+  not exists(DataFlow::Node encrypted |
+    config.hasFlow(source.getNode(), encrypted) and
+    encrypted.asExpr() instanceof Encrypted
+  ) and
+  // construct result
+  if networkSendRecv instanceof NetworkSend
   then
     msg =
       "This operation transmits '" + sink.toString() +
@@ -121,4 +204,4 @@ where
     msg =
       "This operation receives into '" + sink.toString() +
         "', which may put unencrypted sensitive data into $@"
-select transmission, source, sink, msg, source, source.getNode().asExpr().toString()
+select networkSendRecv, source, sink, msg, source, source.getNode().toString()

cpp/ql/src/Security/CWE/CWE-319/UseOfHttp.ql

@@ -28,6 +28,11 @@ class PrivateHostName extends string {
   }
 }
 
+pragma[nomagic]
+predicate privateHostNameFlowsToExpr(Expr e) {
+  TaintTracking::localExprTaint(any(StringLiteral p | p.getValue() instanceof PrivateHostName), e)
+}
+
 /**
  * A string containing an HTTP URL not in a private domain.
  */
@@ -38,11 +43,9 @@ class HttpStringLiteral extends StringLiteral {
       or
       exists(string tail |
         tail = s.regexpCapture("http://(.*)", 1) and not tail instanceof PrivateHostName
-      ) and
-      not TaintTracking::localExprTaint(any(StringLiteral p |
-          p.getValue() instanceof PrivateHostName
-        ), this.getParent*())
-    )
+      )
+    ) and
+    not privateHostNameFlowsToExpr(this.getParent*())
   }
 }
 

cpp/ql/src/change-notes/released/0.0.5.md

@@ -0,0 +1,6 @@
+## 0.0.5
+
+### New Queries
+
+* A new query `cpp/certificate-not-checked` has been added for C/C++. The query flags unsafe use of OpenSSL and similar libraries.
+* A new query `cpp/certificate-result-conflation` has been added for C/C++. The query flags unsafe use of OpenSSL and similar libraries.

cpp/ql/src/change-notes/released/0.0.6.md

@@ -0,0 +1 @@
+## 0.0.6

cpp/ql/src/change-notes/released/0.0.7.md

@@ -0,0 +1 @@
+## 0.0.7

cpp/ql/src/codeql-pack.release.yml

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.0.4
+lastReleaseVersion: 0.0.7

cpp/ql/src/experimental/Security/CWE/CWE-266/IncorrectPrivilegeAssignment.cpp

@@ -0,0 +1,16 @@
+...
+  umask(0); // BAD
+...
+  maskOut = S_IRWXG | S_IRWXO;
+  umask(maskOut); // GOOD
+  ...
+  fchmod(fileno(fp), 0555 - maskOut); // BAD 
+  ...
+  fchmod(fileno(fp), 0555 & ~maskOut); // GOOD
+...
+  umask(0666);
+  chmod(pathname, 0666); // BAD
+...
+  umask(0022);
+  chmod(pathname, 0666); // GOOD
+...

cpp/ql/src/experimental/Security/CWE/CWE-266/IncorrectPrivilegeAssignment.qhelp

@@ -0,0 +1,23 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Finding  for function calls that set file permissions that may have errors in use. Incorrect arithmetic for calculating the resolution mask, using the same mask in opposite functions, using a mask that is too wide.</p>
+
+</overview>
+
+<example>
+<p>The following example demonstrates erroneous and fixed ways to use functions.</p>
+<sample src="IncorrectPrivilegeAssignment.cpp" />
+
+</example>
+<references>
+
+<li>
+  CERT C Coding Standard:
+  <a href="https://wiki.sei.cmu.edu/confluence/display/c/FIO06-C.+Create+files+with+appropriate+access+permissions">FIO06-C. Create files with appropriate access permissions</a>.
+</li>
+
+</references>
+</qhelp>

cpp/ql/src/experimental/Security/CWE/CWE-266/IncorrectPrivilegeAssignment.ql

@@ -0,0 +1,87 @@
+/**
+ * @name Find the wrong use of the umask function.
+ * @description Incorrectly evaluated argument to the umask function may have security implications.
+ * @kind problem
+ * @id cpp/wrong-use-of-the-umask
+ * @problem.severity warning
+ * @precision medium
+ * @tags correctness
+ *       maintainability
+ *       security
+ *       external/cwe/cwe-266
+ *       external/cwe/cwe-264
+ *       external/cwe/cwe-200
+ *       external/cwe/cwe-560
+ *       external/cwe/cwe-687
+ */
+
+import cpp
+import semmle.code.cpp.exprs.BitwiseOperation
+import semmle.code.cpp.valuenumbering.GlobalValueNumbering
+
+/**
+ * An expression that is either a `BinaryArithmeticOperation` or the result of one or more `BinaryBitwiseOperation`s on a `BinaryArithmeticOperation`. For example `1 | (2 + 3)`.
+ */
+class ContainsArithmetic extends Expr {
+  ContainsArithmetic() {
+    this instanceof BinaryArithmeticOperation
+    or
+    // recursive search into `Operation`s
+    this.(BinaryBitwiseOperation).getAnOperand() instanceof ContainsArithmetic
+  }
+}
+
+/** Holds for a function `f` that has an argument at index `apos` used to set file permissions. */
+predicate numberArgumentModFunctions(Function f, int apos) {
+  f.hasGlobalOrStdName("umask") and apos = 0
+  or
+  f.hasGlobalOrStdName("fchmod") and apos = 1
+  or
+  f.hasGlobalOrStdName("chmod") and apos = 1
+}
+
+from FunctionCall fc, string msg, FunctionCall fcsnd
+where
+  fc.getTarget().hasGlobalOrStdName("umask") and
+  fc.getArgument(0).getValue() = "0" and
+  not exists(FunctionCall fctmp |
+    fctmp.getTarget().hasGlobalOrStdName("umask") and
+    not fctmp.getArgument(0).getValue() = "0"
+  ) and
+  exists(FunctionCall fctmp |
+    (
+      fctmp.getTarget().hasGlobalOrStdName("fopen") or
+      fctmp.getTarget().hasGlobalOrStdName("open")
+    ) and
+    not fctmp.getArgument(1).getValue().matches("r%") and
+    fctmp.getNumberOfArguments() = 2 and
+    not fctmp.getArgument(0).getValue() = "/dev/null" and
+    fcsnd = fctmp
+  ) and
+  not exists(FunctionCall fctmp |
+    fctmp.getTarget().hasGlobalOrStdName("chmod") or
+    fctmp.getTarget().hasGlobalOrStdName("fchmod")
+  ) and
+  msg = "Using umask(0) may not be safe with call $@."
+  or
+  fc.getTarget().hasGlobalOrStdName("umask") and
+  exists(FunctionCall fctmp |
+    (
+      fctmp.getTarget().hasGlobalOrStdName("chmod") or
+      fctmp.getTarget().hasGlobalOrStdName("fchmod")
+    ) and
+    (
+      globalValueNumber(fc.getArgument(0)) = globalValueNumber(fctmp.getArgument(1)) and
+      fc.getArgument(0).getValue() != "0"
+    ) and
+    msg = "Not use equal argument in umask and $@ functions." and
+    fcsnd = fctmp
+  )
+  or
+  exists(ContainsArithmetic exptmp, int i |
+    numberArgumentModFunctions(fc.getTarget(), i) and
+    globalValueNumber(exptmp) = globalValueNumber(fc.getArgument(i)) and
+    msg = "Using arithmetic to compute the mask in $@ may not be safe." and
+    fcsnd = fc
+  )
+select fc, msg, fcsnd, fcsnd.getTarget().getName()

cpp/ql/src/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/cpp-queries
-version: 0.0.5-dev
+version: 0.0.7
 groups: cpp
 dependencies:
     codeql/cpp-all: "*"

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-266/semmle/tests/IncorrectPrivilegeAssignment.expected

@@ -0,0 +1,2 @@
+| test.cpp:9:3:9:7 | call to umask | Not use equal argument in umask and $@ functions. | test.cpp:13:3:13:7 | call to chmod | chmod |
+| test.cpp:30:3:30:7 | call to chmod | Using arithmetic to compute the mask in $@ may not be safe. | test.cpp:30:3:30:7 | call to chmod | chmod |