From 416e0df68dd509020096ec2dba0439901d3ced88 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Fri, 29 Aug 2025 10:07:37 +0200 Subject: [PATCH 1/6] Fix violations of ql/field-only-used-in-charpred. --- .../codeql/actions/dataflow/FlowSources.qll | 15 +++---- .../security/ArgumentInjectionQuery.qll | 6 +-- .../security/ArtifactPoisoningQuery.qll | 30 ++++++------- .../codeql/actions/security/ControlChecks.qll | 5 +-- .../actions/security/EnvVarInjectionQuery.qll | 12 +---- .../security/OutputClobberingQuery.qll | 8 +--- .../UseOfUnversionedImmutableAction.qll | 4 +- .../new/internal/semantic/SemanticExpr.qll | 3 +- .../CWE/CWE-457/UninitializedVariables.qll | 33 +++++++------- ...ionOfVariableWithUnnecessarilyWideScope.ql | 13 +++--- csharp/ql/lib/semmle/code/csharp/PrintAst.qll | 8 ++-- .../controlflow/internal/Completion.qll | 4 +- .../lib/semmle/code/csharp/dataflow/SSA.qll | 3 +- csharp/ql/src/Language Abuse/UselessUpcast.ql | 13 +++--- go/ql/lib/semmle/go/StringOps.qll | 7 +-- java/ql/lib/experimental/quantum/JCA.qll | 8 +--- java/ql/lib/semmle/code/java/Conversions.qll | 10 ++--- .../dataflow/internal/BarrierGuards.qll | 45 ++++++++++--------- .../semmle/javascript/frameworks/Babel.qll | 7 +-- .../UnreachableMethodOverloads.ql | 4 +- .../lib/semmle/python/frameworks/Stdlib.qll | 5 +-- .../codeql_ql/style/RedundantCastQuery.qll | 25 ++++++----- .../performance/VarUnusedInDisjunct/Test.qll | 2 + ql/ql/test/queries/style/Misspelling/Test.qll | 2 + .../style/UseInstanceofExtension/Foo.qll | 2 + .../test/queries/style/UseSetLiteral/test.qll | 2 + .../ruby/frameworks/http_clients/Excon.qll | 35 ++++++++------- .../ruby/frameworks/http_clients/NetHttp.qll | 3 +- .../rust/elements/internal/CallImpl.qll | 3 +- rust/ql/lib/codeql/rust/internal/Type.qll | 4 +- .../codeql/quantum/experimental/Model.qll | 4 +- 31 files changed, 151 insertions(+), 174 deletions(-) diff --git a/actions/ql/lib/codeql/actions/dataflow/FlowSources.qll b/actions/ql/lib/codeql/actions/dataflow/FlowSources.qll index df3d513d0050..18cc4322c81b 100644 --- a/actions/ql/lib/codeql/actions/dataflow/FlowSources.qll +++ b/actions/ql/lib/codeql/actions/dataflow/FlowSources.qll @@ -31,14 +31,14 @@ abstract class RemoteFlowSource extends SourceNode { class GitHubCtxSource extends RemoteFlowSource { string flag; string event; - GitHubExpression e; GitHubCtxSource() { - this.asExpr() = e and - // github.head_ref - e.getFieldName() = "head_ref" and - flag = "branch" and - ( + exists(GitHubExpression e | + this.asExpr() = e and + // github.head_ref + e.getFieldName() = "head_ref" and + flag = "branch" + | event = e.getATriggerEvent().getName() and event = "pull_request_target" or @@ -148,7 +148,6 @@ class GhCLICommandSource extends RemoteFlowSource, CommandSource { class GitHubEventPathSource extends RemoteFlowSource, CommandSource { string cmd; string flag; - string access_path; Run run; // Examples @@ -163,7 +162,7 @@ class GitHubEventPathSource extends RemoteFlowSource, CommandSource { run.getScript().getACommand() = cmd and cmd.matches("jq%") and cmd.matches("%GITHUB_EVENT_PATH%") and - exists(string regexp | + exists(string regexp, string access_path | untrustedEventPropertiesDataModel(regexp, flag) and not flag = "json" and access_path = "github.event" + cmd.regexpCapture(".*\\s+([^\\s]+)\\s+.*", 1) and diff --git a/actions/ql/lib/codeql/actions/security/ArgumentInjectionQuery.qll b/actions/ql/lib/codeql/actions/security/ArgumentInjectionQuery.qll index 679b8977cf91..1795e9493cb4 100644 --- a/actions/ql/lib/codeql/actions/security/ArgumentInjectionQuery.qll +++ b/actions/ql/lib/codeql/actions/security/ArgumentInjectionQuery.qll @@ -19,7 +19,6 @@ abstract class ArgumentInjectionSink extends DataFlow::Node { */ class ArgumentInjectionFromEnvVarSink extends ArgumentInjectionSink { string command; - string argument; ArgumentInjectionFromEnvVarSink() { exists(Run run, string var | @@ -28,7 +27,7 @@ class ArgumentInjectionFromEnvVarSink extends ArgumentInjectionSink { exists(run.getInScopeEnvVarExpr(var)) or var = "GITHUB_HEAD_REF" ) and - run.getScript().getAnEnvReachingArgumentInjectionSink(var, command, argument) + run.getScript().getAnEnvReachingArgumentInjectionSink(var, command, _) ) } @@ -44,13 +43,12 @@ class ArgumentInjectionFromEnvVarSink extends ArgumentInjectionSink { */ class ArgumentInjectionFromCommandSink extends ArgumentInjectionSink { string command; - string argument; ArgumentInjectionFromCommandSink() { exists(CommandSource source, Run run | run = source.getEnclosingRun() and this.asExpr() = run.getScript() and - run.getScript().getACmdReachingArgumentInjectionSink(source.getCommand(), command, argument) + run.getScript().getACmdReachingArgumentInjectionSink(source.getCommand(), command, _) ) } diff --git a/actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll b/actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll index 76025a9ba0db..4b937c5bacb0 100644 --- a/actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll +++ b/actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll @@ -125,8 +125,6 @@ class LegitLabsDownloadArtifactActionStep extends UntrustedArtifactDownloadStep, } class ActionsGitHubScriptDownloadStep extends UntrustedArtifactDownloadStep, UsesStep { - string script; - ActionsGitHubScriptDownloadStep() { // eg: // - uses: actions/github-script@v6 @@ -149,12 +147,14 @@ class ActionsGitHubScriptDownloadStep extends UntrustedArtifactDownloadStep, Use // var fs = require('fs'); // fs.writeFileSync('${{github.workspace}}/test-results.zip', Buffer.from(download.data)); this.getCallee() = "actions/github-script" and - this.getArgument("script") = script and - script.matches("%listWorkflowRunArtifacts(%") and - script.matches("%downloadArtifact(%") and - script.matches("%writeFileSync(%") and - // Filter out artifacts that were created by pull-request. - not script.matches("%exclude_pull_requests: true%") + exists(string script | + this.getArgument("script") = script and + script.matches("%listWorkflowRunArtifacts(%") and + script.matches("%downloadArtifact(%") and + script.matches("%writeFileSync(%") and + // Filter out artifacts that were created by pull-request. + not script.matches("%exclude_pull_requests: true%") + ) } override string getPath() { @@ -259,15 +259,15 @@ class DirectArtifactDownloadStep extends UntrustedArtifactDownloadStep, Run { class ArtifactPoisoningSink extends DataFlow::Node { UntrustedArtifactDownloadStep download; - PoisonableStep poisonable; ArtifactPoisoningSink() { - download.getAFollowingStep() = poisonable and - // excluding artifacts downloaded to the temporary directory - not download.getPath().regexpMatch("^/tmp.*") and - not download.getPath().regexpMatch("^\\$\\{\\{\\s*runner\\.temp\\s*}}.*") and - not download.getPath().regexpMatch("^\\$RUNNER_TEMP.*") and - ( + exists(PoisonableStep poisonable | + download.getAFollowingStep() = poisonable and + // excluding artifacts downloaded to the temporary directory + not download.getPath().regexpMatch("^/tmp.*") and + not download.getPath().regexpMatch("^\\$\\{\\{\\s*runner\\.temp\\s*}}.*") and + not download.getPath().regexpMatch("^\\$RUNNER_TEMP.*") + | poisonable.(Run).getScript() = this.asExpr() and ( // Check if the poisonable step is a local script execution step diff --git a/actions/ql/lib/codeql/actions/security/ControlChecks.qll b/actions/ql/lib/codeql/actions/security/ControlChecks.qll index 244c04310d6d..41f512abbc34 100644 --- a/actions/ql/lib/codeql/actions/security/ControlChecks.qll +++ b/actions/ql/lib/codeql/actions/security/ControlChecks.qll @@ -159,11 +159,8 @@ abstract class CommentVsHeadDateCheck extends ControlCheck { /* Specific implementations of control checks */ class LabelIfCheck extends LabelCheck instanceof If { - string condition; - LabelIfCheck() { - condition = normalizeExpr(this.getCondition()) and - ( + exists(string condition | condition = normalizeExpr(this.getCondition()) | // eg: contains(github.event.pull_request.labels.*.name, 'safe to test') condition.regexpMatch(".*(^|[^!])contains\\(\\s*github\\.event\\.pull_request\\.labels\\b.*") or diff --git a/actions/ql/lib/codeql/actions/security/EnvVarInjectionQuery.qll b/actions/ql/lib/codeql/actions/security/EnvVarInjectionQuery.qll index 2022e3dca998..ea8a800ef3f6 100644 --- a/actions/ql/lib/codeql/actions/security/EnvVarInjectionQuery.qll +++ b/actions/ql/lib/codeql/actions/security/EnvVarInjectionQuery.qll @@ -55,12 +55,8 @@ class EnvVarInjectionFromFileReadSink extends EnvVarInjectionSink { * echo "COMMIT_MESSAGE=${COMMIT_MESSAGE}" >> $GITHUB_ENV */ class EnvVarInjectionFromCommandSink extends EnvVarInjectionSink { - CommandSource inCommand; - string injectedVar; - string command; - EnvVarInjectionFromCommandSink() { - exists(Run run | + exists(Run run, CommandSource inCommand, string injectedVar, string command | this.asExpr() = inCommand.getEnclosingRun().getScript() and run = inCommand.getEnclosingRun() and run.getScript().getACmdReachingGitHubEnvWrite(inCommand.getCommand(), injectedVar) and @@ -86,12 +82,8 @@ class EnvVarInjectionFromCommandSink extends EnvVarInjectionSink { * echo "FOO=$BODY" >> $GITHUB_ENV */ class EnvVarInjectionFromEnvVarSink extends EnvVarInjectionSink { - string inVar; - string injectedVar; - string command; - EnvVarInjectionFromEnvVarSink() { - exists(Run run | + exists(Run run, string inVar, string injectedVar, string command | run.getScript() = this.asExpr() and exists(run.getInScopeEnvVarExpr(inVar)) and run.getScript().getAnEnvReachingGitHubEnvWrite(inVar, injectedVar) and diff --git a/actions/ql/lib/codeql/actions/security/OutputClobberingQuery.qll b/actions/ql/lib/codeql/actions/security/OutputClobberingQuery.qll index c67d2876b091..4454a5496a2f 100644 --- a/actions/ql/lib/codeql/actions/security/OutputClobberingQuery.qll +++ b/actions/ql/lib/codeql/actions/security/OutputClobberingQuery.qll @@ -99,18 +99,14 @@ class OutputClobberingFromEnvVarSink extends OutputClobberingSink { * echo $BODY */ class WorkflowCommandClobberingFromEnvVarSink extends OutputClobberingSink { - string clobbering_var; - string clobbered_value; - WorkflowCommandClobberingFromEnvVarSink() { - exists(Run run, string workflow_cmd_stmt, string clobbering_stmt | + exists(Run run, string workflow_cmd_stmt, string clobbering_stmt, string clobbering_var | run.getScript() = this.asExpr() and run.getScript().getAStmt() = clobbering_stmt and clobbering_stmt.regexpMatch("echo\\s+(-e\\s+)?(\"|')?\\$(\\{)?" + clobbering_var + ".*") and exists(run.getInScopeEnvVarExpr(clobbering_var)) and run.getScript().getAStmt() = workflow_cmd_stmt and - clobbered_value = - trimQuotes(workflow_cmd_stmt.regexpCapture(".*::set-output\\s+name=.*::(.*)", 1)) + exists(trimQuotes(workflow_cmd_stmt.regexpCapture(".*::set-output\\s+name=.*::(.*)", 1))) ) } } diff --git a/actions/ql/lib/codeql/actions/security/UseOfUnversionedImmutableAction.qll b/actions/ql/lib/codeql/actions/security/UseOfUnversionedImmutableAction.qll index ef258fce2e5c..8595cd1086d6 100644 --- a/actions/ql/lib/codeql/actions/security/UseOfUnversionedImmutableAction.qll +++ b/actions/ql/lib/codeql/actions/security/UseOfUnversionedImmutableAction.qll @@ -1,10 +1,8 @@ import actions class UnversionedImmutableAction extends UsesStep { - string immutable_action; - UnversionedImmutableAction() { - isImmutableAction(this, immutable_action) and + isImmutableAction(this, _) and not isSemVer(this.getVersion()) } } diff --git a/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticExpr.qll b/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticExpr.qll index 668d9b52659e..f269d0dfff2d 100644 --- a/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticExpr.qll +++ b/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticExpr.qll @@ -307,13 +307,12 @@ class SemStoreExpr extends SemUnaryExpr { } class SemConditionalExpr extends SemKnownExpr { - SemExpr condition; SemExpr trueResult; SemExpr falseResult; SemConditionalExpr() { opcode instanceof Opcode::Conditional and - Specific::conditionalExpr(this, type, condition, trueResult, falseResult) + Specific::conditionalExpr(this, type, any(SemExpr condition), trueResult, falseResult) } final SemExpr getBranchExpr(boolean branch) { diff --git a/cpp/ql/src/Security/CWE/CWE-457/UninitializedVariables.qll b/cpp/ql/src/Security/CWE/CWE-457/UninitializedVariables.qll index 14eec81ff58f..804d6a48754c 100644 --- a/cpp/ql/src/Security/CWE/CWE-457/UninitializedVariables.qll +++ b/cpp/ql/src/Security/CWE/CWE-457/UninitializedVariables.qll @@ -31,27 +31,28 @@ private predicate hasConditionalInitialization( class ConditionallyInitializedVariable extends LocalVariable { ConditionalInitializationCall call; ConditionalInitializationFunction f; - VariableAccess initAccess; Evidence e; ConditionallyInitializedVariable() { // Find a call that conditionally initializes this variable - hasConditionalInitialization(f, call, this, initAccess, e) and - // Ignore cases where the variable is assigned prior to the call - not reaches(this.getAnAssignedValue(), initAccess) and - // Ignore cases where the variable is assigned field-wise prior to the call. - not exists(FieldAccess fa | - exists(Assignment a | - fa = getAFieldAccess(this) and - a.getLValue() = fa + exists(VariableAccess initAccess | + hasConditionalInitialization(f, call, this, initAccess, e) and + // Ignore cases where the variable is assigned prior to the call + not reaches(this.getAnAssignedValue(), initAccess) and + // Ignore cases where the variable is assigned field-wise prior to the call. + not exists(FieldAccess fa | + exists(Assignment a | + fa = getAFieldAccess(this) and + a.getLValue() = fa + ) + | + reaches(fa, initAccess) + ) and + // Ignore cases where the variable is assigned by a prior call to an initialization function + not exists(Call c | + this.getAnAccess() = getAnInitializedArgument(c).(AddressOfExpr).getOperand() and + reaches(c, initAccess) ) - | - reaches(fa, initAccess) - ) and - // Ignore cases where the variable is assigned by a prior call to an initialization function - not exists(Call c | - this.getAnAccess() = getAnInitializedArgument(c).(AddressOfExpr).getOperand() and - reaches(c, initAccess) ) and /* * Static local variables with constant initializers do not have the initializer expr as part of diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-1126/DeclarationOfVariableWithUnnecessarilyWideScope.ql b/cpp/ql/src/experimental/Security/CWE/CWE-1126/DeclarationOfVariableWithUnnecessarilyWideScope.ql index 136931f00ec6..bbb219a22da7 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-1126/DeclarationOfVariableWithUnnecessarilyWideScope.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-1126/DeclarationOfVariableWithUnnecessarilyWideScope.ql @@ -19,16 +19,17 @@ import cpp * Errors when using a variable declaration inside a loop. */ class DangerousWhileLoop extends WhileStmt { - Expr exp; Declaration dl; DangerousWhileLoop() { this = dl.getParentScope().(BlockStmt).getParent*() and - exp = this.getCondition().getAChild*() and - not exp instanceof PointerFieldAccess and - not exp instanceof ValueFieldAccess and - exp.(VariableAccess).getTarget().getName() = dl.getName() and - not exp.getParent*() instanceof FunctionCall + exists(Expr exp | + exp = this.getCondition().getAChild*() and + not exp instanceof PointerFieldAccess and + not exp instanceof ValueFieldAccess and + exp.(VariableAccess).getTarget().getName() = dl.getName() and + not exp.getParent*() instanceof FunctionCall + ) } Declaration getDeclaration() { result = dl } diff --git a/csharp/ql/lib/semmle/code/csharp/PrintAst.qll b/csharp/ql/lib/semmle/code/csharp/PrintAst.qll index fd4bf1cb86b0..1c09cc65f99d 100644 --- a/csharp/ql/lib/semmle/code/csharp/PrintAst.qll +++ b/csharp/ql/lib/semmle/code/csharp/PrintAst.qll @@ -523,11 +523,11 @@ final class AttributeNode extends ElementNode { * A node representing a `TypeParameter`. */ final class TypeParameterNode extends ElementNode { - TypeParameter typeParameter; - TypeParameterNode() { - typeParameter = element and - not isNotNeeded(typeParameter.getDeclaringGeneric()) + exists(TypeParameter typeParameter | + typeParameter = element and + not isNotNeeded(typeParameter.getDeclaringGeneric()) + ) } override ElementNode getChild(int childIndex) { none() } diff --git a/csharp/ql/lib/semmle/code/csharp/controlflow/internal/Completion.qll b/csharp/ql/lib/semmle/code/csharp/controlflow/internal/Completion.qll index 6fed45cdf84d..a3bf97945964 100644 --- a/csharp/ql/lib/semmle/code/csharp/controlflow/internal/Completion.qll +++ b/csharp/ql/lib/semmle/code/csharp/controlflow/internal/Completion.qll @@ -310,10 +310,8 @@ private class Overflowable extends UnaryOperation { /** A control flow element that is inside a `try` block. */ private class TriedControlFlowElement extends ControlFlowElement { - TryStmt try; - TriedControlFlowElement() { - this = try.getATriedElement() and + this = any(TryStmt try).getATriedElement() and not this instanceof NonReturningCall } diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/SSA.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/SSA.qll index f17317af83be..4c9f64de4b9e 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/SSA.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/SSA.qll @@ -451,10 +451,9 @@ module Ssa { * An SSA definition that corresponds to an explicit assignable definition. */ class ExplicitDefinition extends Definition, SsaImpl::WriteDefinition { - SourceVariable sv; AssignableDefinition ad; - ExplicitDefinition() { SsaImpl::explicitDefinition(this, sv, ad) } + ExplicitDefinition() { SsaImpl::explicitDefinition(this, _, ad) } /** * Gets an underlying assignable definition. The result is always unique, diff --git a/csharp/ql/src/Language Abuse/UselessUpcast.ql b/csharp/ql/src/Language Abuse/UselessUpcast.ql index 827d16038b21..a06dc60cc7ab 100644 --- a/csharp/ql/src/Language Abuse/UselessUpcast.ql +++ b/csharp/ql/src/Language Abuse/UselessUpcast.ql @@ -75,15 +75,16 @@ private class ConstructorCall extends Call { /** An explicit upcast. */ class ExplicitUpcast extends ExplicitCast { - ValueOrRefType src; ValueOrRefType dest; ExplicitUpcast() { - src = this.getSourceType() and - dest = this.getTargetType() and - (src instanceof RefType or src instanceof Struct) and - src.isImplicitlyConvertibleTo(dest) and - src != dest // Handled by `cs/useless-cast-to-self` + exists(ValueOrRefType src | + src = this.getSourceType() and + dest = this.getTargetType() and + (src instanceof RefType or src instanceof Struct) and + src.isImplicitlyConvertibleTo(dest) and + src != dest // Handled by `cs/useless-cast-to-self` + ) } pragma[nomagic] diff --git a/go/ql/lib/semmle/go/StringOps.qll b/go/ql/lib/semmle/go/StringOps.qll index 351617abf9de..3463e4fdf878 100644 --- a/go/ql/lib/semmle/go/StringOps.qll +++ b/go/ql/lib/semmle/go/StringOps.qll @@ -306,11 +306,12 @@ module StringOps { */ class StringFormatCall extends DataFlow::CallNode { string fmt; - Range f; StringFormatCall() { - this = f.getACall() and - fmt = this.getArgument(f.getFormatStringIndex()).getStringValue() and + exists(Range f | + this = f.getACall() and + fmt = this.getArgument(f.getFormatStringIndex()).getStringValue() + ) and fmt.regexpMatch(getFormatComponentRegex() + "*") } diff --git a/java/ql/lib/experimental/quantum/JCA.qll b/java/ql/lib/experimental/quantum/JCA.qll index dc86c4637505..9e386279b39f 100644 --- a/java/ql/lib/experimental/quantum/JCA.qll +++ b/java/ql/lib/experimental/quantum/JCA.qll @@ -404,9 +404,7 @@ module JCAModel { * For example, in `Cipher.getInstance(algorithm)`, this class represents `algorithm`. */ class CipherGetInstanceAlgorithmArg extends CipherAlgorithmValueConsumer instanceof Expr { - CipherGetInstanceCall call; - - CipherGetInstanceAlgorithmArg() { this = call.getAlgorithmArg() } + CipherGetInstanceAlgorithmArg() { this = any(CipherGetInstanceCall call).getAlgorithmArg() } override Crypto::ConsumerInputDataFlowNode getInputNode() { result.asExpr() = this } @@ -1539,11 +1537,9 @@ module JCAModel { } class MacOperationCall extends Crypto::MacOperationInstance instanceof MethodCall { - Expr output; - MacOperationCall() { super.getMethod().getDeclaringType().hasQualifiedName("javax.crypto", "Mac") and - ( + exists(Expr output | super.getMethod().hasStringSignature(["doFinal()", "doFinal(byte[])"]) and this = output or super.getMethod().hasStringSignature("doFinal(byte[], int)") and diff --git a/java/ql/lib/semmle/code/java/Conversions.qll b/java/ql/lib/semmle/code/java/Conversions.qll index 76b74fd1eb7c..779eb7620bec 100644 --- a/java/ql/lib/semmle/code/java/Conversions.qll +++ b/java/ql/lib/semmle/code/java/Conversions.qll @@ -100,12 +100,12 @@ class InvocationConversionContext extends ConversionSite { * See the Java Language Specification, Section 5.4. */ class StringConversionContext extends ConversionSite { - AddExpr a; - StringConversionContext() { - a.getAnOperand() = this and - not this.getType() instanceof TypeString and - a.getAnOperand().getType() instanceof TypeString + exists(AddExpr a | + a.getAnOperand() = this and + not this.getType() instanceof TypeString and + a.getAnOperand().getType() instanceof TypeString + ) } override Type getConversionTarget() { result instanceof TypeString } diff --git a/javascript/ql/lib/semmle/javascript/dataflow/internal/BarrierGuards.qll b/javascript/ql/lib/semmle/javascript/dataflow/internal/BarrierGuards.qll index 0af57ec01869..6dd0ebf0bb1c 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/internal/BarrierGuards.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/internal/BarrierGuards.qll @@ -386,34 +386,35 @@ module MakeStateBarrierGuard< */ private class BarrierGuardFunction extends FinalFunction { DataFlow::ParameterNode sanitizedParameter; - BarrierGuard guard; boolean guardOutcome; FlowState state; int paramIndex; BarrierGuardFunction() { - barrierGuardIsRelevant(guard) and - exists(Expr e | - exists(Expr returnExpr | - returnExpr = guard.asExpr() - or - // ad hoc support for conjunctions: - getALogicalAndParent(guard) = returnExpr and guardOutcome = true - or - // ad hoc support for disjunctions: - getALogicalOrParent(guard) = returnExpr and guardOutcome = false - | - exists(SsaExplicitDefinition ssa | - ssa.getDef().getSource() = returnExpr and - ssa.getVariable().getAUse() = this.getAReturnedExpr() - ) - or - returnExpr = this.getAReturnedExpr() + exists(BarrierGuard guard | + barrierGuardIsRelevant(guard) and + exists(Expr e | + exists(Expr returnExpr | + returnExpr = guard.asExpr() + or + // ad hoc support for conjunctions: + getALogicalAndParent(guard) = returnExpr and guardOutcome = true + or + // ad hoc support for disjunctions: + getALogicalOrParent(guard) = returnExpr and guardOutcome = false + | + exists(SsaExplicitDefinition ssa | + ssa.getDef().getSource() = returnExpr and + ssa.getVariable().getAUse() = this.getAReturnedExpr() + ) + or + returnExpr = this.getAReturnedExpr() + ) and + sanitizedParameter.flowsToExpr(e) and + barrierGuardBlocksExpr(guard, guardOutcome, e, state) ) and - sanitizedParameter.flowsToExpr(e) and - barrierGuardBlocksExpr(guard, guardOutcome, e, state) - ) and - sanitizedParameter.getParameter() = this.getParameter(paramIndex) + sanitizedParameter.getParameter() = this.getParameter(paramIndex) + ) } /** diff --git a/javascript/ql/lib/semmle/javascript/frameworks/Babel.qll b/javascript/ql/lib/semmle/javascript/frameworks/Babel.qll index 4b3f70b77725..9ca47fb47276 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/Babel.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/Babel.qll @@ -141,16 +141,17 @@ module Babel { */ deprecated private class BabelRootTransformedPathExpr extends PathExpr, Expr { RootImportConfig plugin; - string prefix; string mappedPrefix; string suffix; BabelRootTransformedPathExpr() { this instanceof PathExpr and plugin.appliesTo(this.getTopLevel()) and - prefix = this.getStringValue().regexpCapture("(.)/(.*)", 1) and suffix = this.getStringValue().regexpCapture("(.)/(.*)", 2) and - mappedPrefix = plugin.getRoot(prefix) + exists(string prefix | + prefix = this.getStringValue().regexpCapture("(.)/(.*)", 1) and + mappedPrefix = plugin.getRoot(prefix) + ) } /** Gets the configuration that applies to this path. */ diff --git a/javascript/ql/src/Declarations/UnreachableMethodOverloads.ql b/javascript/ql/src/Declarations/UnreachableMethodOverloads.ql index 912d58ab54ca..0088af1a2c0c 100644 --- a/javascript/ql/src/Declarations/UnreachableMethodOverloads.ql +++ b/javascript/ql/src/Declarations/UnreachableMethodOverloads.ql @@ -46,9 +46,7 @@ string getKind(MemberDeclaration m) { * A call-signature that originates from a MethodSignature in the AST. */ private class MethodCallSig extends Function { - private MethodSignature signature; - - MethodCallSig() { this = signature.getBody() } + MethodCallSig() { this = any(MethodSignature signature).getBody() } int getNumOptionalParameter() { result = count(Parameter p | p = this.getParameter(_) and p.isDeclaredOptional()) diff --git a/python/ql/lib/semmle/python/frameworks/Stdlib.qll b/python/ql/lib/semmle/python/frameworks/Stdlib.qll index ceb2f1952a02..c6b671e8b781 100644 --- a/python/ql/lib/semmle/python/frameworks/Stdlib.qll +++ b/python/ql/lib/semmle/python/frameworks/Stdlib.qll @@ -3702,11 +3702,8 @@ module StdlibPrivate { * A call to a find method on a tree or an element will execute an XPath expression. */ private class ElementTreeFindCall extends XML::XPathExecution::Range, DataFlow::CallCfgNode { - string methodName; - ElementTreeFindCall() { - methodName in ["find", "findall", "findtext"] and - ( + exists(string methodName | methodName in ["find", "findall", "findtext"] | this = elementTreeInstance().getMember(methodName).getACall() or this = elementInstance().getMember(methodName).getACall() diff --git a/ql/ql/src/codeql_ql/style/RedundantCastQuery.qll b/ql/ql/src/codeql_ql/style/RedundantCastQuery.qll index 9549bb85844b..58dee22665f7 100644 --- a/ql/ql/src/codeql_ql/style/RedundantCastQuery.qll +++ b/ql/ql/src/codeql_ql/style/RedundantCastQuery.qll @@ -1,11 +1,13 @@ import ql class RedundantInlineCast extends AstNode instanceof InlineCast { - Type t; - + // Type t; RedundantInlineCast() { - t = unique( | | super.getType()) and - ( + exists(Type t | + t = unique( | | super.getType()) and + // noopt can require explicit casts + not this.getEnclosingPredicate().getAnAnnotation() instanceof NoOpt + | // The cast is to the type the base expression already has t = unique( | | super.getBase().getType()) or @@ -23,9 +25,7 @@ class RedundantInlineCast extends AstNode instanceof InlineCast { target = unique( | | call.getTarget()) and t = unique( | | target.getParameterType(i)) ) - ) and - // noopt can require explicit casts - not this.getEnclosingPredicate().getAnAnnotation() instanceof NoOpt + ) } TypeExpr getTypeExpr() { result = super.getTypeExpr() } @@ -49,15 +49,16 @@ private class AnyCast extends AstNode instanceof FullAggregate { // `foo = any(Bar b)` is effectively a cast to `Bar`. class RedundantAnyCast extends AstNode instanceof ComparisonFormula { AnyCast cast; - Expr operand; RedundantAnyCast() { super.getOperator() = "=" and super.getAnOperand() = cast and - super.getAnOperand() = operand and - cast != operand and - unique( | | operand.getType()).getASuperType*() = - unique( | | cast.getTypeExpr().getResolvedType()) and + exists(Expr operand | + super.getAnOperand() = operand and + cast != operand and + unique( | | operand.getType()).getASuperType*() = + unique( | | cast.getTypeExpr().getResolvedType()) + ) and not this.getEnclosingPredicate().getAnAnnotation() instanceof NoOpt } diff --git a/ql/ql/test/queries/performance/VarUnusedInDisjunct/Test.qll b/ql/ql/test/queries/performance/VarUnusedInDisjunct/Test.qll index 520acbaa1b3a..10e97e582096 100644 --- a/ql/ql/test/queries/performance/VarUnusedInDisjunct/Test.qll +++ b/ql/ql/test/queries/performance/VarUnusedInDisjunct/Test.qll @@ -165,4 +165,6 @@ class HasField extends Big { or this.toString().matches("%foo") // <- field only defined here. } + + Big getField() { result = field } } diff --git a/ql/ql/test/queries/style/Misspelling/Test.qll b/ql/ql/test/queries/style/Misspelling/Test.qll index f49f4633c6b8..b6619145f8d5 100644 --- a/ql/ql/test/queries/style/Misspelling/Test.qll +++ b/ql/ql/test/queries/style/Misspelling/Test.qll @@ -8,6 +8,8 @@ class PublicallyAccessible extends string { // should be argument predicate hasAgrument() { none() } + + int getNum() { result = numOccurences } } /** diff --git a/ql/ql/test/queries/style/UseInstanceofExtension/Foo.qll b/ql/ql/test/queries/style/UseInstanceofExtension/Foo.qll index 3429ce5b5994..b58cb3f93e37 100644 --- a/ql/ql/test/queries/style/UseInstanceofExtension/Foo.qll +++ b/ql/ql/test/queries/style/UseInstanceofExtension/Foo.qll @@ -22,6 +22,8 @@ class Inst3 extends string { Range range; Inst3() { this = range } + + Range getRange() { result = range } } class Inst4 extends string { diff --git a/ql/ql/test/queries/style/UseSetLiteral/test.qll b/ql/ql/test/queries/style/UseSetLiteral/test.qll index 36a5f938f895..fcc581c3e8cd 100644 --- a/ql/ql/test/queries/style/UseSetLiteral/test.qll +++ b/ql/ql/test/queries/style/UseSetLiteral/test.qll @@ -81,6 +81,8 @@ class MyTest8Class extends int { predicate is(int x) { x = this } int get() { result = this } + + string getS() { result = s } } predicate test9(MyTest8Class c) { diff --git a/ruby/ql/lib/codeql/ruby/frameworks/http_clients/Excon.qll b/ruby/ql/lib/codeql/ruby/frameworks/http_clients/Excon.qll index e2ba1eb48fec..9ffd5e3ef512 100644 --- a/ruby/ql/lib/codeql/ruby/frameworks/http_clients/Excon.qll +++ b/ruby/ql/lib/codeql/ruby/frameworks/http_clients/Excon.qll @@ -25,27 +25,28 @@ private import codeql.ruby.DataFlow */ class ExconHttpRequest extends Http::Client::Request::Range instanceof DataFlow::CallNode { API::Node requestNode; - API::Node connectionNode; DataFlow::Node connectionUse; ExconHttpRequest() { this = requestNode.asSource() and - connectionUse = connectionNode.asSource() and - connectionNode = - [ - // one-off requests - API::getTopLevelMember("Excon"), - // connection re-use - API::getTopLevelMember("Excon").getInstance(), - API::getTopLevelMember("Excon").getMember("Connection").getInstance() - ] and - requestNode = - connectionNode - .getReturn([ - // Excon#request exists but Excon.request doesn't. - // This shouldn't be a problem - in real code the latter would raise NoMethodError anyway. - "get", "head", "delete", "options", "post", "put", "patch", "trace", "request" - ]) + exists(API::Node connectionNode | + connectionUse = connectionNode.asSource() and + connectionNode = + [ + // one-off requests + API::getTopLevelMember("Excon"), + // connection re-use + API::getTopLevelMember("Excon").getInstance(), + API::getTopLevelMember("Excon").getMember("Connection").getInstance() + ] and + requestNode = + connectionNode + .getReturn([ + // Excon#request exists but Excon.request doesn't. + // This shouldn't be a problem - in real code the latter would raise NoMethodError anyway. + "get", "head", "delete", "options", "post", "put", "patch", "trace", "request" + ]) + ) } override DataFlow::Node getResponseBody() { result = requestNode.getAMethodCall("body") } diff --git a/ruby/ql/lib/codeql/ruby/frameworks/http_clients/NetHttp.qll b/ruby/ql/lib/codeql/ruby/frameworks/http_clients/NetHttp.qll index bcb86e2b63d3..a1b58d700b8f 100644 --- a/ruby/ql/lib/codeql/ruby/frameworks/http_clients/NetHttp.qll +++ b/ruby/ql/lib/codeql/ruby/frameworks/http_clients/NetHttp.qll @@ -27,11 +27,10 @@ private import codeql.ruby.DataFlow class NetHttpRequest extends Http::Client::Request::Range instanceof DataFlow::CallNode { private DataFlow::CallNode request; API::Node requestNode; - API::Node connectionNode; private boolean returnsResponseBody; NetHttpRequest() { - exists(string method | + exists(string method, API::Node connectionNode | request = requestNode.asSource() and this = request and requestNode = connectionNode.getReturn(method) diff --git a/rust/ql/lib/codeql/rust/elements/internal/CallImpl.qll b/rust/ql/lib/codeql/rust/elements/internal/CallImpl.qll index ac6e08bb9cf7..020b50594a6d 100644 --- a/rust/ql/lib/codeql/rust/elements/internal/CallImpl.qll +++ b/rust/ql/lib/codeql/rust/elements/internal/CallImpl.qll @@ -123,11 +123,10 @@ module Impl { } class CallExprMethodCall extends Call instanceof CallExpr { - Path qualifier; string methodName; boolean selfIsRef; - CallExprMethodCall() { callIsMethodCall(this, qualifier, methodName, selfIsRef) } + CallExprMethodCall() { callIsMethodCall(this, _, methodName, selfIsRef) } /** * Holds if this call must have an explicit borrow for the `self` argument, diff --git a/rust/ql/lib/codeql/rust/internal/Type.qll b/rust/ql/lib/codeql/rust/internal/Type.qll index 56c179354b40..eaa7e83fc6da 100644 --- a/rust/ql/lib/codeql/rust/internal/Type.qll +++ b/rust/ql/lib/codeql/rust/internal/Type.qll @@ -620,9 +620,7 @@ final class TypeBoundTypeAbstraction extends TypeAbstraction, TypeBound { } final class SelfTypeBoundTypeAbstraction extends TypeAbstraction, Name { - private TraitTypeAbstraction trait; - - SelfTypeBoundTypeAbstraction() { trait.getName() = this } + SelfTypeBoundTypeAbstraction() { any(TraitTypeAbstraction trait).getName() = this } override TypeParameter getATypeParameter() { none() } } diff --git a/shared/quantum/codeql/quantum/experimental/Model.qll b/shared/quantum/codeql/quantum/experimental/Model.qll index d8b1402b5e88..0888b640dbfa 100644 --- a/shared/quantum/codeql/quantum/experimental/Model.qll +++ b/shared/quantum/codeql/quantum/experimental/Model.qll @@ -1841,9 +1841,7 @@ module CryptographyBase Input> { * An SCRYPT key derivation algorithm node. */ class ScryptAlgorithmNode extends KeyDerivationAlgorithmNode { - ScryptAlgorithmInstance scryptInstance; - - ScryptAlgorithmNode() { scryptInstance = instance.asAlg() } + ScryptAlgorithmNode() { instance.asAlg() instanceof ScryptAlgorithmInstance } /** * Gets the iteration count (`N`) argument From 4df95b7762bc530ec30da822bb5a106a5624811c Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Fri, 29 Aug 2025 10:20:26 +0200 Subject: [PATCH 2/6] Update Ql tests expected output. --- ql/ql/test/queries/style/Misspelling/Misspelling.expected | 4 ++-- .../UseInstanceofExtension/UseInstanceofExtension.expected | 2 +- ql/ql/test/queries/style/UseSetLiteral/UseSetLiteral.expected | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ql/ql/test/queries/style/Misspelling/Misspelling.expected b/ql/ql/test/queries/style/Misspelling/Misspelling.expected index 8116e2b5afa7..1c02ca81d621 100644 --- a/ql/ql/test/queries/style/Misspelling/Misspelling.expected +++ b/ql/ql/test/queries/style/Misspelling/Misspelling.expected @@ -2,5 +2,5 @@ | Test.qll:4:7:4:26 | Class PublicallyAccessible | This class name contains the common misspelling 'publically', which should instead be 'publicly'. | | Test.qll:5:3:5:20 | FieldDecl | This field name contains the common misspelling 'occurences', which should instead be 'occurrences'. | | Test.qll:10:13:10:23 | ClassPredicate hasAgrument | This predicate name contains the common misspelling 'agrument', which should instead be 'argument'. | -| Test.qll:13:1:16:3 | QLDoc | This comment contains the non-US spelling 'colour', which should instead be 'color'. | -| Test.qll:17:7:17:17 | Class AnalysedInt | This class name contains the non-US spelling 'analysed', which should instead be 'analyzed'. | +| Test.qll:15:1:18:3 | QLDoc | This comment contains the non-US spelling 'colour', which should instead be 'color'. | +| Test.qll:19:7:19:17 | Class AnalysedInt | This class name contains the non-US spelling 'analysed', which should instead be 'analyzed'. | diff --git a/ql/ql/test/queries/style/UseInstanceofExtension/UseInstanceofExtension.expected b/ql/ql/test/queries/style/UseInstanceofExtension/UseInstanceofExtension.expected index 72f110c632f9..804927fa0327 100644 --- a/ql/ql/test/queries/style/UseInstanceofExtension/UseInstanceofExtension.expected +++ b/ql/ql/test/queries/style/UseInstanceofExtension/UseInstanceofExtension.expected @@ -1,4 +1,4 @@ | Foo.qll:7:7:7:10 | Class Inst | Consider defining this class as non-extending subtype of $@. | Foo.qll:1:7:1:11 | Class Range | Range | | Foo.qll:15:7:15:11 | Class Inst2 | Consider defining this class as non-extending subtype of $@. | Foo.qll:1:7:1:11 | Class Range | Range | | Foo.qll:21:7:21:11 | Class Inst3 | Consider defining this class as non-extending subtype of $@. | Foo.qll:1:7:1:11 | Class Range | Range | -| Foo.qll:27:7:27:11 | Class Inst4 | Consider defining this class as non-extending subtype of $@. | Foo.qll:1:7:1:11 | Class Range | Range | +| Foo.qll:29:7:29:11 | Class Inst4 | Consider defining this class as non-extending subtype of $@. | Foo.qll:1:7:1:11 | Class Range | Range | diff --git a/ql/ql/test/queries/style/UseSetLiteral/UseSetLiteral.expected b/ql/ql/test/queries/style/UseSetLiteral/UseSetLiteral.expected index fac79ff078ed..ed17f5e1f1a7 100644 --- a/ql/ql/test/queries/style/UseSetLiteral/UseSetLiteral.expected +++ b/ql/ql/test/queries/style/UseSetLiteral/UseSetLiteral.expected @@ -4,5 +4,5 @@ | test.qll:62:7:65:14 | Disjunction | This formula of 4 comparisons can be replaced with a single equality on a set literal, improving readability. | | test.qll:68:7:71:13 | Disjunction | This formula of 4 comparisons can be replaced with a single equality on a set literal, improving readability. | | test.qll:74:7:77:13 | Disjunction | This formula of 4 comparisons can be replaced with a single equality on a set literal, improving readability. | -| test.qll:87:3:90:9 | Disjunction | This formula of 4 predicate calls can be replaced with a single call on a set literal, improving readability. | -| test.qll:128:3:134:3 | Disjunction | This formula of 4 comparisons can be replaced with a single equality on a set literal, improving readability. | +| test.qll:89:3:92:9 | Disjunction | This formula of 4 predicate calls can be replaced with a single call on a set literal, improving readability. | +| test.qll:130:3:136:3 | Disjunction | This formula of 4 comparisons can be replaced with a single equality on a set literal, improving readability. | From 55ba842b52bef86ad74ebd638a0273542f5fb861 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Fri, 29 Aug 2025 13:02:53 +0200 Subject: [PATCH 3/6] Fix some violations of ql/could-be-cast. --- .../CWE-829/ArtifactPoisoningPathTraversal.ql | 4 +--- .../CWE/CWE-416/UseAfterExpiredLifetime.ql | 2 +- cpp/ql/src/jsf/4.10 Classes/AV Rule 96.ql | 2 +- csharp/ql/src/Telemetry/DatabaseQuality.qll | 4 ++-- .../quantum/Analysis/ArtifactReuse.qll | 16 +++++++--------- .../Security/CWE-022bis/TarSlipImprov.ql | 2 +- 6 files changed, 13 insertions(+), 17 deletions(-) diff --git a/actions/ql/src/experimental/Security/CWE-829/ArtifactPoisoningPathTraversal.ql b/actions/ql/src/experimental/Security/CWE-829/ArtifactPoisoningPathTraversal.ql index 519437ddb229..517a9d1eaad7 100644 --- a/actions/ql/src/experimental/Security/CWE-829/ArtifactPoisoningPathTraversal.ql +++ b/actions/ql/src/experimental/Security/CWE-829/ArtifactPoisoningPathTraversal.ql @@ -37,8 +37,6 @@ where ) or // upload artifact is not used in the same workflow - not exists(UsesStep upload | - download.getEnclosingWorkflow().getAJob().(LocalJob).getAStep() = upload - ) + not download.getEnclosingWorkflow().getAJob().(LocalJob).getAStep() instanceof UsesStep ) select download, "Potential artifact poisoning" diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-416/UseAfterExpiredLifetime.ql b/cpp/ql/src/experimental/Security/CWE/CWE-416/UseAfterExpiredLifetime.ql index ffcac802b6dc..fec373ce5216 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-416/UseAfterExpiredLifetime.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-416/UseAfterExpiredLifetime.ql @@ -266,7 +266,7 @@ class LifetimePointerType extends LifetimeIndirectionType { class FullExpr extends Expr { FullExpr() { // A full-expression is not a subexpression - not exists(Expr p | this.getParent() = p) + not this.getParent() instanceof Expr or // A sub-expression that is an unevaluated operand this.isUnevaluated() diff --git a/cpp/ql/src/jsf/4.10 Classes/AV Rule 96.ql b/cpp/ql/src/jsf/4.10 Classes/AV Rule 96.ql index 67e01ffc7c07..4697af80c17c 100644 --- a/cpp/ql/src/jsf/4.10 Classes/AV Rule 96.ql +++ b/cpp/ql/src/jsf/4.10 Classes/AV Rule 96.ql @@ -28,7 +28,7 @@ where exists(FunctionCall c, int i, Function f | c.getArgument(i) = e and c.getTarget() = f and - exists(Parameter p | f.getParameter(i) = p) and // varargs + exists(f.getParameter(i)) and // varargs baseElement(e.getType(), cl) and // only interested in arrays with classes not compatible(f.getParameter(i).getUnspecifiedType(), e.getUnspecifiedType()) ) diff --git a/csharp/ql/src/Telemetry/DatabaseQuality.qll b/csharp/ql/src/Telemetry/DatabaseQuality.qll index fa6c70dbc51f..ca2ab3e7e165 100644 --- a/csharp/ql/src/Telemetry/DatabaseQuality.qll +++ b/csharp/ql/src/Telemetry/DatabaseQuality.qll @@ -12,7 +12,7 @@ module CallTargetStats implements StatsSig { private predicate isNoSetterPropertyCallInConstructor(PropertyCall c) { exists(Property p, Constructor ctor | p = c.getProperty() and - not exists(Setter a | a = p.getAnAccessor()) and + not p.getAnAccessor() instanceof Setter and c.getEnclosingCallable() = ctor and ( c.hasThisQualifier() @@ -25,7 +25,7 @@ module CallTargetStats implements StatsSig { private predicate isNoSetterPropertyInitialization(PropertyCall c) { exists(Property p, AssignExpr assign | p = c.getProperty() and - not exists(Setter a | a = p.getAnAccessor()) and + not p.getAnAccessor() instanceof Setter and assign = c.getParent() and assign.getLValue() = c and assign.getParent() instanceof Property diff --git a/java/ql/src/experimental/quantum/Analysis/ArtifactReuse.qll b/java/ql/src/experimental/quantum/Analysis/ArtifactReuse.qll index 88598e615896..bbdbaa290043 100644 --- a/java/ql/src/experimental/quantum/Analysis/ArtifactReuse.qll +++ b/java/ql/src/experimental/quantum/Analysis/ArtifactReuse.qll @@ -10,15 +10,13 @@ import experimental.quantum.Language */ private module WrapperConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { - exists(Call c | - c = source.asExpr() - // not handling references yet, I think we want to flat say references are only ok - // if I know the source, otherwise, it has to be through an additional flow step, which - // we filter as a source, i.e., references are only allowed as sources only, - // no inferrece? Not sure if that would work - //or - // source.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr() = c.getAnArgument() - ) and + source.asExpr() instanceof Call and + // not handling references yet, I think we want to flat say references are only ok + // if I know the source, otherwise, it has to be through an additional flow step, which + // we filter as a source, i.e., references are only allowed as sources only, + // no inferrece? Not sure if that would work + //or + // source.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr() = c.getAnArgument() // Filter out sources that are known additional flow steps, as these are likely not the // kind of wrapper source we are looking for. not exists(AdditionalFlowInputStep s | s.getOutput() = source) diff --git a/python/ql/src/experimental/Security/CWE-022bis/TarSlipImprov.ql b/python/ql/src/experimental/Security/CWE-022bis/TarSlipImprov.ql index 1727da1bcf55..42c0bc170fd9 100755 --- a/python/ql/src/experimental/Security/CWE-022bis/TarSlipImprov.ql +++ b/python/ql/src/experimental/Security/CWE-022bis/TarSlipImprov.ql @@ -63,7 +63,7 @@ private module TarSlipImprovConfig implements DataFlow::ConfigSig { // For a call to `file.extractall` without `members` argument, `file` is considered a sink. exists(MethodCallNode call, AllTarfileOpens atfo | call = atfo.getReturn().getMember("extractall").getACall() and - not exists(Node arg | arg = call.getArgByName("members")) and + not exists(call.getArgByName("members")) and sink = call.getObject() ) or From ff26a6194a2bc1d8b607ace9e7f7b59da1735961 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Fri, 29 Aug 2025 13:12:58 +0200 Subject: [PATCH 4/6] Fix violations of ql/counting-to-zero. --- cpp/ql/lib/semmle/code/cpp/valuenumbering/HashCons.qll | 2 +- .../src/experimental/Likely Bugs/RedundantNullCheckParam.ql | 2 +- java/ql/lib/semmle/code/java/frameworks/javaee/ejb/EJB.qll | 4 ++-- java/ql/src/Likely Bugs/Termination/SpinOnField.ql | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/valuenumbering/HashCons.qll b/cpp/ql/lib/semmle/code/cpp/valuenumbering/HashCons.qll index b74e7c3d741b..b7e4fecb4742 100644 --- a/cpp/ql/lib/semmle/code/cpp/valuenumbering/HashCons.qll +++ b/cpp/ql/lib/semmle/code/cpp/valuenumbering/HashCons.qll @@ -648,7 +648,7 @@ private predicate mk_UuidofOperator(Type t, UuidofOperator e) { } private predicate analyzableTypeidType(TypeidOperator e) { - count(e.getAChild()) = 0 and + not exists(e.getAChild()) and strictcount(e.getResultType()) = 1 } diff --git a/cpp/ql/src/experimental/Likely Bugs/RedundantNullCheckParam.ql b/cpp/ql/src/experimental/Likely Bugs/RedundantNullCheckParam.ql index 36e42cae92a4..ce3497bb9655 100644 --- a/cpp/ql/src/experimental/Likely Bugs/RedundantNullCheckParam.ql +++ b/cpp/ql/src/experimental/Likely Bugs/RedundantNullCheckParam.ql @@ -47,7 +47,7 @@ where // for a function parameter unchecked.getTarget() = param and // this function parameter is not overwritten - count(param.getAnAssignment()) = 0 and + not exists(param.getAnAssignment()) and check.getTarget() = param and // which is once checked candidateResultChecked(check, eqop) and diff --git a/java/ql/lib/semmle/code/java/frameworks/javaee/ejb/EJB.qll b/java/ql/lib/semmle/code/java/frameworks/javaee/ejb/EJB.qll index 2b003b3c94e7..d59976c0c6c8 100644 --- a/java/ql/lib/semmle/code/java/frameworks/javaee/ejb/EJB.qll +++ b/java/ql/lib/semmle/code/java/frameworks/javaee/ejb/EJB.qll @@ -35,14 +35,14 @@ class SessionEjb extends EJB { // Either the EJB does not declare any business interfaces explicitly // and implements a single interface candidate, // which is then considered to be the business interface... - count(this.getAnExplicitBusinessInterface()) = 0 and + not exists(this.getAnExplicitBusinessInterface()) and count(this.getAnImplementedBusinessInterfaceCandidate()) = 1 and result = this.getAnImplementedBusinessInterfaceCandidate() or // ...or each business interface needs to be declared explicitly. ( count(this.getAnImplementedBusinessInterfaceCandidate()) != 1 or - count(this.getAnExplicitBusinessInterface()) != 0 + exists(this.getAnExplicitBusinessInterface()) ) and result = this.getAnExplicitBusinessInterface() } diff --git a/java/ql/src/Likely Bugs/Termination/SpinOnField.ql b/java/ql/src/Likely Bugs/Termination/SpinOnField.ql index 8675bbd418b0..d1a7d49db55c 100644 --- a/java/ql/src/Likely Bugs/Termination/SpinOnField.ql +++ b/java/ql/src/Likely Bugs/Termination/SpinOnField.ql @@ -33,8 +33,8 @@ class Empty extends Stmt { class EmptyLoop extends Stmt { EmptyLoop() { exists(ForStmt stmt | stmt = this | - count(stmt.getAnInit()) = 0 and - count(stmt.getAnUpdate()) = 0 and + not exists(stmt.getAnInit()) and + not exists(stmt.getAnUpdate()) and stmt.getStmt() instanceof Empty ) or From 8ee5d76e58791a3b731bb0578b3da61a8d17c64f Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Fri, 29 Aug 2025 13:34:00 +0200 Subject: [PATCH 5/6] Fix some violations of ql/dataflow-module-naming-convention. --- .../cryptography/modules/OpenSSL.qll | 4 +-- .../cpp/security/boostorg/asio/protocols.qll | 8 ++--- .../ql/test/library-tests/FlowSummary/test.ql | 6 ++-- .../src/Security/CWE-327/FluentApiModel.qll | 9 +++-- shared/dataflow/codeql/dataflow/DataFlow.qll | 12 +++---- .../codeql/dataflow/TaintTracking.qll | 34 +++++++++---------- 6 files changed, 39 insertions(+), 34 deletions(-) diff --git a/cpp/ql/lib/experimental/cryptography/modules/OpenSSL.qll b/cpp/ql/lib/experimental/cryptography/modules/OpenSSL.qll index 0a52fa5bb815..6b6338a49260 100644 --- a/cpp/ql/lib/experimental/cryptography/modules/OpenSSL.qll +++ b/cpp/ql/lib/experimental/cryptography/modules/OpenSSL.qll @@ -652,14 +652,14 @@ module KeyGeneration { * Trace from EVP_PKEY_CTX* at algorithm sink to keygen, * users can then extrapolatae the matching algorithm from the alg sink to the keygen */ - module EVP_PKEY_CTX_Ptr_Source_to_KeyGenOperationWithNoSize implements DataFlow::ConfigSig { + module EVP_PKEY_CTX_Ptr_Source_to_KeyGenOperationWithNoSizeConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { isEVP_PKEY_CTX_Source(source, _) } predicate isSink(DataFlow::Node sink) { isKeyGen_EVP_PKEY_CTX_Sink(sink, _) } } module EVP_PKEY_CTX_Ptr_Source_to_KeyGenOperationWithNoSize_Flow = - DataFlow::Global; + DataFlow::Global; /** * UNKNOWN key sizes to general purpose key generation functions (i.e., that take in no key size and assume diff --git a/cpp/ql/lib/semmle/code/cpp/security/boostorg/asio/protocols.qll b/cpp/ql/lib/semmle/code/cpp/security/boostorg/asio/protocols.qll index 559ebd444f32..23d8789f56dc 100644 --- a/cpp/ql/lib/semmle/code/cpp/security/boostorg/asio/protocols.qll +++ b/cpp/ql/lib/semmle/code/cpp/security/boostorg/asio/protocols.qll @@ -378,12 +378,12 @@ module BoostorgAsio { * Constructs a standard data flow computation for protocol values to the first argument * of a context constructor. */ - module SslContextCallGlobal { - private module C implements DataFlow::ConfigSig { - import Config + module SslContextCallGlobal { + private module Config implements DataFlow::ConfigSig { + import SslConfig } - import DataFlow::Global + import DataFlow::Global } /** diff --git a/javascript/ql/test/library-tests/FlowSummary/test.ql b/javascript/ql/test/library-tests/FlowSummary/test.ql index e8ca23a423cd..0e40dcdadb09 100644 --- a/javascript/ql/test/library-tests/FlowSummary/test.ql +++ b/javascript/ql/test/library-tests/FlowSummary/test.ql @@ -8,7 +8,7 @@ DataFlow::CallNode getACall(string name) { result.getCalleeNode().getALocalSource() = DataFlow::globalVarRef(name) } -module ConfigArg implements DataFlow::ConfigSig { +module FlowConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node node) { node = getACall("source") } predicate isSink(DataFlow::Node node) { node = getACall("sink").getAnArgument() } @@ -19,7 +19,7 @@ module ConfigArg implements DataFlow::ConfigSig { } } -module Configuration = DataFlow::Global; +module Flow = DataFlow::Global; class BasicBarrierGuard extends DataFlow::CallNode { BasicBarrierGuard() { this = getACall("isSafe") } @@ -32,5 +32,5 @@ class BasicBarrierGuard extends DataFlow::CallNode { deprecated class ConsistencyConfig extends ConsistencyConfiguration { ConsistencyConfig() { this = "ConsistencyConfig" } - override DataFlow::Node getAnAlert() { Configuration::flow(_, result) } + override DataFlow::Node getAnAlert() { Flow::flow(_, result) } } diff --git a/python/ql/src/Security/CWE-327/FluentApiModel.qll b/python/ql/src/Security/CWE-327/FluentApiModel.qll index 8dd90a588217..cd20a852d51f 100644 --- a/python/ql/src/Security/CWE-327/FluentApiModel.qll +++ b/python/ql/src/Security/CWE-327/FluentApiModel.qll @@ -15,7 +15,7 @@ import TlsLibraryModel * The state is represented as a bit vector, where each bit corresponds to a * protocol version. The bit is set if the protocol is allowed. */ -module InsecureContextConfiguration implements DataFlow::StateConfigSig { +module InsecureContextConfig implements DataFlow::StateConfigSig { private newtype TFlowState = TMkFlowState(TlsLibrary library, int bits) { bits in [0 .. max(any(ProtocolVersion v).getBit()) * 2 - 1] @@ -116,7 +116,12 @@ module InsecureContextConfiguration implements DataFlow::StateConfigSig { } } -private module InsecureContextFlow = DataFlow::GlobalWithState; +/** + * DEPRECATED: Will be removed in the future. + */ +deprecated module InsecureContextConfiguration = InsecureContextConfig; + +private module InsecureContextFlow = DataFlow::GlobalWithState; /** * Holds if `conectionCreation` marks the creation of a connection based on the contex diff --git a/shared/dataflow/codeql/dataflow/DataFlow.qll b/shared/dataflow/codeql/dataflow/DataFlow.qll index 3483287e3b39..07430741128e 100644 --- a/shared/dataflow/codeql/dataflow/DataFlow.qll +++ b/shared/dataflow/codeql/dataflow/DataFlow.qll @@ -695,7 +695,7 @@ module DataFlowMake Lang> { * Constructs a global data flow computation. */ module Global implements GlobalFlowSig { - private module C implements FullStateConfigSig { + private module StateConfig implements FullStateConfigSig { import DefaultState import Config @@ -706,11 +706,11 @@ module DataFlowMake Lang> { } } - private module Stage1 = ImplStage1; + private module Stage1 = ImplStage1; import Stage1::PartialFlow - private module Flow = Impl; + private module Flow = Impl; import Flow } @@ -719,7 +719,7 @@ module DataFlowMake Lang> { * Constructs a global data flow computation using flow state. */ module GlobalWithState implements GlobalFlowSig { - private module C implements FullStateConfigSig { + private module StateConfig implements FullStateConfigSig { import Config predicate accessPathLimit = Config::accessPathLimit/0; @@ -735,11 +735,11 @@ module DataFlowMake Lang> { } } - private module Stage1 = ImplStage1; + private module Stage1 = ImplStage1; import Stage1::PartialFlow - private module Flow = Impl; + private module Flow = Impl; import Flow } diff --git a/shared/dataflow/codeql/dataflow/TaintTracking.qll b/shared/dataflow/codeql/dataflow/TaintTracking.qll index bd4b4ecd6ca5..9f82b38ffe75 100644 --- a/shared/dataflow/codeql/dataflow/TaintTracking.qll +++ b/shared/dataflow/codeql/dataflow/TaintTracking.qll @@ -56,7 +56,7 @@ module TaintFlowMake< private import MakeImpl as DataFlowInternal private import MakeImplStage1 as DataFlowInternalStage1 - private module AddTaintDefaults implements + private module MakeAddTaintDefaultsConfig implements DataFlowInternal::FullStateConfigSig { import Config @@ -98,15 +98,15 @@ module TaintFlowMake< } } - private module C implements DataFlowInternal::FullStateConfigSig { - import AddTaintDefaults + private module AddTaintDefaultsConfig implements DataFlowInternal::FullStateConfigSig { + import MakeAddTaintDefaultsConfig } - private module Stage1 = DataFlowInternalStage1::ImplStage1; + private module Stage1 = DataFlowInternalStage1::ImplStage1; import Stage1::PartialFlow - private module Flow = DataFlowInternal::Impl; + private module Flow = DataFlowInternal::Impl; import Flow } @@ -132,15 +132,15 @@ module TaintFlowMake< } } - private module C implements DataFlowInternal::FullStateConfigSig { - import AddTaintDefaults + private module AddTaintDefaultsConfig implements DataFlowInternal::FullStateConfigSig { + import MakeAddTaintDefaultsConfig } - private module Stage1 = DataFlowInternalStage1::ImplStage1; + private module Stage1 = DataFlowInternalStage1::ImplStage1; import Stage1::PartialFlow - private module Flow = DataFlowInternal::Impl; + private module Flow = DataFlowInternal::Impl; import Flow } @@ -234,15 +234,15 @@ module TaintFlowMake< } } - private module C implements DataFlowInternal::FullStateConfigSig { - import AddTaintDefaults> + private module AddTaintDefaultsConfig implements DataFlowInternal::FullStateConfigSig { + import MakeAddTaintDefaultsConfig> } - private module Stage1 = DataFlowInternalStage1::ImplStage1; + private module Stage1 = DataFlowInternalStage1::ImplStage1; import Stage1::PartialFlow - private module Flow = DataFlowInternal::Impl; + private module Flow = DataFlowInternal::Impl; import Flow } @@ -272,15 +272,15 @@ module TaintFlowMake< } } - private module C implements DataFlowInternal::FullStateConfigSig { - import AddTaintDefaults> + private module AddTaintDefaultsConfig implements DataFlowInternal::FullStateConfigSig { + import MakeAddTaintDefaultsConfig> } - private module Stage1 = DataFlowInternalStage1::ImplStage1; + private module Stage1 = DataFlowInternalStage1::ImplStage1; import Stage1::PartialFlow - private module Flow = DataFlowInternal::Impl; + private module Flow = DataFlowInternal::Impl; import Flow } From 106dc1af5072680f1d82cb2613de55d65cfb8a83 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Fri, 29 Aug 2025 14:17:11 +0200 Subject: [PATCH 6/6] Fix violations of ql/if-with-none. --- .../lib/codeql/actions/ast/internal/Ast.qll | 16 +- .../codeql/actions/dataflow/ExternalFlow.qll | 8 +- .../security/ArtifactPoisoningQuery.qll | 17 +- .../code/cpp/rangeanalysis/RangeAnalysis.qll | 18 +- .../CWE/CWE-190/IntegerOverflowTainted.ql | 8 +- .../IntegerOverflow/RangeAnalysis.qll | 311 +++++++++--------- .../semmle/code/java/frameworks/Mockito.qll | 35 +- .../modules/CryptographyModule.qll | 40 +-- .../codeql/rangeanalysis/RangeAnalysis.qll | 9 +- 9 files changed, 221 insertions(+), 241 deletions(-) diff --git a/actions/ql/lib/codeql/actions/ast/internal/Ast.qll b/actions/ql/lib/codeql/actions/ast/internal/Ast.qll index b0cbb8a1d79e..b922214e21c5 100644 --- a/actions/ql/lib/codeql/actions/ast/internal/Ast.qll +++ b/actions/ql/lib/codeql/actions/ast/internal/Ast.qll @@ -125,12 +125,11 @@ abstract class AstNodeImpl extends TAstNode { * Gets the enclosing Step. */ StepImpl getEnclosingStep() { - if this instanceof StepImpl - then result = this - else - if this instanceof ScalarValueImpl - then result.getAChildNode*() = this.getParentNode() - else none() + this instanceof StepImpl and + result = this + or + this instanceof ScalarValueImpl and + result.getAChildNode*() = this.getParentNode() } /** @@ -1416,9 +1415,8 @@ class ExternalJobImpl extends JobImpl, UsesImpl { override string getVersion() { exists(YamlString name | n.lookup("uses") = name and - if not name.getValue().matches("\\.%") - then result = name.getValue().regexpCapture(repoUsesParser(), 4) - else none() + not name.getValue().matches("\\.%") and + result = name.getValue().regexpCapture(repoUsesParser(), 4) ) } } diff --git a/actions/ql/lib/codeql/actions/dataflow/ExternalFlow.qll b/actions/ql/lib/codeql/actions/dataflow/ExternalFlow.qll index 2914dac5f0a6..9667c6e525ea 100644 --- a/actions/ql/lib/codeql/actions/dataflow/ExternalFlow.qll +++ b/actions/ql/lib/codeql/actions/dataflow/ExternalFlow.qll @@ -63,10 +63,10 @@ predicate madSource(DataFlow::Node source, string kind, string fieldName) { ( if fieldName.trim().matches("env.%") then source.asExpr() = uses.getInScopeEnvVarExpr(fieldName.trim().replaceAll("env.", "")) - else - if fieldName.trim().matches("output.%") - then source.asExpr() = uses - else none() + else ( + fieldName.trim().matches("output.%") and + source.asExpr() = uses + ) ) ) } diff --git a/actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll b/actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll index 4b937c5bacb0..9f3ed33db961 100644 --- a/actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll +++ b/actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll @@ -171,10 +171,10 @@ class ActionsGitHubScriptDownloadStep extends UntrustedArtifactDownloadStep, Use .getScript() .getACommand() .regexpCapture(unzipRegexp() + unzipDirArgRegexp(), 3))) - else - if this.getAFollowingStep().(Run).getScript().getACommand().regexpMatch(unzipRegexp()) - then result = "GITHUB_WORKSPACE/" - else none() + else ( + this.getAFollowingStep().(Run).getScript().getACommand().regexpMatch(unzipRegexp()) and + result = "GITHUB_WORKSPACE/" + ) } } @@ -207,12 +207,13 @@ class GHRunArtifactDownloadStep extends UntrustedArtifactDownloadStep, Run { .getScript() .getACommand() .regexpCapture(unzipRegexp() + unzipDirArgRegexp(), 3))) - else - if + else ( + ( this.getAFollowingStep().(Run).getScript().getACommand().regexpMatch(unzipRegexp()) or this.getScript().getACommand().regexpMatch(unzipRegexp()) - then result = "GITHUB_WORKSPACE/" - else none() + ) and + result = "GITHUB_WORKSPACE/" + ) } } diff --git a/cpp/ql/lib/experimental/semmle/code/cpp/rangeanalysis/RangeAnalysis.qll b/cpp/ql/lib/experimental/semmle/code/cpp/rangeanalysis/RangeAnalysis.qll index e5de44b396d8..e026c4dbe4b9 100644 --- a/cpp/ql/lib/experimental/semmle/code/cpp/rangeanalysis/RangeAnalysis.qll +++ b/cpp/ql/lib/experimental/semmle/code/cpp/rangeanalysis/RangeAnalysis.qll @@ -298,10 +298,11 @@ private predicate boundFlowStep(Instruction i, NonPhiOperand op, int delta, bool else if strictlyNegative(x) then upper = true and delta = -1 - else - if negative(x) - then upper = true and delta = 0 - else none() + else ( + negative(x) and + upper = true and + delta = 0 + ) ) or exists(Operand x | @@ -321,10 +322,11 @@ private predicate boundFlowStep(Instruction i, NonPhiOperand op, int delta, bool else if strictlyNegative(x) then upper = false and delta = 1 - else - if negative(x) - then upper = false and delta = 0 - else none() + else ( + negative(x) and + upper = false and + delta = 0 + ) ) or i.(RemInstruction).getRightOperand() = op and positive(op) and delta = -1 and upper = true diff --git a/cpp/ql/src/Security/CWE/CWE-190/IntegerOverflowTainted.ql b/cpp/ql/src/Security/CWE/CWE-190/IntegerOverflowTainted.ql index 6ff06d355b9b..3978d2ded95d 100644 --- a/cpp/ql/src/Security/CWE/CWE-190/IntegerOverflowTainted.ql +++ b/cpp/ql/src/Security/CWE/CWE-190/IntegerOverflowTainted.ql @@ -25,10 +25,10 @@ import semmle.code.cpp.controlflow.IRGuards as IRGuards predicate outOfBoundsExpr(Expr expr, string kind) { if convertedExprMightOverflowPositively(expr) then kind = "overflow" - else - if convertedExprMightOverflowNegatively(expr) - then kind = "overflow negatively" - else none() + else ( + convertedExprMightOverflowNegatively(expr) and + kind = "overflow negatively" + ) } predicate isSource(FS::FlowSource source, string sourceType) { sourceType = source.getSourceType() } diff --git a/go/ql/src/experimental/IntegerOverflow/RangeAnalysis.qll b/go/ql/src/experimental/IntegerOverflow/RangeAnalysis.qll index eccb0b735896..828fc392a2df 100644 --- a/go/ql/src/experimental/IntegerOverflow/RangeAnalysis.qll +++ b/go/ql/src/experimental/IntegerOverflow/RangeAnalysis.qll @@ -347,91 +347,86 @@ float getALowerBound(Expr expr) { * Gets a possible upper bound of SSA definition `def`. */ float getAnSsaUpperBound(SsaDefinition def) { - if recursiveSelfDef(def) - then none() - else ( - if def instanceof SsaExplicitDefinition - then - exists(SsaExplicitDefinition explicitDef | explicitDef = def | - //SSA definition coresponding to a `SimpleAssignStmt` - if explicitDef.getInstruction() instanceof IR::AssignInstruction - then - exists(IR::AssignInstruction assignInstr, SimpleAssignStmt simpleAssign | - assignInstr = explicitDef.getInstruction() and - assignInstr.getRhs().(IR::EvalInstruction).getExpr() = simpleAssign.getRhs() and - result = getAnUpperBound(simpleAssign.getRhs()) - ) - or - //SSA definition coresponding to a ValueSpec(used in a variable declaration) - exists(IR::AssignInstruction declInstr, ValueSpec vs, int i, Expr init | - declInstr = explicitDef.getInstruction() and - declInstr = IR::initInstruction(vs, i) and - init = vs.getInit(i) and - result = getAnUpperBound(init) - ) - or - //SSA definition coresponding to an `AddAssignStmt` (x += y) or `SubAssignStmt` (x -= y) - exists( - IR::AssignInstruction assignInstr, SsaExplicitDefinition prevDef, - CompoundAssignStmt compoundAssign, float prevBound, float delta - | - assignInstr = explicitDef.getInstruction() and - getAUse(prevDef) = compoundAssign.getLhs() and - assignInstr = IR::assignInstruction(compoundAssign, 0) and - prevBound = getAnSsaUpperBound(prevDef) and - if compoundAssign instanceof AddAssignStmt - then - delta = getAnUpperBound(compoundAssign.getRhs()) and - result = addRoundingUp(prevBound, delta) - else - if compoundAssign instanceof SubAssignStmt - then - delta = getALowerBound(compoundAssign.getRhs()) and - result = addRoundingUp(prevBound, -delta) - else none() - ) - else - //SSA definition coresponding to an `IncDecStmt` - if explicitDef.getInstruction() instanceof IR::IncDecInstruction + not recursiveSelfDef(def) and + ( + def instanceof SsaExplicitDefinition and + exists(SsaExplicitDefinition explicitDef | explicitDef = def | + //SSA definition coresponding to a `SimpleAssignStmt` + if explicitDef.getInstruction() instanceof IR::AssignInstruction + then + exists(IR::AssignInstruction assignInstr, SimpleAssignStmt simpleAssign | + assignInstr = explicitDef.getInstruction() and + assignInstr.getRhs().(IR::EvalInstruction).getExpr() = simpleAssign.getRhs() and + result = getAnUpperBound(simpleAssign.getRhs()) + ) + or + //SSA definition coresponding to a ValueSpec(used in a variable declaration) + exists(IR::AssignInstruction declInstr, ValueSpec vs, int i, Expr init | + declInstr = explicitDef.getInstruction() and + declInstr = IR::initInstruction(vs, i) and + init = vs.getInit(i) and + result = getAnUpperBound(init) + ) + or + //SSA definition coresponding to an `AddAssignStmt` (x += y) or `SubAssignStmt` (x -= y) + exists( + IR::AssignInstruction assignInstr, SsaExplicitDefinition prevDef, + CompoundAssignStmt compoundAssign, float prevBound, float delta + | + assignInstr = explicitDef.getInstruction() and + getAUse(prevDef) = compoundAssign.getLhs() and + assignInstr = IR::assignInstruction(compoundAssign, 0) and + prevBound = getAnSsaUpperBound(prevDef) and + if compoundAssign instanceof AddAssignStmt then - exists(IncDecStmt incOrDec, IR::IncDecInstruction instr, float exprLB | - instr = explicitDef.getInstruction() and - exprLB = getAnUpperBound(incOrDec.getOperand()) and - instr.getRhs().(IR::EvalIncDecRhsInstruction).getStmt() = incOrDec and - ( - //IncStmt(x++) - exists(IncStmt inc | - inc = incOrDec and - result = addRoundingUp(exprLB, 1) - ) - or - //DecStmt(x--) - exists(DecStmt dec | - dec = incOrDec and - result = addRoundingUp(exprLB, -1) - ) - ) - ) + delta = getAnUpperBound(compoundAssign.getRhs()) and + result = addRoundingUp(prevBound, delta) else - //SSA definition coreponding to the init of the parameter - if explicitDef.getInstruction() instanceof IR::InitParameterInstruction + if compoundAssign instanceof SubAssignStmt then - exists(IR::InitParameterInstruction instr, Parameter p | - instr = explicitDef.getInstruction() and - IR::initParamInstruction(p) = instr and - result = typeMaxValue(p.getType()) - ) + delta = getALowerBound(compoundAssign.getRhs()) and + result = addRoundingUp(prevBound, -delta) else none() - ) - else - //this SSA definition is a phi node. - if def instanceof SsaPhiNode - then - exists(SsaPhiNode phi | - phi = def and - result = getAnSsaUpperBound(phi.getAnInput().getDefinition()) ) - else none() + else + //SSA definition coresponding to an `IncDecStmt` + if explicitDef.getInstruction() instanceof IR::IncDecInstruction + then + exists(IncDecStmt incOrDec, IR::IncDecInstruction instr, float exprLB | + instr = explicitDef.getInstruction() and + exprLB = getAnUpperBound(incOrDec.getOperand()) and + instr.getRhs().(IR::EvalIncDecRhsInstruction).getStmt() = incOrDec and + ( + //IncStmt(x++) + exists(IncStmt inc | + inc = incOrDec and + result = addRoundingUp(exprLB, 1) + ) + or + //DecStmt(x--) + exists(DecStmt dec | + dec = incOrDec and + result = addRoundingUp(exprLB, -1) + ) + ) + ) + else ( + //SSA definition coreponding to the init of the parameter + explicitDef.getInstruction() instanceof IR::InitParameterInstruction and + exists(IR::InitParameterInstruction instr, Parameter p | + instr = explicitDef.getInstruction() and + IR::initParamInstruction(p) = instr and + result = typeMaxValue(p.getType()) + ) + ) + ) + or + //this SSA definition is a phi node. + def instanceof SsaPhiNode and + exists(SsaPhiNode phi | + phi = def and + result = getAnSsaUpperBound(phi.getAnInput().getDefinition()) + ) ) } @@ -439,91 +434,85 @@ float getAnSsaUpperBound(SsaDefinition def) { * Gets a possible lower bound of SSA definition `def`. */ float getAnSsaLowerBound(SsaDefinition def) { - if recursiveSelfDef(def) - then none() - else ( - if def instanceof SsaExplicitDefinition - then - exists(SsaExplicitDefinition explicitDef | explicitDef = def | - if explicitDef.getInstruction() instanceof IR::AssignInstruction - then - //SimpleAssignStmt - exists(IR::AssignInstruction assignInstr, SimpleAssignStmt simpleAssign | - assignInstr = explicitDef.getInstruction() and - assignInstr.getRhs().(IR::EvalInstruction).getExpr() = simpleAssign.getRhs() and - result = getALowerBound(simpleAssign.getRhs()) - ) - or - //ValueSpec - exists(IR::AssignInstruction declInstr, ValueSpec vs, int i, Expr init | - declInstr = explicitDef.getInstruction() and - declInstr = IR::initInstruction(vs, i) and - init = vs.getInit(i) and - result = getALowerBound(init) - ) - or - //AddAssignStmt(x += y) - exists( - IR::AssignInstruction assignInstr, SsaExplicitDefinition prevDef, - CompoundAssignStmt compoundAssign, float prevBound, float delta - | - assignInstr = explicitDef.getInstruction() and - getAUse(prevDef) = compoundAssign.getLhs() and - assignInstr = IR::assignInstruction(compoundAssign, 0) and - prevBound = getAnSsaLowerBound(prevDef) and - if compoundAssign instanceof AddAssignStmt - then - delta = getALowerBound(compoundAssign.getRhs()) and - result = addRoundingDown(prevBound, delta) - else - if compoundAssign instanceof SubAssignStmt - then - delta = getAnUpperBound(compoundAssign.getRhs()) and - result = addRoundingDown(prevBound, -delta) - else none() + not recursiveSelfDef(def) and + ( + def instanceof SsaExplicitDefinition and + exists(SsaExplicitDefinition explicitDef | explicitDef = def | + if explicitDef.getInstruction() instanceof IR::AssignInstruction + then + //SimpleAssignStmt + exists(IR::AssignInstruction assignInstr, SimpleAssignStmt simpleAssign | + assignInstr = explicitDef.getInstruction() and + assignInstr.getRhs().(IR::EvalInstruction).getExpr() = simpleAssign.getRhs() and + result = getALowerBound(simpleAssign.getRhs()) + ) + or + //ValueSpec + exists(IR::AssignInstruction declInstr, ValueSpec vs, int i, Expr init | + declInstr = explicitDef.getInstruction() and + declInstr = IR::initInstruction(vs, i) and + init = vs.getInit(i) and + result = getALowerBound(init) + ) + or + //AddAssignStmt(x += y) + exists( + IR::AssignInstruction assignInstr, SsaExplicitDefinition prevDef, + CompoundAssignStmt compoundAssign, float prevBound, float delta + | + assignInstr = explicitDef.getInstruction() and + getAUse(prevDef) = compoundAssign.getLhs() and + assignInstr = IR::assignInstruction(compoundAssign, 0) and + prevBound = getAnSsaLowerBound(prevDef) and + ( + compoundAssign instanceof AddAssignStmt and + delta = getALowerBound(compoundAssign.getRhs()) and + result = addRoundingDown(prevBound, delta) + or + compoundAssign instanceof SubAssignStmt and + delta = getAnUpperBound(compoundAssign.getRhs()) and + result = addRoundingDown(prevBound, -delta) ) - else - //IncDecStmt - if explicitDef.getInstruction() instanceof IR::IncDecInstruction - then - exists(IncDecStmt incOrDec, IR::IncDecInstruction instr, float exprLB | - instr = explicitDef.getInstruction() and - exprLB = getALowerBound(incOrDec.getOperand()) and - instr.getRhs().(IR::EvalIncDecRhsInstruction).getStmt() = incOrDec and - ( - //IncStmt(x++) - exists(IncStmt inc | - inc = incOrDec and - result = addRoundingDown(exprLB, 1) - ) - or - //DecStmt(x--) - exists(DecStmt dec | - dec = incOrDec and - result = addRoundingDown(exprLB, -1) - ) + ) + else + //IncDecStmt + if explicitDef.getInstruction() instanceof IR::IncDecInstruction + then + exists(IncDecStmt incOrDec, IR::IncDecInstruction instr, float exprLB | + instr = explicitDef.getInstruction() and + exprLB = getALowerBound(incOrDec.getOperand()) and + instr.getRhs().(IR::EvalIncDecRhsInstruction).getStmt() = incOrDec and + ( + //IncStmt(x++) + exists(IncStmt inc | + inc = incOrDec and + result = addRoundingDown(exprLB, 1) ) - ) - else - //init of the function parameter - if explicitDef.getInstruction() instanceof IR::InitParameterInstruction - then - exists(IR::InitParameterInstruction instr, Parameter p | - instr = explicitDef.getInstruction() and - IR::initParamInstruction(p) = instr and - result = typeMinValue(p.getType()) + or + //DecStmt(x--) + exists(DecStmt dec | + dec = incOrDec and + result = addRoundingDown(exprLB, -1) ) - else none() - ) - else - //phi node - if def instanceof SsaPhiNode - then - exists(SsaPhiNode phi | - phi = def and - result = getAnSsaLowerBound(phi.getAnInput().getDefinition()) + ) + ) + else ( + //init of the function parameter + explicitDef.getInstruction() instanceof IR::InitParameterInstruction and + exists(IR::InitParameterInstruction instr, Parameter p | + instr = explicitDef.getInstruction() and + IR::initParamInstruction(p) = instr and + result = typeMinValue(p.getType()) + ) ) - else none() + ) + or + //phi node + def instanceof SsaPhiNode and + exists(SsaPhiNode phi | + phi = def and + result = getAnSsaLowerBound(phi.getAnInput().getDefinition()) + ) ) } diff --git a/java/ql/lib/semmle/code/java/frameworks/Mockito.qll b/java/ql/lib/semmle/code/java/frameworks/Mockito.qll index 1a8d987a38e4..a8559060d306 100644 --- a/java/ql/lib/semmle/code/java/frameworks/Mockito.qll +++ b/java/ql/lib/semmle/code/java/frameworks/Mockito.qll @@ -223,10 +223,10 @@ class MockitoInjectedField extends MockitoAnnotatedField { // If there is no initializer for this field, and there is a most mockable constructor, // then we are doing a parameterized injection of mocks into a most mockable constructor. result = mockInjectedClass.getAMostMockableConstructor() - else - if this.usingPropertyInjection() - then - // We will call the no-arg constructor if the field wasn't initialized. + else ( + this.usingPropertyInjection() and + // We will call the no-arg constructor if the field wasn't initialized. + ( not exists(this.getInitializer()) and result = mockInjectedClass.getNoArgsConstructor() or @@ -241,9 +241,8 @@ class MockitoInjectedField extends MockitoAnnotatedField { // once, but we instead assume that there are sufficient mocks to go around. mockedField.getType().(RefType).getAnAncestor() = result.getParameterType(0) ) - else - // There's no instance, and no no-arg constructor we can call, so injection fails. - none() + ) + ) ) } @@ -253,18 +252,16 @@ class MockitoInjectedField extends MockitoAnnotatedField { * Field injection only occurs if property injection and not constructor injection is used. */ Field getASetField() { - if this.usingPropertyInjection() - then - result = this.getMockInjectedClass().getASetField() and - exists(MockitoMockedField mockedField | - mockedField.getDeclaringType() = this.getDeclaringType() and - mockedField.isValid() - | - // We make a simplifying assumption here - in theory, each mock can only be injected - // once, but we instead assume that there are sufficient mocks to go around. - mockedField.getType().(RefType).getAnAncestor() = result.getType() - ) - else none() + this.usingPropertyInjection() and + result = this.getMockInjectedClass().getASetField() and + exists(MockitoMockedField mockedField | + mockedField.getDeclaringType() = this.getDeclaringType() and + mockedField.isValid() + | + // We make a simplifying assumption here - in theory, each mock can only be injected + // once, but we instead assume that there are sufficient mocks to go around. + mockedField.getType().(RefType).getAnAncestor() = result.getType() + ) } } diff --git a/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll b/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll index 405433b07354..0831d625d803 100644 --- a/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll +++ b/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll @@ -117,31 +117,25 @@ module KDF { override predicate requiresIteration() { this.getAlgorithm().getKDFName() in ["PBKDF2HMAC"] } override DataFlow::Node getIterationSizeSrc() { - if this.requiresIteration() - then - // ASSUMPTION: ONLY EVER in arg 3 in PBKDF2HMAC - result = Utils::getUltimateSrcFromApiNode(this.getParameter(3, "iterations")) - else none() + this.requiresIteration() and + // ASSUMPTION: ONLY EVER in arg 3 in PBKDF2HMAC + result = Utils::getUltimateSrcFromApiNode(this.getParameter(3, "iterations")) } override DataFlow::Node getSaltConfigSrc() { - if this.requiresSalt() - then - // SCRYPT has it in arg 1 - if this.getAlgorithm().getKDFName() = "SCRYPT" - then result = Utils::getUltimateSrcFromApiNode(this.getParameter(1, "salt")) - else - // EVERYTHING ELSE that uses salt is in arg 2 - result = Utils::getUltimateSrcFromApiNode(this.getParameter(2, "salt")) - else none() + this.requiresSalt() and + // SCRYPT has it in arg 1 + if this.getAlgorithm().getKDFName() = "SCRYPT" + then result = Utils::getUltimateSrcFromApiNode(this.getParameter(1, "salt")) + else + // EVERYTHING ELSE that uses salt is in arg 2 + result = Utils::getUltimateSrcFromApiNode(this.getParameter(2, "salt")) } override DataFlow::Node getHashConfigSrc() { - if this.requiresHash() - then - // ASSUMPTION: ONLY EVER in arg 0 - result = Utils::getUltimateSrcFromApiNode(this.getParameter(0, "algorithm")) - else none() + this.requiresHash() and + // ASSUMPTION: ONLY EVER in arg 0 + result = Utils::getUltimateSrcFromApiNode(this.getParameter(0, "algorithm")) } // TODO: get encryption algorithm for CBC-based KDF? @@ -152,11 +146,9 @@ module KDF { } override DataFlow::Node getModeSrc() { - if this.requiresMode() - then - // ASSUMPTION: ONLY EVER in arg 1 - result = Utils::getUltimateSrcFromApiNode(this.getParameter(1, "mode")) - else none() + this.requiresMode() and + // ASSUMPTION: ONLY EVER in arg 1 + result = Utils::getUltimateSrcFromApiNode(this.getParameter(1, "mode")) } } } diff --git a/shared/rangeanalysis/codeql/rangeanalysis/RangeAnalysis.qll b/shared/rangeanalysis/codeql/rangeanalysis/RangeAnalysis.qll index 1d17ad8346c4..23393f7f0253 100644 --- a/shared/rangeanalysis/codeql/rangeanalysis/RangeAnalysis.qll +++ b/shared/rangeanalysis/codeql/rangeanalysis/RangeAnalysis.qll @@ -784,10 +784,11 @@ module RangeStage< else if strictlyNegativeIntegralExpr(x) then upper = false and delta = D::fromInt(1) - else - if semNegative(x) - then upper = false and delta = D::fromInt(0) - else none() + else ( + semNegative(x) and + upper = false and + delta = D::fromInt(0) + ) ) or e2.(Sem::RemExpr).getRightOperand() = e1 and