diff --git a/actions/ql/src/Security/CWE-275/MissingActionsPermissions.ql b/actions/ql/src/Security/CWE-275/MissingActionsPermissions.ql index a8bd8a5f93d..00f601fd5da 100644 --- a/actions/ql/src/Security/CWE-275/MissingActionsPermissions.ql +++ b/actions/ql/src/Security/CWE-275/MissingActionsPermissions.ql @@ -26,10 +26,23 @@ string permissionsForJob(Job job) { "{" + concat(string permission | permission = jobNeedsPermission(job) | permission, ", ") + "}" } +predicate jobHasPermissions(Job job) { + exists(job.getPermissions()) + or + exists(job.getEnclosingWorkflow().getPermissions()) + or + // The workflow is reusable and cannot be triggered in any other way; check callers + exists(ReusableWorkflow r | r = job.getEnclosingWorkflow() | + not exists(Event e | e = r.getOn().getAnEvent() | e.getName() != "workflow_call") and + forall(Job caller | caller = job.getEnclosingWorkflow().(ReusableWorkflow).getACaller() | + jobHasPermissions(caller) + ) + ) +} + from Job job, string permissions where - not exists(job.getPermissions()) and - not exists(job.getEnclosingWorkflow().getPermissions()) and + not jobHasPermissions(job) and // exists a trigger event that is not a workflow_call exists(Event e | e = job.getATriggerEvent() and diff --git a/actions/ql/src/change-notes/2026-04-02-permissions.md b/actions/ql/src/change-notes/2026-04-02-permissions.md new file mode 100644 index 00000000000..2672a30ef87 --- /dev/null +++ b/actions/ql/src/change-notes/2026-04-02-permissions.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The query `actions/missing-workflow-permissions` no longer produces false positive results on reusable workflows where all callers set permissions. \ No newline at end of file diff --git a/actions/ql/test/query-tests/Security/CWE-275/.github/workflows/perms11.yml b/actions/ql/test/query-tests/Security/CWE-275/.github/workflows/perms11.yml new file mode 100644 index 00000000000..717cdabc302 --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-275/.github/workflows/perms11.yml @@ -0,0 +1,9 @@ +on: + workflow_call: + +jobs: + build: + name: Build and test + runs-on: ubuntu-latest + steps: + - uses: actions/deploy-pages diff --git a/actions/ql/test/query-tests/Security/CWE-275/.github/workflows/perms12.yml b/actions/ql/test/query-tests/Security/CWE-275/.github/workflows/perms12.yml new file mode 100644 index 00000000000..25ac1f53248 --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-275/.github/workflows/perms12.yml @@ -0,0 +1,11 @@ +on: + workflow_dispatch: + +permissions: + contents: read + id-token: write + pages: write + +jobs: + call-workflow: + uses: ./.github/workflows/perms11.yml diff --git a/cpp/ql/integration-tests/query-suite/cpp-code-scanning.qls.expected b/cpp/ql/integration-tests/query-suite/cpp-code-scanning.qls.expected index 4e86c27d53f..d4b80599950 100644 --- a/cpp/ql/integration-tests/query-suite/cpp-code-scanning.qls.expected +++ b/cpp/ql/integration-tests/query-suite/cpp-code-scanning.qls.expected @@ -7,10 +7,12 @@ ql/cpp/ql/src/Diagnostics/ExtractedFiles.ql ql/cpp/ql/src/Diagnostics/ExtractionWarnings.ql ql/cpp/ql/src/Diagnostics/FailedExtractorInvocations.ql ql/cpp/ql/src/Likely Bugs/Arithmetic/BadAdditionOverflowCheck.ql +ql/cpp/ql/src/Likely Bugs/Arithmetic/IntMultToLong.ql ql/cpp/ql/src/Likely Bugs/Arithmetic/SignedOverflowCheck.ql ql/cpp/ql/src/Likely Bugs/Conversion/CastArrayPointerArithmetic.ql ql/cpp/ql/src/Likely Bugs/Format/SnprintfOverflow.ql ql/cpp/ql/src/Likely Bugs/Format/WrongNumberOfFormatArguments.ql +ql/cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql ql/cpp/ql/src/Likely Bugs/Memory Management/AllocaInLoop.ql ql/cpp/ql/src/Likely Bugs/Memory Management/PointerOverflow.ql ql/cpp/ql/src/Likely Bugs/Memory Management/ReturnStackAllocatedMemory.ql @@ -28,6 +30,7 @@ ql/cpp/ql/src/Security/CWE/CWE-120/VeryLikelyOverrunWrite.ql ql/cpp/ql/src/Security/CWE/CWE-131/NoSpaceForZeroTerminator.ql ql/cpp/ql/src/Security/CWE/CWE-134/UncontrolledFormatString.ql ql/cpp/ql/src/Security/CWE/CWE-190/ArithmeticUncontrolled.ql +ql/cpp/ql/src/Security/CWE/CWE-190/ComparisonWithWiderType.ql ql/cpp/ql/src/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero.ql ql/cpp/ql/src/Security/CWE/CWE-253/HResultBooleanConversion.ql ql/cpp/ql/src/Security/CWE/CWE-311/CleartextFileWrite.ql diff --git a/cpp/ql/lib/semmle/code/cpp/commons/Printf.qll b/cpp/ql/lib/semmle/code/cpp/commons/Printf.qll index d189dd36f87..624465761c2 100644 --- a/cpp/ql/lib/semmle/code/cpp/commons/Printf.qll +++ b/cpp/ql/lib/semmle/code/cpp/commons/Printf.qll @@ -459,6 +459,13 @@ class FormatLiteral extends Literal instanceof StringLiteral { */ int getConvSpecOffset(int n) { result = this.getFormat().indexOf("%", n, 0) } + /** + * Gets the nth conversion specifier string. + */ + private string getConvSpecString(int n) { + n >= 0 and result = "%" + this.getFormat().splitAt("%", n + 1) + } + /* * Each of these predicates gets a regular expressions to match each individual * parts of a conversion specifier. @@ -524,22 +531,20 @@ class FormatLiteral extends Literal instanceof StringLiteral { int n, string spec, string params, string flags, string width, string prec, string len, string conv ) { - exists(int offset, string fmt, string rst, string regexp | - offset = this.getConvSpecOffset(n) and - fmt = this.getFormat() and - rst = fmt.substring(offset, fmt.length()) and + exists(string convSpec, string regexp | + convSpec = this.getConvSpecString(n) and regexp = this.getConvSpecRegexp() and ( - spec = rst.regexpCapture(regexp, 1) and - params = rst.regexpCapture(regexp, 2) and - flags = rst.regexpCapture(regexp, 3) and - width = rst.regexpCapture(regexp, 4) and - prec = rst.regexpCapture(regexp, 5) and - len = rst.regexpCapture(regexp, 6) and - conv = rst.regexpCapture(regexp, 7) + spec = convSpec.regexpCapture(regexp, 1) and + params = convSpec.regexpCapture(regexp, 2) and + flags = convSpec.regexpCapture(regexp, 3) and + width = convSpec.regexpCapture(regexp, 4) and + prec = convSpec.regexpCapture(regexp, 5) and + len = convSpec.regexpCapture(regexp, 6) and + conv = convSpec.regexpCapture(regexp, 7) or - spec = rst.regexpCapture(regexp, 1) and - not exists(rst.regexpCapture(regexp, 2)) and + spec = convSpec.regexpCapture(regexp, 1) and + not exists(convSpec.regexpCapture(regexp, 2)) and params = "" and flags = "" and width = "" and @@ -554,12 +559,10 @@ class FormatLiteral extends Literal instanceof StringLiteral { * Gets the nth conversion specifier (including the initial `%`). */ string getConvSpec(int n) { - exists(int offset, string fmt, string rst, string regexp | - offset = this.getConvSpecOffset(n) and - fmt = this.getFormat() and - rst = fmt.substring(offset, fmt.length()) and + exists(string convSpec, string regexp | + convSpec = this.getConvSpecString(n) and regexp = this.getConvSpecRegexp() and - result = rst.regexpCapture(regexp, 1) + result = convSpec.regexpCapture(regexp, 1) ) } diff --git a/cpp/ql/lib/semmle/code/cpp/commons/Scanf.qll b/cpp/ql/lib/semmle/code/cpp/commons/Scanf.qll index f032ba4749e..98280a522cf 100644 --- a/cpp/ql/lib/semmle/code/cpp/commons/Scanf.qll +++ b/cpp/ql/lib/semmle/code/cpp/commons/Scanf.qll @@ -194,6 +194,13 @@ class ScanfFormatLiteral extends Expr { ) } + /** + * Gets the nth conversion specifier string. + */ + private string getConvSpecString(int n) { + n >= 0 and result = "%" + this.getFormat().splitAt("%", n + 1) + } + /** * Gets the regular expression to match each individual part of a conversion specifier. */ @@ -227,16 +234,14 @@ class ScanfFormatLiteral extends Expr { * specifier. */ predicate parseConvSpec(int n, string spec, string width, string len, string conv) { - exists(int offset, string fmt, string rst, string regexp | - offset = this.getConvSpecOffset(n) and - fmt = this.getFormat() and - rst = fmt.substring(offset, fmt.length()) and + exists(string convSpec, string regexp | + convSpec = this.getConvSpecString(n) and regexp = this.getConvSpecRegexp() and ( - spec = rst.regexpCapture(regexp, 1) and - width = rst.regexpCapture(regexp, 2) and - len = rst.regexpCapture(regexp, 3) and - conv = rst.regexpCapture(regexp, 4) + spec = convSpec.regexpCapture(regexp, 1) and + width = convSpec.regexpCapture(regexp, 2) and + len = convSpec.regexpCapture(regexp, 3) and + conv = convSpec.regexpCapture(regexp, 4) ) ) } diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll index 8b71f140b01..fb2108c2ac5 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll @@ -6,11 +6,15 @@ * * The extensible relations have the following columns: * - Sources: - * `namespace; type; subtypes; name; signature; ext; output; kind` + * `namespace; type; subtypes; name; signature; ext; output; kind; provenance` * - Sinks: - * `namespace; type; subtypes; name; signature; ext; input; kind` + * `namespace; type; subtypes; name; signature; ext; input; kind; provenance` * - Summaries: - * `namespace; type; subtypes; name; signature; ext; input; output; kind` + * `namespace; type; subtypes; name; signature; ext; input; output; kind; provenance` + * - Barriers: + * `namespace; type; subtypes; name; signature; ext; output; kind; provenance` + * - BarrierGuards: + * `namespace; type; subtypes; name; signature; ext; input; acceptingValue; kind; provenance` * * The interpretation of a row is similar to API-graphs with a left-to-right * reading. @@ -87,11 +91,23 @@ * value, and * - flow from the _second_ indirection of the 0th argument to the first * indirection of the return value, etc. - * 8. The `kind` column is a tag that can be referenced from QL to determine to + * 8. The `acceptingValue` column of barrier guard models specifies the condition + * under which the guard blocks flow. It can be one of "true" or "false". In + * the future "no-exception", "not-zero", "null", "not-null" may be supported. + * 9. The `kind` column is a tag that can be referenced from QL to determine to * which classes the interpreted elements should be added. For example, for * sources "remote" indicates a default remote flow source, and for summaries * "taint" indicates a default additional taint step and "value" indicates a * globally applicable value-preserving step. + * 10. The `provenance` column is a tag to indicate the origin and verification of a model. + * The format is {origin}-{verification} or just "manual" where the origin describes + * the origin of the model and verification describes how the model has been verified. + * Some examples are: + * - "df-generated": The model has been generated by the model generator tool. + * - "df-manual": The model has been generated by the model generator and verified by a human. + * - "manual": The model has been written by hand. + * This information is used in a heuristic for dataflow analysis to determine, if a + * model or source code should be used for determining flow. */ import cpp @@ -931,13 +947,13 @@ private module Cached { private predicate barrierGuardChecks(IRGuardCondition g, Expr e, boolean gv, TKindModelPair kmp) { exists( - SourceSinkInterpretationInput::InterpretNode n, Public::AcceptingValue acceptingvalue, + SourceSinkInterpretationInput::InterpretNode n, Public::AcceptingValue acceptingValue, string kind, string model | - isBarrierGuardNode(n, acceptingvalue, kind, model) and + isBarrierGuardNode(n, acceptingValue, kind, model) and n.asNode().asExpr() = e and kmp = TMkPair(kind, model) and - gv = convertAcceptingValue(acceptingvalue).asBooleanValue() and + gv = convertAcceptingValue(acceptingValue).asBooleanValue() and n.asNode().(Private::ArgumentNode).getCall().asCallInstruction() = g ) } @@ -954,14 +970,14 @@ private module Cached { ) { exists( SourceSinkInterpretationInput::InterpretNode interpretNode, - Public::AcceptingValue acceptingvalue, string kind, string model, int indirectionIndex, + Public::AcceptingValue acceptingValue, string kind, string model, int indirectionIndex, Private::ArgumentNode arg | - isBarrierGuardNode(interpretNode, acceptingvalue, kind, model) and + isBarrierGuardNode(interpretNode, acceptingValue, kind, model) and arg = interpretNode.asNode() and arg.asIndirectExpr(indirectionIndex) = e and kmp = MkKindModelPairIntPair(TMkPair(kind, model), indirectionIndex) and - gv = convertAcceptingValue(acceptingvalue).asBooleanValue() and + gv = convertAcceptingValue(acceptingValue).asBooleanValue() and arg.getCall().asCallInstruction() = g ) } diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/ExternalFlowExtensions.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/ExternalFlowExtensions.qll index 1a572c221d9..22c74c2aa71 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/ExternalFlowExtensions.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/ExternalFlowExtensions.qll @@ -33,7 +33,7 @@ extensible predicate barrierModel( */ extensible predicate barrierGuardModel( string namespace, string type, boolean subtypes, string name, string signature, string ext, - string input, string acceptingvalue, string kind, string provenance, QlBuiltins::ExtensionId madId + string input, string acceptingValue, string kind, string provenance, QlBuiltins::ExtensionId madId ); /** diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/FlowSummaryImpl.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/FlowSummaryImpl.qll index cce1b80e7fc..d91dc41febe 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/FlowSummaryImpl.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/FlowSummaryImpl.qll @@ -162,13 +162,13 @@ module SourceSinkInterpretationInput implements } predicate barrierGuardElement( - Element e, string input, Public::AcceptingValue acceptingvalue, string kind, + Element e, string input, Public::AcceptingValue acceptingValue, string kind, Public::Provenance provenance, string model ) { exists( string package, string type, boolean subtypes, string name, string signature, string ext | - barrierGuardModel(package, type, subtypes, name, signature, ext, input, acceptingvalue, kind, + barrierGuardModel(package, type, subtypes, name, signature, ext, input, acceptingValue, kind, provenance, model) and e = interpretElement(package, type, subtypes, name, signature, ext) ) diff --git a/cpp/ql/src/Likely Bugs/Arithmetic/IntMultToLong.ql b/cpp/ql/src/Likely Bugs/Arithmetic/IntMultToLong.ql index 6747d177c80..b05bd637dc2 100644 --- a/cpp/ql/src/Likely Bugs/Arithmetic/IntMultToLong.ql +++ b/cpp/ql/src/Likely Bugs/Arithmetic/IntMultToLong.ql @@ -5,7 +5,7 @@ * @kind problem * @problem.severity warning * @security-severity 8.1 - * @precision medium + * @precision high * @id cpp/integer-multiplication-cast-to-long * @tags reliability * security diff --git a/cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql b/cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql index 7f0a4833cb5..5842b9474f7 100644 --- a/cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql +++ b/cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql @@ -5,7 +5,7 @@ * @kind problem * @problem.severity error * @security-severity 7.5 - * @precision medium + * @precision high * @id cpp/wrong-type-format-argument * @tags reliability * correctness diff --git a/cpp/ql/src/Likely Bugs/Underspecified Functions/ImplicitFunctionDeclaration.qhelp b/cpp/ql/src/Likely Bugs/Underspecified Functions/ImplicitFunctionDeclaration.qhelp index 6ff60d38341..90a98e1bf57 100644 --- a/cpp/ql/src/Likely Bugs/Underspecified Functions/ImplicitFunctionDeclaration.qhelp +++ b/cpp/ql/src/Likely Bugs/Underspecified Functions/ImplicitFunctionDeclaration.qhelp @@ -14,6 +14,9 @@ function may behave unpredictably.

This may indicate a misspelled function name, or that the required header containing the function declaration has not been included.

+

Note: This query is not compatible with build mode: none databases, and produces +no results on those databases.

+

Provide an explicit declaration of the function before invoking it.

@@ -26,4 +29,4 @@ the function declaration has not been included.

  • SEI CERT C Coding Standard: DCL31-C. Declare identifiers before using them
    •  - \ No newline at end of file + diff --git a/cpp/ql/src/Likely Bugs/Underspecified Functions/ImplicitFunctionDeclaration.ql b/cpp/ql/src/Likely Bugs/Underspecified Functions/ImplicitFunctionDeclaration.ql index 6a55557cf70..00b29efbd0f 100644 --- a/cpp/ql/src/Likely Bugs/Underspecified Functions/ImplicitFunctionDeclaration.ql +++ b/cpp/ql/src/Likely Bugs/Underspecified Functions/ImplicitFunctionDeclaration.ql @@ -5,7 +5,7 @@ * may lead to unpredictable behavior. * @kind problem * @problem.severity warning - * @precision medium + * @precision high * @id cpp/implicit-function-declaration * @tags correctness * maintainability @@ -17,6 +17,11 @@ import TooFewArguments import TooManyArguments import semmle.code.cpp.commons.Exclusions +/* + * This query is not compatible with build mode: none databases, and produces + * no results on those databases. + */ + predicate locInfo(Locatable e, File file, int line, int col) { e.getFile() = file and e.getLocation().getStartLine() = line and @@ -39,6 +44,7 @@ predicate isCompiledAsC(File f) { from FunctionDeclarationEntry fdeIm, FunctionCall fc where isCompiledAsC(fdeIm.getFile()) and + not any(Compilation c).buildModeNone() and not isFromMacroDefinition(fc) and fdeIm.isImplicit() and sameLocation(fdeIm, fc) and diff --git a/cpp/ql/src/Likely Bugs/Underspecified Functions/MistypedFunctionArguments.qll b/cpp/ql/src/Likely Bugs/Underspecified Functions/MistypedFunctionArguments.qll index 2dced5d8d84..dbb457db505 100644 --- a/cpp/ql/src/Likely Bugs/Underspecified Functions/MistypedFunctionArguments.qll +++ b/cpp/ql/src/Likely Bugs/Underspecified Functions/MistypedFunctionArguments.qll @@ -79,9 +79,7 @@ private predicate hasZeroParamDecl(Function f) { // True if this file (or header) was compiled as a C file private predicate isCompiledAsC(File f) { - f.compiledAsC() - or - exists(File src | isCompiledAsC(src) | src.getAnIncludedFile() = f) + exists(File src | src.compiledAsC() | src.getAnIncludedFile*() = f) } predicate mistypedFunctionArguments(FunctionCall fc, Function f, Parameter p) { diff --git a/cpp/ql/src/Likely Bugs/Underspecified Functions/TooFewArguments.qll b/cpp/ql/src/Likely Bugs/Underspecified Functions/TooFewArguments.qll index 218a54b36c5..fd323513a49 100644 --- a/cpp/ql/src/Likely Bugs/Underspecified Functions/TooFewArguments.qll +++ b/cpp/ql/src/Likely Bugs/Underspecified Functions/TooFewArguments.qll @@ -28,9 +28,7 @@ private predicate hasZeroParamDecl(Function f) { /* Holds if this file (or header) was compiled as a C file. */ private predicate isCompiledAsC(File f) { - f.compiledAsC() - or - exists(File src | isCompiledAsC(src) | src.getAnIncludedFile() = f) + exists(File src | src.compiledAsC() | src.getAnIncludedFile*() = f) } /** Holds if `fc` is a call to `f` with too few arguments. */ diff --git a/cpp/ql/src/Likely Bugs/Underspecified Functions/TooManyArguments.qll b/cpp/ql/src/Likely Bugs/Underspecified Functions/TooManyArguments.qll index 7fba78b5550..ab2a98ae3a5 100644 --- a/cpp/ql/src/Likely Bugs/Underspecified Functions/TooManyArguments.qll +++ b/cpp/ql/src/Likely Bugs/Underspecified Functions/TooManyArguments.qll @@ -19,9 +19,7 @@ private predicate hasZeroParamDecl(Function f) { // True if this file (or header) was compiled as a C file private predicate isCompiledAsC(File f) { - f.compiledAsC() - or - exists(File src | isCompiledAsC(src) | src.getAnIncludedFile() = f) + exists(File src | src.compiledAsC() | src.getAnIncludedFile*() = f) } predicate tooManyArguments(FunctionCall fc, Function f) { diff --git a/cpp/ql/src/Security/CWE/CWE-190/ComparisonWithWiderType.ql b/cpp/ql/src/Security/CWE/CWE-190/ComparisonWithWiderType.ql index 3f330807304..7d9ef88adea 100644 --- a/cpp/ql/src/Security/CWE/CWE-190/ComparisonWithWiderType.ql +++ b/cpp/ql/src/Security/CWE/CWE-190/ComparisonWithWiderType.ql @@ -6,7 +6,7 @@ * @kind problem * @problem.severity warning * @security-severity 7.8 - * @precision medium + * @precision high * @tags reliability * security * external/cwe/cwe-190 diff --git a/cpp/ql/src/change-notes/2026-03-23-implicit-function-declaration.md b/cpp/ql/src/change-notes/2026-03-23-implicit-function-declaration.md new file mode 100644 index 00000000000..8c2c431ec24 --- /dev/null +++ b/cpp/ql/src/change-notes/2026-03-23-implicit-function-declaration.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The "Implicit function declaration" (`cpp/implicit-function-declaration`) query no longer produces results on `build mode: none` databases. These results were found to be very noisy and fundamentally imprecise in this mode. diff --git a/cpp/ql/src/change-notes/2026-04-02-comparison-with-wider-type.md b/cpp/ql/src/change-notes/2026-04-02-comparison-with-wider-type.md new file mode 100644 index 00000000000..c84e1dba404 --- /dev/null +++ b/cpp/ql/src/change-notes/2026-04-02-comparison-with-wider-type.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The "Comparison of narrow type with wide type in loop condition" (`cpp/comparison-with-wider-type`) query has been upgraded to `high` precision. This query will now run in the default code scanning suite. diff --git a/cpp/ql/src/change-notes/2026-04-02-implicit-function-declaration.md b/cpp/ql/src/change-notes/2026-04-02-implicit-function-declaration.md new file mode 100644 index 00000000000..dd0dbd4bc7d --- /dev/null +++ b/cpp/ql/src/change-notes/2026-04-02-implicit-function-declaration.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The "Implicit function declaration" (`cpp/implicit-function-declaration`) query has been upgraded to `high` precision. diff --git a/cpp/ql/src/change-notes/2026-04-02-integer-multiplication-cast-to-long.md b/cpp/ql/src/change-notes/2026-04-02-integer-multiplication-cast-to-long.md new file mode 100644 index 00000000000..cd6796b408f --- /dev/null +++ b/cpp/ql/src/change-notes/2026-04-02-integer-multiplication-cast-to-long.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The "Multiplication result converted to larger type" (`cpp/integer-multiplication-cast-to-long`) query has been upgraded to `high` precision. This query will now run in the default code scanning suite. diff --git a/cpp/ql/src/change-notes/2026-04-02-wrong-type-format-argument.md b/cpp/ql/src/change-notes/2026-04-02-wrong-type-format-argument.md new file mode 100644 index 00000000000..f8b9085dacc --- /dev/null +++ b/cpp/ql/src/change-notes/2026-04-02-wrong-type-format-argument.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The "Wrong type of arguments to formatting function" (`cpp/wrong-type-format-argument`) query has been upgraded to `high` precision. This query will now run in the default code scanning suite. diff --git a/csharp/ql/examples/snippets/integer_literal.ql b/csharp/ql/examples/snippets/integer_literal.ql index 4546c1d174b..36791fc8c37 100644 --- a/csharp/ql/examples/snippets/integer_literal.ql +++ b/csharp/ql/examples/snippets/integer_literal.ql @@ -9,5 +9,5 @@ import csharp from IntegerLiteral literal -where literal.getValue().toInt() = 0 +where literal.getIntValue() = 0 select literal diff --git a/csharp/ql/lib/semmle/code/csharp/Conversion.qll b/csharp/ql/lib/semmle/code/csharp/Conversion.qll index ec7ef9cac95..e151944dc38 100644 --- a/csharp/ql/lib/semmle/code/csharp/Conversion.qll +++ b/csharp/ql/lib/semmle/code/csharp/Conversion.qll @@ -232,14 +232,9 @@ private module Identity { */ pragma[nomagic] private predicate convTypeArguments(Type fromTypeArgument, Type toTypeArgument, int i) { - exists(int j | - fromTypeArgument = getTypeArgumentRanked(_, _, i) and - toTypeArgument = getTypeArgumentRanked(_, _, j) and - i <= j and - j <= i - | - convIdentity(fromTypeArgument, toTypeArgument) - ) + fromTypeArgument = getTypeArgumentRanked(_, _, pragma[only_bind_into](i)) and + toTypeArgument = getTypeArgumentRanked(_, _, pragma[only_bind_into](i)) and + convIdentity(fromTypeArgument, toTypeArgument) } pragma[nomagic] @@ -718,7 +713,7 @@ private class SignedIntegralConstantExpr extends Expr { } private predicate convConstantIntExpr(SignedIntegralConstantExpr e, SimpleType toType) { - exists(int n | n = e.getValue().toInt() | + exists(int n | n = e.getIntValue() | toType = any(SByteType t | n in [t.minValue() .. t.maxValue()]) or toType = any(ByteType t | n in [t.minValue() .. t.maxValue()]) @@ -735,7 +730,7 @@ private predicate convConstantIntExpr(SignedIntegralConstantExpr e, SimpleType t private predicate convConstantLongExpr(SignedIntegralConstantExpr e) { e.getType() instanceof LongType and - e.getValue().toInt() >= 0 + e.getIntValue() >= 0 } /** 6.1.10: Implicit reference conversions involving type parameters. */ @@ -929,19 +924,16 @@ private module Variance { private predicate convTypeArguments( TypeArgument fromTypeArgument, TypeArgument toTypeArgument, int i, TVariance v ) { - exists(int j | - fromTypeArgument = getTypeArgumentRanked(_, _, i, _) and - toTypeArgument = getTypeArgumentRanked(_, _, j, _) and - i <= j and - j <= i - | + fromTypeArgument = getTypeArgumentRanked(_, _, pragma[only_bind_into](i), _) and + toTypeArgument = getTypeArgumentRanked(_, _, pragma[only_bind_into](i), _) and + ( convIdentity(fromTypeArgument, toTypeArgument) and v = TNone() or - convRefTypeTypeArgumentOut(fromTypeArgument, toTypeArgument, j) and + convRefTypeTypeArgumentOut(fromTypeArgument, toTypeArgument, i) and v = TOut() or - convRefTypeTypeArgumentIn(toTypeArgument, fromTypeArgument, j) and + convRefTypeTypeArgumentIn(toTypeArgument, fromTypeArgument, i) and v = TIn() ) } diff --git a/csharp/ql/lib/semmle/code/csharp/commons/ComparisonTest.qll b/csharp/ql/lib/semmle/code/csharp/commons/ComparisonTest.qll index b4641560892..776e2e97c37 100644 --- a/csharp/ql/lib/semmle/code/csharp/commons/ComparisonTest.qll +++ b/csharp/ql/lib/semmle/code/csharp/commons/ComparisonTest.qll @@ -161,7 +161,7 @@ private newtype TComparisonTest = compare.getComparisonKind().isCompare() and outerKind = outer.getComparisonKind() and outer.getAnArgument() = compare.getExpr() and - i = outer.getAnArgument().getValue().toInt() + i = outer.getAnArgument().getIntValue() | outerKind.isEquality() and ( diff --git a/csharp/ql/lib/semmle/code/csharp/commons/Constants.qll b/csharp/ql/lib/semmle/code/csharp/commons/Constants.qll index 5025202eb21..a5f1bc43abe 100644 --- a/csharp/ql/lib/semmle/code/csharp/commons/Constants.qll +++ b/csharp/ql/lib/semmle/code/csharp/commons/Constants.qll @@ -32,13 +32,13 @@ private module ConstantComparisonOperation { private int maxValue(Expr expr) { if convertedType(expr) instanceof IntegralType and exists(expr.getValue()) - then result = expr.getValue().toInt() + then result = expr.getIntValue() else result = convertedType(expr).maxValue() } private int minValue(Expr expr) { if convertedType(expr) instanceof IntegralType and exists(expr.getValue()) - then result = expr.getValue().toInt() + then result = expr.getIntValue() else result = convertedType(expr).minValue() } diff --git a/csharp/ql/lib/semmle/code/csharp/controlflow/Guards.qll b/csharp/ql/lib/semmle/code/csharp/controlflow/Guards.qll index 6c1eb8c06eb..66b591cfcd2 100644 --- a/csharp/ql/lib/semmle/code/csharp/controlflow/Guards.qll +++ b/csharp/ql/lib/semmle/code/csharp/controlflow/Guards.qll @@ -60,25 +60,16 @@ private module GuardsInput implements override boolean asBooleanValue() { boolConst(this, result) } } - private predicate intConst(Expr e, int i) { - e.getValue().toInt() = i and - ( - e.getType() instanceof Enum - or - e.getType() instanceof IntegralType - ) - } - private class IntegerConstant extends ConstantExpr { - IntegerConstant() { intConst(this, _) } + IntegerConstant() { exists(this.getIntValue()) } - override int asIntegerValue() { intConst(this, result) } + override int asIntegerValue() { result = this.getIntValue() } } private class EnumConst extends ConstantExpr { EnumConst() { this.getType() instanceof Enum and this.hasValue() } - override int asIntegerValue() { result = this.getValue().toInt() } + override int asIntegerValue() { result = this.getIntValue() } } private class StringConstant extends ConstantExpr instanceof StringLiteral { @@ -517,35 +508,35 @@ class EnumerableCollectionExpr extends Expr { | // x.Length == 0 ct.getComparisonKind().isEquality() and - ct.getAnArgument().getValue().toInt() = 0 and + ct.getAnArgument().getIntValue() = 0 and branch = isEmpty or // x.Length == k, k > 0 ct.getComparisonKind().isEquality() and - ct.getAnArgument().getValue().toInt() > 0 and + ct.getAnArgument().getIntValue() > 0 and branch = true and isEmpty = false or // x.Length != 0 ct.getComparisonKind().isInequality() and - ct.getAnArgument().getValue().toInt() = 0 and + ct.getAnArgument().getIntValue() = 0 and branch = isEmpty.booleanNot() or // x.Length != k, k != 0 ct.getComparisonKind().isInequality() and - ct.getAnArgument().getValue().toInt() != 0 and + ct.getAnArgument().getIntValue() != 0 and branch = false and isEmpty = false or // x.Length > k, k >= 0 ct.getComparisonKind().isLessThan() and - ct.getFirstArgument().getValue().toInt() >= 0 and + ct.getFirstArgument().getIntValue() >= 0 and branch = true and isEmpty = false or // x.Length >= k, k > 0 ct.getComparisonKind().isLessThanEquals() and - ct.getFirstArgument().getValue().toInt() > 0 and + ct.getFirstArgument().getIntValue() > 0 and branch = true and isEmpty = false ) diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/ExternalFlow.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/ExternalFlow.qll index 024e9cf119d..f8cec8c4d9f 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/ExternalFlow.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/ExternalFlow.qll @@ -4,13 +4,17 @@ * Provides classes and predicates for dealing with MaD flow models specified * in data extensions and CSV format. * - * The CSV specification has the following columns: + * The extensible relations have the following columns: * - Sources: * `namespace; type; subtypes; name; signature; ext; output; kind; provenance` * - Sinks: * `namespace; type; subtypes; name; signature; ext; input; kind; provenance` * - Summaries: * `namespace; type; subtypes; name; signature; ext; input; output; kind; provenance` + * - Barriers: + * `namespace; type; subtypes; name; signature; ext; output; kind; provenance` + * - BarrierGuards: + * `namespace; type; subtypes; name; signature; ext; input; acceptingValue; kind; provenance` * - Neutrals: * `namespace; type; name; signature; kind; provenance` * A neutral is used to indicate that a callable is neutral with respect to flow (no summary), source (is not a source) or sink (is not a sink). @@ -69,14 +73,17 @@ * - "Field[f]": Selects the contents of field `f`. * - "Property[p]": Selects the contents of property `p`. * - * 8. The `kind` column is a tag that can be referenced from QL to determine to + * 8. The `acceptingValue` column of barrier guard models specifies the condition + * under which the guard blocks flow. It can be one of "true" or "false". In + * the future "no-exception", "not-zero", "null", "not-null" may be supported. + * 9. The `kind` column is a tag that can be referenced from QL to determine to * which classes the interpreted elements should be added. For example, for * sources "remote" indicates a default remote flow source, and for summaries * "taint" indicates a default additional taint step and "value" indicates a * globally applicable value-preserving step. For neutrals the kind can be `summary`, * `source` or `sink` to indicate that the neutral is neutral with respect to * flow (no summary), source (is not a source) or sink (is not a sink). - * 9. The `provenance` column is a tag to indicate the origin and verification of a model. + * 10. The `provenance` column is a tag to indicate the origin and verification of a model. * The format is {origin}-{verification} or just "manual" where the origin describes * the origin of the model and verification describes how the model has been verified. * Some examples are: @@ -230,11 +237,11 @@ module ModelValidation { result = "Unrecognized provenance description \"" + provenance + "\" in " + pred + " model." ) or - exists(string acceptingvalue | - barrierGuardModel(_, _, _, _, _, _, _, acceptingvalue, _, _, _) and - invalidAcceptingValue(acceptingvalue) and + exists(string acceptingValue | + barrierGuardModel(_, _, _, _, _, _, _, acceptingValue, _, _, _) and + invalidAcceptingValue(acceptingValue) and result = - "Unrecognized accepting value description \"" + acceptingvalue + + "Unrecognized accepting value description \"" + acceptingValue + "\" in barrier guard model." ) } @@ -482,13 +489,13 @@ private module Cached { private predicate barrierGuardChecks(Guard g, Expr e, GuardValue gv, TKindModelPair kmp) { exists( - SourceSinkInterpretationInput::InterpretNode n, AcceptingValue acceptingvalue, string kind, + SourceSinkInterpretationInput::InterpretNode n, AcceptingValue acceptingValue, string kind, string model | - isBarrierGuardNode(n, acceptingvalue, kind, model) and + isBarrierGuardNode(n, acceptingValue, kind, model) and n.asNode().asExpr() = e and kmp = TMkPair(kind, model) and - gv = convertAcceptingValue(acceptingvalue) + gv = convertAcceptingValue(acceptingValue) | g.(Call).getAnArgument() = e or g.(QualifiableExpr).getQualifier() = e ) diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/ExternalFlowExtensions.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/ExternalFlowExtensions.qll index 3461f0a5186..cd438ece284 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/ExternalFlowExtensions.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/ExternalFlowExtensions.qll @@ -33,7 +33,7 @@ extensible predicate barrierModel( */ extensible predicate barrierGuardModel( string namespace, string type, boolean subtypes, string name, string signature, string ext, - string input, string acceptingvalue, string kind, string provenance, QlBuiltins::ExtensionId madId + string input, string acceptingValue, string kind, string provenance, QlBuiltins::ExtensionId madId ); /** diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll index ec6bbf81fee..a7ab18a6290 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll @@ -253,13 +253,13 @@ module SourceSinkInterpretationInput implements } predicate barrierGuardElement( - Element e, string input, Public::AcceptingValue acceptingvalue, string kind, + Element e, string input, Public::AcceptingValue acceptingValue, string kind, Public::Provenance provenance, string model ) { exists( string namespace, string type, boolean subtypes, string name, string signature, string ext | - barrierGuardModel(namespace, type, subtypes, name, signature, ext, input, acceptingvalue, + barrierGuardModel(namespace, type, subtypes, name, signature, ext, input, acceptingValue, kind, provenance, model) and e = interpretElement(namespace, type, subtypes, name, signature, ext, _) ) diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/rangeanalysis/ConstantUtils.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/rangeanalysis/ConstantUtils.qll index e3f5deb9898..bea31ed7f55 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/rangeanalysis/ConstantUtils.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/rangeanalysis/ConstantUtils.qll @@ -23,7 +23,7 @@ predicate systemArrayLengthAccess(PropertyAccess pa) { * - a read of the `Length` of an array with `val` lengths. */ private predicate constantIntegerExpr(ExprNode e, int val) { - e.getValue().toInt() = val + e.getExpr().getIntValue() = val or exists(ExprNode src | e = getAnExplicitDefinitionRead(src) and diff --git a/csharp/ql/lib/semmle/code/csharp/exprs/Expr.qll b/csharp/ql/lib/semmle/code/csharp/exprs/Expr.qll index 36eda82531c..a26afb00490 100644 --- a/csharp/ql/lib/semmle/code/csharp/exprs/Expr.qll +++ b/csharp/ql/lib/semmle/code/csharp/exprs/Expr.qll @@ -57,6 +57,13 @@ class Expr extends ControlFlowElement, @expr { /** Gets the value of this expression, if any */ string getValue() { expr_value(this, result) } + /** Gets the integer value of this expression, if any. */ + cached + int getIntValue() { + result = this.getValue().toInt() and + (this.getType() instanceof IntegralType or this.getType() instanceof Enum) + } + /** Holds if this expression has a value. */ final predicate hasValue() { exists(this.getValue()) } diff --git a/csharp/ql/lib/semmle/code/csharp/frameworks/system/runtime/CompilerServices.qll b/csharp/ql/lib/semmle/code/csharp/frameworks/system/runtime/CompilerServices.qll index 2c0ba292b9c..aca2cb2cb1c 100644 --- a/csharp/ql/lib/semmle/code/csharp/frameworks/system/runtime/CompilerServices.qll +++ b/csharp/ql/lib/semmle/code/csharp/frameworks/system/runtime/CompilerServices.qll @@ -81,7 +81,7 @@ class SystemRuntimeCompilerServicesInlineArrayAttribute extends Attribute { /** * Gets the length of the inline array. */ - int getLength() { result = this.getConstructorArgument(0).getValue().toInt() } + int getLength() { result = this.getConstructorArgument(0).getIntValue() } } /** An attribute of type `System.Runtime.CompilerServices.OverloadResolutionPriority`. */ @@ -94,5 +94,5 @@ class SystemRuntimeCompilerServicesOverloadResolutionPriorityAttribute extends A /** * Gets the priority number. */ - int getPriority() { result = this.getConstructorArgument(0).getValue().toInt() } + int getPriority() { result = this.getConstructorArgument(0).getIntValue() } } diff --git a/csharp/ql/src/Likely Bugs/BadCheckOdd.ql b/csharp/ql/src/Likely Bugs/BadCheckOdd.ql index 34ae4b632ae..72924f9103d 100644 --- a/csharp/ql/src/Likely Bugs/BadCheckOdd.ql +++ b/csharp/ql/src/Likely Bugs/BadCheckOdd.ql @@ -13,7 +13,7 @@ import csharp predicate isDefinitelyPositive(Expr e) { - e.getValue().toInt() >= 0 or + e.getIntValue() >= 0 or e.(PropertyAccess).getTarget().hasName("Length") or e.(MethodCall).getTarget().hasUndecoratedName("Count") } @@ -23,12 +23,12 @@ where t.getLeftOperand() = lhs and t.getRightOperand() = rhs and not isDefinitelyPositive(lhs.getLeftOperand().stripCasts()) and - lhs.getRightOperand().(IntegerLiteral).getValue() = "2" and + lhs.getRightOperand().(IntegerLiteral).getIntValue() = 2 and ( - t instanceof EQExpr and rhs.getValue() = "1" and parity = "oddness" + t instanceof EQExpr and rhs.getIntValue() = 1 and parity = "oddness" or - t instanceof NEExpr and rhs.getValue() = "1" and parity = "evenness" + t instanceof NEExpr and rhs.getIntValue() = 1 and parity = "evenness" or - t instanceof GTExpr and rhs.getValue() = "0" and parity = "oddness" + t instanceof GTExpr and rhs.getIntValue() = 0 and parity = "oddness" ) select t, "Possibly invalid test for " + parity + ". This will fail for negative numbers." diff --git a/csharp/ql/src/Likely Bugs/MishandlingJapaneseEra.ql b/csharp/ql/src/Likely Bugs/MishandlingJapaneseEra.ql index c8df36bf7bf..3e54a3a00db 100644 --- a/csharp/ql/src/Likely Bugs/MishandlingJapaneseEra.ql +++ b/csharp/ql/src/Likely Bugs/MishandlingJapaneseEra.ql @@ -27,8 +27,8 @@ predicate isExactEraStartDateCreation(ObjectCreation cr) { cr.getType().hasFullyQualifiedName("System", "DateTime") or cr.getType().hasFullyQualifiedName("System", "DateTimeOffset") ) and - isEraStart(cr.getArgument(0).getValue().toInt(), cr.getArgument(1).getValue().toInt(), - cr.getArgument(2).getValue().toInt()) + isEraStart(cr.getArgument(0).getIntValue(), cr.getArgument(1).getIntValue(), + cr.getArgument(2).getIntValue()) } predicate isDateFromJapaneseCalendarToDateTime(MethodCall mc) { @@ -44,7 +44,7 @@ predicate isDateFromJapaneseCalendarToDateTime(MethodCall mc) { mc.getNumberOfArguments() = 7 // implicitly current era or mc.getNumberOfArguments() = 8 and - mc.getArgument(7).getValue() = "0" + mc.getArgument(7).getIntValue() = 0 ) // explicitly current era } diff --git a/csharp/ql/src/Likely Bugs/PossibleLossOfPrecision.ql b/csharp/ql/src/Likely Bugs/PossibleLossOfPrecision.ql index 8b719bb92a5..1c41d24fb3c 100644 --- a/csharp/ql/src/Likely Bugs/PossibleLossOfPrecision.ql +++ b/csharp/ql/src/Likely Bugs/PossibleLossOfPrecision.ql @@ -40,8 +40,8 @@ predicate convertedToFloatOrDecimal(Expr e, Type t) { /** Holds if `div` is an exact integer division. */ predicate exactDivision(DivExpr div) { exists(int numerator, int denominator | - numerator = div.getNumerator().stripCasts().getValue().toInt() and - denominator = div.getDenominator().stripCasts().getValue().toInt() and + numerator = div.getNumerator().stripCasts().getIntValue() and + denominator = div.getDenominator().stripCasts().getIntValue() and numerator % denominator = 0 ) } diff --git a/csharp/ql/src/Security Features/InsufficientKeySize.ql b/csharp/ql/src/Security Features/InsufficientKeySize.ql index 02be9125835..98a7852fbaf 100644 --- a/csharp/ql/src/Security Features/InsufficientKeySize.ql +++ b/csharp/ql/src/Security Features/InsufficientKeySize.ql @@ -20,7 +20,7 @@ predicate incorrectUseOfRC2(Assignment e, string msg) { .getDeclaringType() .hasFullyQualifiedName("System.Security.Cryptography", "RC2CryptoServiceProvider") ) and - e.getRightOperand().getValue().toInt() < 128 and + e.getRightOperand().getIntValue() < 128 and msg = "Key size should be at least 128 bits for RC2 encryption." } @@ -28,7 +28,7 @@ predicate incorrectUseOfDsa(ObjectCreation e, string msg) { e.getTarget() .getDeclaringType() .hasFullyQualifiedName("System.Security.Cryptography", "DSACryptoServiceProvider") and - exists(Expr i | e.getArgument(0) = i and i.getValue().toInt() < 2048) and + exists(Expr i | e.getArgument(0) = i and i.getIntValue() < 2048) and msg = "Key size should be at least 2048 bits for DSA encryption." } @@ -36,7 +36,7 @@ predicate incorrectUseOfRsa(ObjectCreation e, string msg) { e.getTarget() .getDeclaringType() .hasFullyQualifiedName("System.Security.Cryptography", "RSACryptoServiceProvider") and - exists(Expr i | e.getArgument(0) = i and i.getValue().toInt() < 2048) and + exists(Expr i | e.getArgument(0) = i and i.getIntValue() < 2048) and msg = "Key size should be at least 2048 bits for RSA encryption." } diff --git a/go/ql/lib/semmle/go/dataflow/ExternalFlow.qll b/go/ql/lib/semmle/go/dataflow/ExternalFlow.qll index e1170aeda24..f0dc0cf0ca2 100644 --- a/go/ql/lib/semmle/go/dataflow/ExternalFlow.qll +++ b/go/ql/lib/semmle/go/dataflow/ExternalFlow.qll @@ -4,13 +4,17 @@ * Provides classes and predicates for dealing with flow models specified * in data extensions and CSV format. * - * The CSV specification has the following columns: + * The extensible relations have the following columns: * - Sources: * `package; type; subtypes; name; signature; ext; output; kind; provenance` * - Sinks: * `package; type; subtypes; name; signature; ext; input; kind; provenance` * - Summaries: * `package; type; subtypes; name; signature; ext; input; output; kind; provenance` + * - Barriers: + * `package; type; subtypes; name; signature; ext; output; kind; provenance` + * - BarrierGuards: + * `package; type; subtypes; name; signature; ext; input; acceptingValue; kind; provenance` * - Neutrals: * `package; type; name; signature; kind; provenance` * A neutral is used to indicate that a callable is neutral with respect to flow (no summary), source (is not a source) or sink (is not a sink). @@ -78,11 +82,23 @@ * - "MapValue": Selects a value in a map. * - "Dereference": Selects the value referenced by a pointer. * - * 8. The `kind` column is a tag that can be referenced from QL to determine to + * 8. The `acceptingValue` column of barrier guard models specifies the condition + * under which the guard blocks flow. It can be one of "true" or "false". In + * the future "no-exception", "not-zero", "null", "not-null" may be supported. + * 9. The `kind` column is a tag that can be referenced from QL to determine to * which classes the interpreted elements should be added. For example, for * sources "remote" indicates a default remote flow source, and for summaries * "taint" indicates a default additional taint step and "value" indicates a * globally applicable value-preserving step. + * 10. The `provenance` column is a tag to indicate the origin and verification of a model. + * The format is {origin}-{verification} or just "manual" where the origin describes + * the origin of the model and verification describes how the model has been verified. + * Some examples are: + * - "df-generated": The model has been generated by the model generator tool. + * - "df-manual": The model has been generated by the model generator and verified by a human. + * - "manual": The model has been written by hand. + * This information is used in a heuristic for dataflow analysis to determine, if a + * model or source code should be used for determining flow. */ overlay[local?] module; @@ -250,11 +266,11 @@ module ModelValidation { result = "Unrecognized provenance description \"" + provenance + "\" in " + pred + " model." ) or - exists(string acceptingvalue | - barrierGuardModel(_, _, _, _, _, _, _, acceptingvalue, _, _, _) and - invalidAcceptingValue(acceptingvalue) and + exists(string acceptingValue | + barrierGuardModel(_, _, _, _, _, _, _, acceptingValue, _, _, _) and + invalidAcceptingValue(acceptingValue) and result = - "Unrecognized accepting value description \"" + acceptingvalue + + "Unrecognized accepting value description \"" + acceptingValue + "\" in barrier guard model." ) } @@ -462,13 +478,13 @@ private module Cached { private predicate barrierGuardChecks(DataFlow::Node g, Expr e, boolean gv, TKindModelPair kmp) { exists( - SourceSinkInterpretationInput::InterpretNode n, Public::AcceptingValue acceptingvalue, + SourceSinkInterpretationInput::InterpretNode n, Public::AcceptingValue acceptingValue, string kind, string model | - isBarrierGuardNode(n, acceptingvalue, kind, model) and + isBarrierGuardNode(n, acceptingValue, kind, model) and n.asNode().asExpr() = e and kmp = TMkPair(kind, model) and - gv = convertAcceptingValue(acceptingvalue) + gv = convertAcceptingValue(acceptingValue) | g.asExpr().(CallExpr).getAnArgument() = e // TODO: qualifier? ) diff --git a/go/ql/lib/semmle/go/dataflow/internal/ExternalFlowExtensions.qll b/go/ql/lib/semmle/go/dataflow/internal/ExternalFlowExtensions.qll index 5d43cf674c1..ab2a241e14a 100644 --- a/go/ql/lib/semmle/go/dataflow/internal/ExternalFlowExtensions.qll +++ b/go/ql/lib/semmle/go/dataflow/internal/ExternalFlowExtensions.qll @@ -35,7 +35,7 @@ extensible predicate barrierModel( */ extensible predicate barrierGuardModel( string package, string type, boolean subtypes, string name, string signature, string ext, - string input, string acceptingvalue, string kind, string provenance, QlBuiltins::ExtensionId madId + string input, string acceptingValue, string kind, string provenance, QlBuiltins::ExtensionId madId ); /** diff --git a/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll b/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll index 240665bd492..ff727286c3b 100644 --- a/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll +++ b/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll @@ -174,13 +174,13 @@ module SourceSinkInterpretationInput implements } predicate barrierGuardElement( - Element e, string input, Public::AcceptingValue acceptingvalue, string kind, + Element e, string input, Public::AcceptingValue acceptingValue, string kind, Public::Provenance provenance, string model ) { exists( string package, string type, boolean subtypes, string name, string signature, string ext | - barrierGuardModel(package, type, subtypes, name, signature, ext, input, acceptingvalue, kind, + barrierGuardModel(package, type, subtypes, name, signature, ext, input, acceptingValue, kind, provenance, model) and e = interpretElement(package, type, subtypes, name, signature, ext) ) diff --git a/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll b/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll index 1536c81aa08..a6a9347ca03 100644 --- a/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll +++ b/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll @@ -4,13 +4,17 @@ * Provides classes and predicates for dealing with flow models specified * in data extensions and CSV format. * - * The CSV specification has the following columns: + * The extensible relations have the following columns: * - Sources: * `package; type; subtypes; name; signature; ext; output; kind; provenance` * - Sinks: * `package; type; subtypes; name; signature; ext; input; kind; provenance` * - Summaries: * `package; type; subtypes; name; signature; ext; input; output; kind; provenance` + * - Barriers: + * `package; type; subtypes; name; signature; ext; output; kind; provenance` + * - BarrierGuards: + * `package; type; subtypes; name; signature; ext; input; acceptingValue; kind; provenance` * - Neutrals: * `package; type; name; signature; kind; provenance` * A neutral is used to indicate that a callable is neutral with respect to flow (no summary), source (is not a source) or sink (is not a sink). @@ -69,14 +73,17 @@ * in the given range. The range is inclusive at both ends. * - "ReturnValue": Selects the return value of a call to the selected element. * - "Element": Selects the collection elements of the selected element. - * 8. The `kind` column is a tag that can be referenced from QL to determine to + * 8. The `acceptingValue` column of barrier guard models specifies the condition + * under which the guard blocks flow. It can be one of "true" or "false". In + * the future "no-exception", "not-zero", "null", "not-null" may be supported. + * 9. The `kind` column is a tag that can be referenced from QL to determine to * which classes the interpreted elements should be added. For example, for * sources "remote" indicates a default remote flow source, and for summaries * "taint" indicates a default additional taint step and "value" indicates a * globally applicable value-preserving step. For neutrals the kind can be `summary`, * `source` or `sink` to indicate that the neutral is neutral with respect to * flow (no summary), source (is not a source) or sink (is not a sink). - * 9. The `provenance` column is a tag to indicate the origin and verification of a model. + * 10. The `provenance` column is a tag to indicate the origin and verification of a model. * The format is {origin}-{verification} or just "manual" where the origin describes * the origin of the model and verification describes how the model has been verified. * Some examples are: @@ -358,11 +365,11 @@ module ModelValidation { result = "Unrecognized provenance description \"" + provenance + "\" in " + pred + " model." ) or - exists(string acceptingvalue | - barrierGuardModel(_, _, _, _, _, _, _, acceptingvalue, _, _, _) and - invalidAcceptingValue(acceptingvalue) and + exists(string acceptingValue | + barrierGuardModel(_, _, _, _, _, _, _, acceptingValue, _, _, _) and + invalidAcceptingValue(acceptingValue) and result = - "Unrecognized accepting value description \"" + acceptingvalue + + "Unrecognized accepting value description \"" + acceptingValue + "\" in barrier guard model." ) } @@ -583,13 +590,13 @@ private module Cached { private predicate barrierGuardChecks(Guard g, Expr e, GuardValue gv, TKindModelPair kmp) { exists( - SourceSinkInterpretationInput::InterpretNode n, AcceptingValue acceptingvalue, string kind, + SourceSinkInterpretationInput::InterpretNode n, AcceptingValue acceptingValue, string kind, string model | - isBarrierGuardNode(n, acceptingvalue, kind, model) and + isBarrierGuardNode(n, acceptingValue, kind, model) and n.asNode().asExpr() = e and kmp = TMkPair(kind, model) and - gv = convertAcceptingValue(acceptingvalue) + gv = convertAcceptingValue(acceptingValue) | g.(Call).getAnArgument() = e or g.(MethodCall).getQualifier() = e ) diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/ExternalFlowExtensions.qll b/java/ql/lib/semmle/code/java/dataflow/internal/ExternalFlowExtensions.qll index be474ad4535..3c6b003876d 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/ExternalFlowExtensions.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/ExternalFlowExtensions.qll @@ -35,7 +35,7 @@ extensible predicate barrierModel( */ extensible predicate barrierGuardModel( string package, string type, boolean subtypes, string name, string signature, string ext, - string input, string acceptingvalue, string kind, string provenance, QlBuiltins::ExtensionId madId + string input, string acceptingValue, string kind, string provenance, QlBuiltins::ExtensionId madId ); /** diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll b/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll index 64fa30c7d91..453b7ccae11 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll @@ -282,7 +282,7 @@ module SourceSinkInterpretationInput implements } predicate barrierGuardElement( - Element e, string input, Public::AcceptingValue acceptingvalue, string kind, + Element e, string input, Public::AcceptingValue acceptingValue, string kind, Public::Provenance provenance, string model ) { exists( @@ -290,7 +290,7 @@ module SourceSinkInterpretationInput implements SourceOrSinkElement baseBarrier, string originalInput | barrierGuardModel(namespace, type, subtypes, name, signature, ext, originalInput, - acceptingvalue, kind, provenance, model) and + acceptingValue, kind, provenance, model) and baseBarrier = interpretElement(namespace, type, subtypes, name, signature, ext, _) and ( e = baseBarrier and input = originalInput diff --git a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll index 60fe40e716d..155fb4b7c78 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll @@ -10,6 +10,10 @@ * `type, path, kind` * - Summaries: * `type, path, input, output, kind` + * - Barriers: + * `type, path, kind` + * - BarrierGuards: + * `type, path, acceptingValue, kind` * - Types: * `type1, type2, path` * @@ -42,7 +46,8 @@ * 3. The `input` and `output` columns specify how data enters and leaves the element selected by the * first `(type, path)` tuple. Both strings are `.`-separated access paths * of the same syntax as the `path` column. - * 4. The `kind` column is a tag that can be referenced from QL to determine to + * 4. The `acceptingValue` column of barrier guard models specifies which branch of the guard is blocking flow. It can be "true" or "false". + * 5. The `kind` column is a tag that can be referenced from QL to determine to * which classes the interpreted elements should be added. For example, for * sources `"remote"` indicates a default remote flow source, and for summaries * `"taint"` indicates a default additional taint step and `"value"` indicates a @@ -355,11 +360,11 @@ private predicate barrierModel(string type, string path, string kind, string mod /** Holds if a barrier guard model exists for the given parameters. */ private predicate barrierGuardModel( - string type, string path, string branch, string kind, string model + string type, string path, string acceptingValue, string kind, string model ) { // No deprecation adapter for barrier models, they were not around back then. exists(QlBuiltins::ExtensionId madId | - Extensions::barrierGuardModel(type, path, branch, kind, madId) and + Extensions::barrierGuardModel(type, path, acceptingValue, kind, madId) and model = "MaD:" + madId.toString() ) } @@ -783,16 +788,16 @@ module ModelOutput { } /** - * Holds if a barrier model contributed `barrier` with the given `kind` for the given `branch`. + * Holds if a barrier model contributed `barrier` with the given `kind` for the given `acceptingValue`. */ cached - API::Node getABarrierGuardNode(string kind, boolean branch, string model) { - exists(string type, string path, string branch_str | - branch = true and branch_str = "true" + API::Node getABarrierGuardNode(string kind, boolean acceptingValue, string model) { + exists(string type, string path, string acceptingValue_str | + acceptingValue = true and acceptingValue_str = "true" or - branch = false and branch_str = "false" + acceptingValue = false and acceptingValue_str = "false" | - barrierGuardModel(type, path, branch_str, kind, model) and + barrierGuardModel(type, path, acceptingValue_str, kind, model) and result = getNodeFromPath(type, path) ) } @@ -856,12 +861,12 @@ module ModelOutput { API::Node getABarrierNode(string kind) { result = getABarrierNode(kind, _) } /** - * Holds if an external model contributed `barrier-guard` with the given `kind` and `branch`. + * Holds if an external model contributed `barrier-guard` with the given `kind` and `acceptingValue`. * * INTERNAL: Do not use. */ - API::Node getABarrierGuardNode(string kind, boolean branch) { - result = getABarrierGuardNode(kind, branch, _) + API::Node getABarrierGuardNode(string kind, boolean acceptingValue) { + result = getABarrierGuardNode(kind, acceptingValue, _) } /** diff --git a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsExtensions.qll b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsExtensions.qll index 2a644aabb95..8d8a4f5fd88 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsExtensions.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsExtensions.qll @@ -33,11 +33,11 @@ extensible predicate barrierModel( * of the given `kind` and `madId` is the data extension row number. * `path` is assumed to lead to a parameter of a call (possibly `self`), and * the call is guarding the parameter. - * `branch` is either `true` or `false`, indicating which branch of the guard - * is protecting the parameter. + * `acceptingValue` is either `true` or `false`, indicating which branch of + * the guard is protecting the parameter. */ extensible predicate barrierGuardModel( - string type, string path, string branch, string kind, QlBuiltins::ExtensionId madId + string type, string path, string acceptingValue, string kind, QlBuiltins::ExtensionId madId ); /** diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/MissingRateLimiting.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/MissingRateLimiting.qll index 5f4ad1b3d73..8dd9c483144 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/MissingRateLimiting.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/MissingRateLimiting.qll @@ -191,3 +191,21 @@ class RouteHandlerLimitedByRateLimiterFlexible extends RateLimitingMiddleware in private class FastifyRateLimiter extends RateLimitingMiddleware { FastifyRateLimiter() { this = DataFlow::moduleImport("fastify-rate-limit") } } + +/** + * An options object with a `rateLimit` config passed to a Fastify shorthand route method, + * such as `fastify.post('/path', { config: { rateLimit: { ... } } }, handler)`. + */ +private class FastifyPerRouteRateLimit extends RateLimitingMiddleware { + FastifyPerRouteRateLimit() { + exists(Fastify::RouteSetup setup | + not setup.getMethodName() = ["route", "addHook"] and + setup.getNumArgument() >= 3 and + this.flowsTo(setup.getArgument(1)) + | + exists(this.getAPropertySource("config").getAPropertySource("rateLimit")) + or + exists(this.getAPropertySource("rateLimit")) + ) + } +} diff --git a/javascript/ql/src/change-notes/2026-04-13-fastify-per-route-rate-limit.md b/javascript/ql/src/change-notes/2026-04-13-fastify-per-route-rate-limit.md new file mode 100644 index 00000000000..56d52388524 --- /dev/null +++ b/javascript/ql/src/change-notes/2026-04-13-fastify-per-route-rate-limit.md @@ -0,0 +1,5 @@ +--- +category: minorAnalysis +--- +* The query `js/missing-rate-limiting` now takes Fastify per-route + rate limiting into account. diff --git a/javascript/ql/test/query-tests/Security/CWE-770/MissingRateLimit/MissingRateLimiting.expected b/javascript/ql/test/query-tests/Security/CWE-770/MissingRateLimit/MissingRateLimiting.expected index 3d7bc2954eb..8d197d6e37f 100644 --- a/javascript/ql/test/query-tests/Security/CWE-770/MissingRateLimit/MissingRateLimiting.expected +++ b/javascript/ql/test/query-tests/Security/CWE-770/MissingRateLimit/MissingRateLimiting.expected @@ -9,3 +9,4 @@ | tst.js:64:25:64:63 | functio ... req); } | This route handler performs $@, but is not rate-limited. | tst.js:64:46:64:60 | verifyUser(req) | authorization | | tst.js:76:25:76:53 | catchAs ... ndler1) | This route handler performs $@, but is not rate-limited. | tst.js:14:40:14:46 | login() | authorization | | tst.js:88:24:88:40 | expensiveHandler1 | This route handler performs $@, but is not rate-limited. | tst.js:14:40:14:46 | login() | authorization | +| tst.js:112:28:112:44 | expensiveHandler1 | This route handler performs $@, but is not rate-limited. | tst.js:14:40:14:46 | login() | authorization | diff --git a/javascript/ql/test/query-tests/Security/CWE-770/MissingRateLimit/tst.js b/javascript/ql/test/query-tests/Security/CWE-770/MissingRateLimit/tst.js index 4f778afef68..5b4312bbbe0 100644 --- a/javascript/ql/test/query-tests/Security/CWE-770/MissingRateLimit/tst.js +++ b/javascript/ql/test/query-tests/Security/CWE-770/MissingRateLimit/tst.js @@ -88,3 +88,25 @@ const fastifyApp = require('fastify')(); fastifyApp.get('/foo', expensiveHandler1); // $ Alert fastifyApp.register(require('fastify-rate-limit')); fastifyApp.get('/bar', expensiveHandler1); + +// Fastify per-route rate limiting via config.rateLimit +const fastifyApp2 = require('fastify')(); +fastifyApp2.register(require('@fastify/rate-limit')); + +fastifyApp2.post('/login', { + config: { + rateLimit: { + max: 3, + timeWindow: '1 minute' + } + } +}, expensiveHandler1); // OK - has per-route rateLimit config + +fastifyApp2.post('/signup', { + rateLimit: { + max: 5, + timeWindow: '1 minute' + } +}, expensiveHandler1); // OK - has per-route rateLimit directly in options + +fastifyApp2.post('/other', expensiveHandler1); // $ Alert - no rate limiting diff --git a/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModels.qll b/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModels.qll index 60fe40e716d..155fb4b7c78 100644 --- a/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModels.qll +++ b/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModels.qll @@ -10,6 +10,10 @@ * `type, path, kind` * - Summaries: * `type, path, input, output, kind` + * - Barriers: + * `type, path, kind` + * - BarrierGuards: + * `type, path, acceptingValue, kind` * - Types: * `type1, type2, path` * @@ -42,7 +46,8 @@ * 3. The `input` and `output` columns specify how data enters and leaves the element selected by the * first `(type, path)` tuple. Both strings are `.`-separated access paths * of the same syntax as the `path` column. - * 4. The `kind` column is a tag that can be referenced from QL to determine to + * 4. The `acceptingValue` column of barrier guard models specifies which branch of the guard is blocking flow. It can be "true" or "false". + * 5. The `kind` column is a tag that can be referenced from QL to determine to * which classes the interpreted elements should be added. For example, for * sources `"remote"` indicates a default remote flow source, and for summaries * `"taint"` indicates a default additional taint step and `"value"` indicates a @@ -355,11 +360,11 @@ private predicate barrierModel(string type, string path, string kind, string mod /** Holds if a barrier guard model exists for the given parameters. */ private predicate barrierGuardModel( - string type, string path, string branch, string kind, string model + string type, string path, string acceptingValue, string kind, string model ) { // No deprecation adapter for barrier models, they were not around back then. exists(QlBuiltins::ExtensionId madId | - Extensions::barrierGuardModel(type, path, branch, kind, madId) and + Extensions::barrierGuardModel(type, path, acceptingValue, kind, madId) and model = "MaD:" + madId.toString() ) } @@ -783,16 +788,16 @@ module ModelOutput { } /** - * Holds if a barrier model contributed `barrier` with the given `kind` for the given `branch`. + * Holds if a barrier model contributed `barrier` with the given `kind` for the given `acceptingValue`. */ cached - API::Node getABarrierGuardNode(string kind, boolean branch, string model) { - exists(string type, string path, string branch_str | - branch = true and branch_str = "true" + API::Node getABarrierGuardNode(string kind, boolean acceptingValue, string model) { + exists(string type, string path, string acceptingValue_str | + acceptingValue = true and acceptingValue_str = "true" or - branch = false and branch_str = "false" + acceptingValue = false and acceptingValue_str = "false" | - barrierGuardModel(type, path, branch_str, kind, model) and + barrierGuardModel(type, path, acceptingValue_str, kind, model) and result = getNodeFromPath(type, path) ) } @@ -856,12 +861,12 @@ module ModelOutput { API::Node getABarrierNode(string kind) { result = getABarrierNode(kind, _) } /** - * Holds if an external model contributed `barrier-guard` with the given `kind` and `branch`. + * Holds if an external model contributed `barrier-guard` with the given `kind` and `acceptingValue`. * * INTERNAL: Do not use. */ - API::Node getABarrierGuardNode(string kind, boolean branch) { - result = getABarrierGuardNode(kind, branch, _) + API::Node getABarrierGuardNode(string kind, boolean acceptingValue) { + result = getABarrierGuardNode(kind, acceptingValue, _) } /** diff --git a/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModelsExtensions.qll b/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModelsExtensions.qll index 2a644aabb95..8d8a4f5fd88 100644 --- a/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModelsExtensions.qll +++ b/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModelsExtensions.qll @@ -33,11 +33,11 @@ extensible predicate barrierModel( * of the given `kind` and `madId` is the data extension row number. * `path` is assumed to lead to a parameter of a call (possibly `self`), and * the call is guarding the parameter. - * `branch` is either `true` or `false`, indicating which branch of the guard - * is protecting the parameter. + * `acceptingValue` is either `true` or `false`, indicating which branch of + * the guard is protecting the parameter. */ extensible predicate barrierGuardModel( - string type, string path, string branch, string kind, QlBuiltins::ExtensionId madId + string type, string path, string acceptingValue, string kind, QlBuiltins::ExtensionId madId ); /** diff --git a/python/ql/src/Statements/ModificationOfLocals.ql b/python/ql/src/Statements/ModificationOfLocals.ql index e4791a410f7..f32ddcf7884 100644 --- a/python/ql/src/Statements/ModificationOfLocals.ql +++ b/python/ql/src/Statements/ModificationOfLocals.ql @@ -12,10 +12,10 @@ */ import python -private import LegacyPointsTo +private import semmle.python.ApiGraphs -predicate originIsLocals(ControlFlowNodeWithPointsTo n) { - n.pointsTo(_, _, Value::named("locals").getACall()) +predicate originIsLocals(ControlFlowNode n) { + API::builtin("locals").getReturn().getAValueReachableFromSource().asCfgNode() = n } predicate modification_of_locals(ControlFlowNode f) { @@ -37,5 +37,8 @@ where // in module level scope `locals() == globals()` // see https://docs.python.org/3/library/functions.html#locals // FP report in https://github.com/github/codeql/issues/6674 - not a.getScope() instanceof ModuleScope + not a.getScope() instanceof Module and + // in class level scope `locals()` reflects the class namespace, + // so modifications do take effect. + not a.getScope() instanceof Class select a, "Modification of the locals() dictionary will have no effect on the local variables." diff --git a/python/ql/src/Statements/UnreachableCode.ql b/python/ql/src/Statements/UnreachableCode.ql index 55582ed2f06..200e073cff6 100644 --- a/python/ql/src/Statements/UnreachableCode.ql +++ b/python/ql/src/Statements/UnreachableCode.ql @@ -13,7 +13,7 @@ */ import python -private import LegacyPointsTo +private import semmle.python.ApiGraphs predicate typing_import(ImportingStmt is) { exists(Module m | @@ -34,11 +34,7 @@ predicate unique_yield(Stmt s) { /** Holds if `contextlib.suppress` may be used in the same scope as `s` */ predicate suppression_in_scope(Stmt s) { exists(With w | - w.getContextExpr() - .(Call) - .getFunc() - .(ExprWithPointsTo) - .pointsTo(Value::named("contextlib.suppress")) and + w.getContextExpr() = API::moduleImport("contextlib").getMember("suppress").getACall().asExpr() and w.getScope() = s.getScope() ) } diff --git a/python/ql/src/Statements/UnusedExceptionObject.ql b/python/ql/src/Statements/UnusedExceptionObject.ql index 9a6a3650b7e..890cdc963ac 100644 --- a/python/ql/src/Statements/UnusedExceptionObject.ql +++ b/python/ql/src/Statements/UnusedExceptionObject.ql @@ -12,11 +12,49 @@ */ import python -private import LegacyPointsTo +private import semmle.python.dataflow.new.internal.DataFlowDispatch +private import semmle.python.dataflow.new.internal.Builtins +private import semmle.python.ApiGraphs -from Call call, ClassValue ex +/** + * Holds if `cls` is a user-defined exception class, i.e. it transitively + * extends one of the builtin exception base classes. + */ +predicate isUserDefinedExceptionClass(Class cls) { + cls.getABase() = + API::builtin(["BaseException", "Exception"]).getAValueReachableFromSource().asExpr() + or + isUserDefinedExceptionClass(getADirectSuperclass(cls)) +} + +/** + * Gets the name of a builtin exception class. + */ +string getBuiltinExceptionName() { + result = Builtins::getBuiltinName() and + ( + result.matches("%Error") or + result.matches("%Exception") or + result.matches("%Warning") or + result = + ["GeneratorExit", "KeyboardInterrupt", "StopIteration", "StopAsyncIteration", "SystemExit"] + ) +} + +/** + * Holds if `call` is an instantiation of an exception class. + */ +predicate isExceptionInstantiation(Call call) { + exists(Class cls | + classTracker(cls).asExpr() = call.getFunc() and + isUserDefinedExceptionClass(cls) + ) + or + call.getFunc() = API::builtin(getBuiltinExceptionName()).getAValueReachableFromSource().asExpr() +} + +from Call call where - call.getFunc().(ExprWithPointsTo).pointsTo(ex) and - ex.getASuperType() = ClassValue::exception() and + isExceptionInstantiation(call) and exists(ExprStmt s | s.getValue() = call) select call, "Instantiating an exception, but not raising it, has no effect." diff --git a/python/ql/src/Statements/UseOfExit.ql b/python/ql/src/Statements/UseOfExit.ql index 437ff93b537..2310a839f67 100644 --- a/python/ql/src/Statements/UseOfExit.ql +++ b/python/ql/src/Statements/UseOfExit.ql @@ -12,10 +12,12 @@ */ import python -private import LegacyPointsTo +private import semmle.python.ApiGraphs from CallNode call, string name -where call.getFunction().(ControlFlowNodeWithPointsTo).pointsTo(Value::siteQuitter(name)) +where + name = ["exit", "quit"] and + call = API::builtin(name).getACall().asCfgNode() select call, "The '" + name + "' site.Quitter object may not exist if the 'site' module is not loaded or is modified." diff --git a/python/ql/test/query-tests/Statements/general/test.py b/python/ql/test/query-tests/Statements/general/test.py index eee63fa89e8..a5848f7c718 100644 --- a/python/ql/test/query-tests/Statements/general/test.py +++ b/python/ql/test/query-tests/Statements/general/test.py @@ -174,3 +174,9 @@ def assert_ok(seq): # False positive. ODASA-8042. Fixed in PR #2401. class false_positive: e = (x for x in []) + +# In class-level scope `locals()` reflects the class namespace, +# so modifications do take effect. +class MyClass: + locals()['x'] = 43 # OK + y = x diff --git a/python/scripts/create-extractor-pack.sh b/python/scripts/create-extractor-pack.sh new file mode 100755 index 00000000000..ae9248f6ccf --- /dev/null +++ b/python/scripts/create-extractor-pack.sh @@ -0,0 +1,67 @@ +#!/bin/bash +# +# Build a local Python extractor pack from source. +# +# Usage with the CodeQL CLI (run from the repository root): +# +# codeql database create -l python -s --search-path . +# codeql test run --search-path . python/ql/test/ +# +set -eux + +if [[ "$OSTYPE" == "linux-gnu"* ]]; then + platform="linux64" +elif [[ "$OSTYPE" == "darwin"* ]]; then + platform="osx64" +else + echo "Unknown OS" + exit 1 +fi + +cd "$(dirname "$0")/.." + +# Build the tsg-python Rust binary +(cd extractor/tsg-python && cargo build --release) +tsg_bin="extractor/tsg-python/target/release/tsg-python" + +# Generate python3src.zip from the Python extractor source. +# make_zips.py creates the zip in the source directory and then copies it to the +# given output directory. We use a temporary directory to avoid a same-file copy +# error, then move the zip back. +tmpdir=$(mktemp -d) +trap 'rm -rf "$tmpdir"' EXIT +(cd extractor && python3 make_zips.py "$tmpdir") +cp "$tmpdir/python3src.zip" extractor/python3src.zip + +# Assemble the extractor pack +rm -rf extractor-pack +mkdir -p extractor-pack/tools/${platform} + +# Root-level metadata and schema files +cp codeql-extractor.yml extractor-pack/ +cp ql/lib/semmlecode.python.dbscheme extractor-pack/ +cp ql/lib/semmlecode.python.dbscheme.stats extractor-pack/ + +# Python extractor engine files (into tools/) +cp extractor/python_tracer.py extractor-pack/tools/ +cp extractor/index.py extractor-pack/tools/ +cp extractor/setup.py extractor-pack/tools/ +cp extractor/convert_setup.py extractor-pack/tools/ +cp extractor/get_venv_lib.py extractor-pack/tools/ +cp extractor/imp.py extractor-pack/tools/ +cp extractor/LICENSE-PSF.md extractor-pack/tools/ +cp extractor/python3src.zip extractor-pack/tools/ +cp -r extractor/data extractor-pack/tools/ + +# Shell tool scripts (autobuild, pre-finalize, lgtm-scripts) +cp tools/autobuild.sh extractor-pack/tools/ +cp tools/autobuild.cmd extractor-pack/tools/ +cp tools/pre-finalize.sh extractor-pack/tools/ +cp tools/pre-finalize.cmd extractor-pack/tools/ +cp -r tools/lgtm-scripts extractor-pack/tools/ + +# Downgrades +cp -r downgrades extractor-pack/ + +# Platform-specific Rust binary +cp "${tsg_bin}" extractor-pack/tools/${platform}/tsg-python diff --git a/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll b/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll index 60fe40e716d..155fb4b7c78 100644 --- a/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll +++ b/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll @@ -10,6 +10,10 @@ * `type, path, kind` * - Summaries: * `type, path, input, output, kind` + * - Barriers: + * `type, path, kind` + * - BarrierGuards: + * `type, path, acceptingValue, kind` * - Types: * `type1, type2, path` * @@ -42,7 +46,8 @@ * 3. The `input` and `output` columns specify how data enters and leaves the element selected by the * first `(type, path)` tuple. Both strings are `.`-separated access paths * of the same syntax as the `path` column. - * 4. The `kind` column is a tag that can be referenced from QL to determine to + * 4. The `acceptingValue` column of barrier guard models specifies which branch of the guard is blocking flow. It can be "true" or "false". + * 5. The `kind` column is a tag that can be referenced from QL to determine to * which classes the interpreted elements should be added. For example, for * sources `"remote"` indicates a default remote flow source, and for summaries * `"taint"` indicates a default additional taint step and `"value"` indicates a @@ -355,11 +360,11 @@ private predicate barrierModel(string type, string path, string kind, string mod /** Holds if a barrier guard model exists for the given parameters. */ private predicate barrierGuardModel( - string type, string path, string branch, string kind, string model + string type, string path, string acceptingValue, string kind, string model ) { // No deprecation adapter for barrier models, they were not around back then. exists(QlBuiltins::ExtensionId madId | - Extensions::barrierGuardModel(type, path, branch, kind, madId) and + Extensions::barrierGuardModel(type, path, acceptingValue, kind, madId) and model = "MaD:" + madId.toString() ) } @@ -783,16 +788,16 @@ module ModelOutput { } /** - * Holds if a barrier model contributed `barrier` with the given `kind` for the given `branch`. + * Holds if a barrier model contributed `barrier` with the given `kind` for the given `acceptingValue`. */ cached - API::Node getABarrierGuardNode(string kind, boolean branch, string model) { - exists(string type, string path, string branch_str | - branch = true and branch_str = "true" + API::Node getABarrierGuardNode(string kind, boolean acceptingValue, string model) { + exists(string type, string path, string acceptingValue_str | + acceptingValue = true and acceptingValue_str = "true" or - branch = false and branch_str = "false" + acceptingValue = false and acceptingValue_str = "false" | - barrierGuardModel(type, path, branch_str, kind, model) and + barrierGuardModel(type, path, acceptingValue_str, kind, model) and result = getNodeFromPath(type, path) ) } @@ -856,12 +861,12 @@ module ModelOutput { API::Node getABarrierNode(string kind) { result = getABarrierNode(kind, _) } /** - * Holds if an external model contributed `barrier-guard` with the given `kind` and `branch`. + * Holds if an external model contributed `barrier-guard` with the given `kind` and `acceptingValue`. * * INTERNAL: Do not use. */ - API::Node getABarrierGuardNode(string kind, boolean branch) { - result = getABarrierGuardNode(kind, branch, _) + API::Node getABarrierGuardNode(string kind, boolean acceptingValue) { + result = getABarrierGuardNode(kind, acceptingValue, _) } /** diff --git a/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModelsExtensions.qll b/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModelsExtensions.qll index 2a644aabb95..8d8a4f5fd88 100644 --- a/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModelsExtensions.qll +++ b/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModelsExtensions.qll @@ -33,11 +33,11 @@ extensible predicate barrierModel( * of the given `kind` and `madId` is the data extension row number. * `path` is assumed to lead to a parameter of a call (possibly `self`), and * the call is guarding the parameter. - * `branch` is either `true` or `false`, indicating which branch of the guard - * is protecting the parameter. + * `acceptingValue` is either `true` or `false`, indicating which branch of + * the guard is protecting the parameter. */ extensible predicate barrierGuardModel( - string type, string path, string branch, string kind, QlBuiltins::ExtensionId madId + string type, string path, string acceptingValue, string kind, QlBuiltins::ExtensionId madId ); /** diff --git a/rust/ql/lib/codeql/files/FileSystem.qll b/rust/ql/lib/codeql/files/FileSystem.qll index cfab33b9a44..93ff7be604e 100644 --- a/rust/ql/lib/codeql/files/FileSystem.qll +++ b/rust/ql/lib/codeql/files/FileSystem.qll @@ -45,13 +45,16 @@ extensible predicate additionalExternalFile(string relativePath); /** A file. */ class File extends Container, Impl::File { + pragma[nomagic] + private predicate isAdditionalExternalFile() { additionalExternalFile(this.getRelativePath()) } + /** * Holds if this file was extracted from the source code of the target project * (rather than another location such as inside a dependency). */ predicate fromSource() { exists(ExtractorStep s | s.getAction() = "Extract" and s.getFile() = this) and - not additionalExternalFile(this.getRelativePath()) + not this.isAdditionalExternalFile() } /** diff --git a/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll b/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll index 27773758fc4..7c1fdd8cf78 100644 --- a/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll +++ b/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll @@ -1183,12 +1183,12 @@ private module Cached { exists( FlowSummaryImpl::Public::BarrierGuardElement b, FlowSummaryImpl::Private::SummaryComponentStack stack, - FlowSummaryImpl::Public::AcceptingValue acceptingvalue, string kind, string model + FlowSummaryImpl::Public::AcceptingValue acceptingValue, string kind, string model | - FlowSummaryImpl::Private::barrierGuardSpec(b, stack, acceptingvalue, kind, model) and + FlowSummaryImpl::Private::barrierGuardSpec(b, stack, acceptingValue, kind, model) and e = FlowSummaryImpl::StepsInput::getSinkNode(b, stack.headOfSingleton()).asExpr() and kmp = TMkPair(kind, model) and - gv = convertAcceptingValue(acceptingvalue) and + gv = convertAcceptingValue(acceptingValue) and g = b.getCall() ) } diff --git a/rust/ql/lib/codeql/rust/dataflow/internal/ModelsAsData.qll b/rust/ql/lib/codeql/rust/dataflow/internal/ModelsAsData.qll index 4d28dd8de81..2b3ecf51fe4 100644 --- a/rust/ql/lib/codeql/rust/dataflow/internal/ModelsAsData.qll +++ b/rust/ql/lib/codeql/rust/dataflow/internal/ModelsAsData.qll @@ -9,6 +9,13 @@ * `path; input; kind; provenance` * - Summaries: * `path; input; output; kind; provenance` + * - Barriers: + * `path; output; kind; provenance` + * - BarrierGuards: + * `path; input; acceptingValue; kind; provenance` + * - Neutrals: + * `path; kind; provenance` + * A neutral is used to indicate that a callable is neutral with respect to flow (no summary), source (is not a source) or sink (is not a sink). * * The interpretation of a row is similar to API-graphs with a left-to-right * reading. @@ -34,12 +41,15 @@ * - `Field[i]`: the `i`th element of a tuple. * - `Reference`: the referenced value. * - `Future`: the value being computed asynchronously. - * 3. The `kind` column is a tag that can be referenced from QL to determine to + * 3. The `acceptingValue` column of barrier guard models specifies which branch of the + * guard is blocking flow. It can be "true" or "false". In the future + * "no-exception", "not-zero", "null", "not-null" may be supported. + * 4. The `kind` column is a tag that can be referenced from QL to determine to * which classes the interpreted elements should be added. For example, for * sources `"remote"` indicates a default remote flow source, and for summaries * `"taint"` indicates a default additional taint step and `"value"` indicates a * globally applicable value-preserving step. - * 4. The `provenance` column is mainly used internally, and should be set to `"manual"` for + * 5. The `provenance` column is mainly used internally, and should be set to `"manual"` for * all custom models. */ @@ -114,11 +124,12 @@ extensible predicate barrierModel( * extension row number. * * The value referred to by `input` is assumed to lead to an argument of a call - * (possibly `self`), and the call is guarding the argument. `branch` is either `true` - * or `false`, indicating which branch of the guard is protecting the argument. + * (possibly `self`), and the call is guarding the argument. + * `acceptingValue` is either `true` or `false`, indicating which branch of + * the guard is protecting the parameter. */ extensible predicate barrierGuardModel( - string path, string input, string branch, string kind, string provenance, + string path, string input, string acceptingValue, string kind, string provenance, QlBuiltins::ExtensionId madId ); @@ -153,9 +164,9 @@ predicate interpretModelForTest(QlBuiltins::ExtensionId madId, string model) { model = "Barrier: " + path + "; " + output + "; " + kind ) or - exists(string path, string input, string branch, string kind | - barrierGuardModel(path, input, branch, kind, _, madId) and - model = "Barrier guard: " + path + "; " + input + "; " + branch + "; " + kind + exists(string path, string input, string acceptingValue, string kind | + barrierGuardModel(path, input, acceptingValue, kind, _, madId) and + model = "Barrier guard: " + path + "; " + input + "; " + acceptingValue + "; " + kind ) } @@ -265,10 +276,10 @@ private class FlowBarrierGuardFromModel extends FlowBarrierGuard::Range { } override predicate isBarrierGuard( - string input, string branch, string kind, Provenance provenance, string model + string input, string acceptingValue, string kind, Provenance provenance, string model ) { exists(QlBuiltins::ExtensionId madId | - barrierGuardModel(path, input, branch, kind, provenance, madId) and + barrierGuardModel(path, input, acceptingValue, kind, provenance, madId) and model = "MaD:" + madId.toString() ) } diff --git a/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll b/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll index 8b25c54bfa0..ce980724778 100644 --- a/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll +++ b/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll @@ -388,11 +388,11 @@ module Make< /** * Holds if this element is a flow barrier guard of kind `kind`, for data - * flowing in as described by `input`, when `this` evaluates to `branch`. + * flowing in as described by `input`, when `this` evaluates to `acceptingValue`. */ pragma[nomagic] abstract predicate isBarrierGuard( - string input, string branch, string kind, Provenance provenance, string model + string input, string acceptingValue, string kind, Provenance provenance, string model ); } @@ -764,10 +764,10 @@ module Make< } private predicate isRelevantBarrierGuard( - BarrierGuardElement e, string input, string branch, string kind, Provenance provenance, - string model + BarrierGuardElement e, string input, string acceptingValue, string kind, + Provenance provenance, string model ) { - e.isBarrierGuard(input, branch, kind, provenance, model) and + e.isBarrierGuard(input, acceptingValue, kind, provenance, model) and ( provenance.isManual() or @@ -1588,11 +1588,11 @@ module Make< * Holds if `barrierGuard` is a relevant barrier guard element with input specification `inSpec`. */ predicate barrierGuardSpec( - BarrierGuardElement barrierGuard, SummaryComponentStack inSpec, string branch, string kind, - string model + BarrierGuardElement barrierGuard, SummaryComponentStack inSpec, string acceptingValue, + string kind, string model ) { exists(string input | - isRelevantBarrierGuard(barrierGuard, input, branch, kind, _, model) and + isRelevantBarrierGuard(barrierGuard, input, acceptingValue, kind, _, model) and External::interpretSpec(input, inSpec) ) } @@ -2189,10 +2189,10 @@ module Make< not exists(interpretComponent(c)) } - /** Holds if `acceptingvalue` is not a valid barrier guard accepting-value. */ - bindingset[acceptingvalue] - predicate invalidAcceptingValue(string acceptingvalue) { - not acceptingvalue instanceof AcceptingValue + /** Holds if `acceptingValue` is not a valid barrier guard accepting-value. */ + bindingset[acceptingValue] + predicate invalidAcceptingValue(string acceptingValue) { + not acceptingValue instanceof AcceptingValue } /** Holds if `provenance` is not a valid provenance value. */ @@ -2242,10 +2242,10 @@ module Make< /** * Holds if an external barrier guard specification exists for `n` with input - * specification `input`, accepting value `acceptingvalue`, and kind `kind`. + * specification `input`, accepting value `acceptingValue`, and kind `kind`. */ predicate barrierGuardElement( - Element n, string input, AcceptingValue acceptingvalue, string kind, + Element n, string input, AcceptingValue acceptingValue, string kind, Provenance provenance, string model ); @@ -2371,11 +2371,11 @@ module Make< } private predicate barrierGuardElementRef( - InterpretNode ref, SourceSinkAccessPath input, AcceptingValue acceptingvalue, string kind, + InterpretNode ref, SourceSinkAccessPath input, AcceptingValue acceptingValue, string kind, string model ) { exists(SourceOrSinkElement e | - barrierGuardElement(e, input, acceptingvalue, kind, _, model) and + barrierGuardElement(e, input, acceptingValue, kind, _, model) and if inputNeedsReferenceExt(input.getToken(0)) then e = ref.getCallTarget() else e = ref.asElement() @@ -2518,10 +2518,10 @@ module Make< * given kind in a MaD flow model. */ predicate isBarrierGuardNode( - InterpretNode node, AcceptingValue acceptingvalue, string kind, string model + InterpretNode node, AcceptingValue acceptingValue, string kind, string model ) { exists(InterpretNode ref, SourceSinkAccessPath input | - barrierGuardElementRef(ref, input, acceptingvalue, kind, model) and + barrierGuardElementRef(ref, input, acceptingValue, kind, model) and interpretInput(input, input.getNumToken(), ref, node) ) } diff --git a/shared/mad/codeql/mad/static/ModelsAsData.qll b/shared/mad/codeql/mad/static/ModelsAsData.qll index 84daaa9b6c8..4b58a23186a 100644 --- a/shared/mad/codeql/mad/static/ModelsAsData.qll +++ b/shared/mad/codeql/mad/static/ModelsAsData.qll @@ -31,7 +31,7 @@ signature module ExtensionsSig { */ predicate barrierGuardModel( string namespace, string type, boolean subtypes, string name, string signature, string ext, - string input, string acceptingvalue, string kind, string provenance, + string input, string acceptingValue, string kind, string provenance, QlBuiltins::ExtensionId madId ); @@ -142,14 +142,14 @@ module ModelsAsData { or exists( string namespace, string type, boolean subtypes, string name, string signature, string ext, - string input, string acceptingvalue, string kind, string provenance + string input, string acceptingValue, string kind, string provenance | Extensions::barrierGuardModel(namespace, type, subtypes, name, signature, ext, input, - acceptingvalue, kind, provenance, madId) + acceptingValue, kind, provenance, madId) | model = "Barrier Guard: " + namespace + "; " + type + "; " + subtypes + "; " + name + "; " + - signature + "; " + ext + "; " + input + "; " + acceptingvalue + "; " + kind + "; " + + signature + "; " + ext + "; " + input + "; " + acceptingValue + "; " + kind + "; " + provenance ) or @@ -241,12 +241,12 @@ module ModelsAsData { /** Holds if a barrier guard model exists for the given parameters. */ predicate barrierGuardModel( string namespace, string type, boolean subtypes, string name, string signature, string ext, - string input, string acceptingvalue, string kind, string provenance, string model + string input, string acceptingValue, string kind, string provenance, string model ) { exists(string namespaceOrGroup, QlBuiltins::ExtensionId madId | namespace = getNamespace(namespaceOrGroup) and Extensions::barrierGuardModel(namespaceOrGroup, type, subtypes, name, signature, ext, input, - acceptingvalue, kind, provenance, madId) and + acceptingValue, kind, provenance, madId) and model = "MaD:" + madId.toString() ) } diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/FlowSummaryImpl.qll b/swift/ql/lib/codeql/swift/dataflow/internal/FlowSummaryImpl.qll index c1ddb7f781f..3a096fe3d57 100644 --- a/swift/ql/lib/codeql/swift/dataflow/internal/FlowSummaryImpl.qll +++ b/swift/ql/lib/codeql/swift/dataflow/internal/FlowSummaryImpl.qll @@ -168,7 +168,7 @@ module SourceSinkInterpretationInput implements } predicate barrierGuardElement( - Element n, string input, Public::AcceptingValue acceptingvalue, string kind, + Element n, string input, Public::AcceptingValue acceptingValue, string kind, Public::Provenance provenance, string model ) { none()