diff --git a/actions/ql/lib/codeql/Locations.qll b/actions/ql/lib/codeql/Locations.qll index 96b5d45f18e0..24c6ae9cda1d 100644 --- a/actions/ql/lib/codeql/Locations.qll +++ b/actions/ql/lib/codeql/Locations.qll @@ -70,8 +70,8 @@ class Location extends TLocation, TBaseLocation { /** * Holds if this element is at the specified location. - * The location spans column `startcolumn` of line `startline` to - * column `endcolumn` of line `endline` in file `filepath`. + * The location spans column `sc` of line `sl` to + * column `ec` of line `el` in file `p`. * For more information, see * [Providing locations in CodeQL queries](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/). */ diff --git a/actions/ql/lib/codeql/actions/Ast.qll b/actions/ql/lib/codeql/actions/Ast.qll index ae19a7a7e8ca..6e76e4cd665a 100644 --- a/actions/ql/lib/codeql/actions/Ast.qll +++ b/actions/ql/lib/codeql/actions/Ast.qll @@ -261,7 +261,7 @@ class If extends AstNode instanceof IfImpl { } /** - * An Environemnt node representing a deployment environment. + * An Environment node representing a deployment environment. */ class Environment extends AstNode instanceof EnvironmentImpl { string getName() { result = super.getName() } 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/controlflow/BasicBlocks.qll b/actions/ql/lib/codeql/actions/controlflow/BasicBlocks.qll index af5e0f62552f..2dcfd81a47dc 100644 --- a/actions/ql/lib/codeql/actions/controlflow/BasicBlocks.qll +++ b/actions/ql/lib/codeql/actions/controlflow/BasicBlocks.qll @@ -286,7 +286,7 @@ private module Cached { /** * Holds if `cfn` is the `i`th node in basic block `bb`. * - * In other words, `i` is the shortest distance from a node `bb` + * In other words, `i` is the shortest distance from a node `bbStart` * that starts a basic block to `cfn` along the `intraBBSucc` relation. */ cached 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/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..9f3ed33db961 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() { @@ -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/" + ) } } @@ -259,15 +260,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/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/lib/Options.qll b/cpp/ql/lib/Options.qll index a0a13881a941..c4652e3f6cae 100644 --- a/cpp/ql/lib/Options.qll +++ b/cpp/ql/lib/Options.qll @@ -35,7 +35,7 @@ class CustomOptions extends Options { override predicate returnsNull(Call call) { Options.super.returnsNull(call) } /** - * Holds if a call to this function will never return. + * Holds if a call to the function `f` will never return. * * By default, this holds for `exit`, `_exit`, `abort`, `__assert_fail`, * `longjmp`, `error`, `__builtin_unreachable` and any function with a diff --git a/cpp/ql/lib/experimental/cryptography/CryptoArtifact.qll b/cpp/ql/lib/experimental/cryptography/CryptoArtifact.qll index 0bb22d688edf..296b97f66f21 100644 --- a/cpp/ql/lib/experimental/cryptography/CryptoArtifact.qll +++ b/cpp/ql/lib/experimental/cryptography/CryptoArtifact.qll @@ -127,7 +127,7 @@ abstract class CryptographicAlgorithm extends CryptographicArtifact { /** * Normalizes a raw name into a normalized name as found in `CryptoAlgorithmNames.qll`. * Subclassess should override for more api-specific normalization. - * By deafult, converts a raw name to upper-case with no hyphen, underscore, hash, or space. + * By default, converts a raw name to upper-case with no hyphen, underscore, hash, or space. */ bindingset[s] string normalizeName(string s) { 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/experimental/cryptography/utils/OpenSSL/CryptoFunction.qll b/cpp/ql/lib/experimental/cryptography/utils/OpenSSL/CryptoFunction.qll index 2c46a7c06744..1e0f3b6dfbfb 100644 --- a/cpp/ql/lib/experimental/cryptography/utils/OpenSSL/CryptoFunction.qll +++ b/cpp/ql/lib/experimental/cryptography/utils/OpenSSL/CryptoFunction.qll @@ -59,7 +59,7 @@ private string privateNormalizeFunctionName(Function f, string algType) { * * The predicate attempts to restrict normalization to what looks like an openssl * library by looking for functions only in an openssl path (see `isPossibleOpenSSLFunction`). - * This may give false postive functions if a directory erronously appears to be openssl; + * This may give false positive functions if a directory erronously appears to be openssl; * however, we take the stance that if a function * exists strongly mapping to a known function name in a directory such as these, * regardless of whether its actually a part of openSSL or not, we will analyze it as though it were. diff --git a/cpp/ql/lib/experimental/cryptography/utils/OpenSSL/DataBuilders.qll b/cpp/ql/lib/experimental/cryptography/utils/OpenSSL/DataBuilders.qll index ba83de345970..54121c367665 100644 --- a/cpp/ql/lib/experimental/cryptography/utils/OpenSSL/DataBuilders.qll +++ b/cpp/ql/lib/experimental/cryptography/utils/OpenSSL/DataBuilders.qll @@ -49,7 +49,7 @@ private string privateNormalizeFunctionName(Function f, string algType) { * * The predicate attempts to restrict normalization to what looks like an openssl * library by looking for functions only in an openssl path (see `isPossibleOpenSSLFunction`). - * This may give false postive functions if a directory erronously appears to be openssl; + * This may give false positive functions if a directory erronously appears to be openssl; * however, we take the stance that if a function * exists strongly mapping to a known function name in a directory such as these, * regardless of whether its actually a part of openSSL or not, we will analyze it as though it were. diff --git a/cpp/ql/lib/experimental/cryptography/utils/OpenSSL/PassthroughFunction.qll b/cpp/ql/lib/experimental/cryptography/utils/OpenSSL/PassthroughFunction.qll index f772f85afb10..e1e64f78e3ec 100644 --- a/cpp/ql/lib/experimental/cryptography/utils/OpenSSL/PassthroughFunction.qll +++ b/cpp/ql/lib/experimental/cryptography/utils/OpenSSL/PassthroughFunction.qll @@ -31,7 +31,7 @@ predicate knownPassthroughFunction(Function f, int inInd, int outInd) { /** * `c` is a call to a function that preserves the algorithm but changes its form. - * `onExpr` is the input argument passing through to, `outExpr` is the next expression in a dataflow step associated with `c` + * `inExpr` is the input argument passing through to, `outExpr` is the next expression in a dataflow step associated with `c` */ predicate knownPassthoughCall(Call c, Expr inExpr, Expr outExpr) { exists(int inInd, int outInd | 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/lib/semmle/code/cpp/Concept.qll b/cpp/ql/lib/semmle/code/cpp/Concept.qll index 1770c1965ed2..6e66bca98238 100644 --- a/cpp/ql/lib/semmle/code/cpp/Concept.qll +++ b/cpp/ql/lib/semmle/code/cpp/Concept.qll @@ -198,7 +198,7 @@ class ConceptIdExpr extends Expr, @concept_id { final Locatable getATemplateArgumentKind() { result = this.getTemplateArgumentKind(_) } /** - * Gets the `i`th template argument passed to the concept. + * Gets template argument at index `index` passed to the concept, if any. * * For example, if: * ```cpp @@ -219,7 +219,7 @@ class ConceptIdExpr extends Expr, @concept_id { } /** - * Gets the kind of the `i`th template argument value passed to the concept. + * Gets the kind of the template argument value at index `index` passed to the concept, if any. * * For example, if: * ```cpp diff --git a/cpp/ql/lib/semmle/code/cpp/Declaration.qll b/cpp/ql/lib/semmle/code/cpp/Declaration.qll index cedacca4dfe4..6f791234b638 100644 --- a/cpp/ql/lib/semmle/code/cpp/Declaration.qll +++ b/cpp/ql/lib/semmle/code/cpp/Declaration.qll @@ -223,8 +223,8 @@ class Declaration extends Locatable, @declaration { final Locatable getATemplateArgumentKind() { result = this.getTemplateArgumentKind(_) } /** - * Gets the `i`th template argument used to instantiate this declaration from a - * template. + * Gets the template argument at index `index` used to instantiate this declaration from a + * template, if any. * * For example: * @@ -245,9 +245,9 @@ class Declaration extends Locatable, @declaration { } /** - * Gets the `i`th template argument value used to instantiate this declaration - * from a template. When called on a template, this will return the `i`th template - * parameter value if it exists. + * Gets the template argument value at index `index` used to instantiate this declaration + * from a template. When called on a template, this will return the template + * parameter value at index `index` if it exists. * * For example: * diff --git a/cpp/ql/lib/semmle/code/cpp/commons/Printf.qll b/cpp/ql/lib/semmle/code/cpp/commons/Printf.qll index 51d2e294b36e..023ce09c5c18 100644 --- a/cpp/ql/lib/semmle/code/cpp/commons/Printf.qll +++ b/cpp/ql/lib/semmle/code/cpp/commons/Printf.qll @@ -877,7 +877,7 @@ class FormatLiteral extends Literal instanceof StringLiteral { } /** - * Gets the char type required by the nth conversion specifier. + * Gets the char type required by the `n`th conversion specifier. * - in the base case this is the default for the formatting function * (e.g. `char` for `printf`, `char` or `wchar_t` for `wprintf`). * - the `%C` format character reverses wideness. @@ -922,7 +922,7 @@ class FormatLiteral extends Literal instanceof StringLiteral { } /** - * Gets the string type required by the nth conversion specifier. + * Gets the string type required by the `n`th conversion specifier. * - in the base case this is the default for the formatting function * (e.g. `char *` for `printf`, `char *` or `wchar_t *` for `wprintf`). * - the `%S` format character reverses wideness on some platforms. diff --git a/cpp/ql/lib/semmle/code/cpp/controlflow/Dominance.qll b/cpp/ql/lib/semmle/code/cpp/controlflow/Dominance.qll index 424e615f3ea8..a8a4b867ba67 100644 --- a/cpp/ql/lib/semmle/code/cpp/controlflow/Dominance.qll +++ b/cpp/ql/lib/semmle/code/cpp/controlflow/Dominance.qll @@ -101,7 +101,7 @@ predicate postDominates(ControlFlowNode postDominator, ControlFlowNode node) { */ /** - * Holds if `dominator` is an immediate dominator of `node` in the control-flow + * Holds if `dom` is an immediate dominator of `node` in the control-flow * graph of basic blocks. */ predicate bbIDominates(BasicBlock dom, BasicBlock node) = @@ -117,7 +117,7 @@ private predicate bb_predecessor(BasicBlock succ, BasicBlock pred) { bb_successo private predicate bb_exit(ExitBasicBlock exit) { any() } /** - * Holds if `postDominator` is an immediate post-dominator of `node` in the control-flow + * Holds if `pDom` is an immediate post-dominator of `node` in the control-flow * graph of basic blocks. */ predicate bbIPostDominates(BasicBlock pDom, BasicBlock node) = diff --git a/cpp/ql/lib/semmle/code/cpp/controlflow/internal/CFG.qll b/cpp/ql/lib/semmle/code/cpp/controlflow/internal/CFG.qll index 05eafe28275f..23ef58592216 100644 --- a/cpp/ql/lib/semmle/code/cpp/controlflow/internal/CFG.qll +++ b/cpp/ql/lib/semmle/code/cpp/controlflow/internal/CFG.qll @@ -1042,8 +1042,8 @@ private predicate subEdgeIncludingDestructors(Pos p1, Node n1, Node n2, Pos p2) * - `MicrosoftTryFinallyStmt`: On the edge following the `__finally` block for * the case where an exception was thrown and needs to be propagated. */ -DestructorCall getSynthesisedDestructorCallAfterNode(Node n, int i) { - synthetic_destructor_call(n, i, result) +DestructorCall getSynthesisedDestructorCallAfterNode(Node node, int index) { + synthetic_destructor_call(node, index, result) } /** diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll index 72e742f13aa0..b388669d0415 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll @@ -829,8 +829,8 @@ class ContentSet instanceof Content { /** * Holds if this element is at the specified location. - * The location spans column `startcolumn` of line `startline` to - * column `endcolumn` of line `endline` in file `filepath`. + * The location spans column `sc` of line `sl` to + * column `ec` of line `el` in file `path`. * For more information, see * [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/). */ diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll index ef4051171afb..51ffbefb027d 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll @@ -2268,8 +2268,8 @@ class ContentSet instanceof Content { /** * Holds if this element is at the specified location. - * The location spans column `startcolumn` of line `startline` to - * column `endcolumn` of line `endline` in file `filepath`. + * The location spans column `sc` of line `sl` to + * column `ec` of line `el` in file `path`. * For more information, see * [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/). */ diff --git a/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedFunction.qll b/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedFunction.qll index 83736ae98d04..26f5393db103 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedFunction.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedFunction.qll @@ -50,7 +50,7 @@ CppType getEllipsisVariablePRValueType() { CppType getEllipsisVariableGLValueType() { result = getTypeForGLValue(any(UnknownType t)) } /** - * Holds if the function returns a value, as opposed to returning `void`. + * Holds if the function `func` returns a value, as opposed to returning `void`. */ predicate hasReturnValue(Function func) { not func.getUnspecifiedType() instanceof VoidType } diff --git a/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedStmt.qll b/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedStmt.qll index 3914c5e8e597..9dccf7752aa8 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedStmt.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedStmt.qll @@ -601,7 +601,7 @@ class TranslatedReturnVoidStmt extends TranslatedReturnStmt { * The IR translation of an implicit `return` statement generated by the extractor to handle control * flow that reaches the end of a non-`void`-returning function body. Such control flow * produces undefined behavior in C++ but not in C. However even in C using the return value is - * undefined behaviour. We make it return uninitialized memory to get as much flow as possible. + * undefined behavior. We make it return uninitialized memory to get as much flow as possible. */ class TranslatedNoValueReturnStmt extends TranslatedReturnStmt, TranslatedVariableInitialization { TranslatedNoValueReturnStmt() { diff --git a/cpp/ql/lib/semmle/code/cpp/ir/internal/IRUtilities.qll b/cpp/ql/lib/semmle/code/cpp/ir/internal/IRUtilities.qll index bfd850384aca..c42f734a62ac 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/internal/IRUtilities.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/internal/IRUtilities.qll @@ -49,7 +49,8 @@ Type getVariableType(Variable v) { } /** - * Holds if the database contains a `case` label with the specified minimum and maximum value. + * Holds if the database contains a `switchCase` label with the specified minimum `minValue` + * and maximum `maxValue` value. */ predicate hasCaseEdge(SwitchCase switchCase, string minValue, string maxValue) { minValue = switchCase.getExpr().getFullyConverted().getValue() and diff --git a/cpp/ql/lib/semmle/code/cpp/models/interfaces/FunctionInputsAndOutputs.qll b/cpp/ql/lib/semmle/code/cpp/models/interfaces/FunctionInputsAndOutputs.qll index 9c8ee43b52a6..d81bc960988c 100644 --- a/cpp/ql/lib/semmle/code/cpp/models/interfaces/FunctionInputsAndOutputs.qll +++ b/cpp/ql/lib/semmle/code/cpp/models/interfaces/FunctionInputsAndOutputs.qll @@ -371,7 +371,7 @@ class FunctionOutput extends TFunctionOutput { /** * Holds if this is the output value pointed to by a pointer parameter to a function, or the * output value referred to by a reference parameter to a function, where the parameter has - * index `index`. + * index `i`. * * Example: * ``` @@ -389,7 +389,7 @@ class FunctionOutput extends TFunctionOutput { /** * Holds if this is the output value pointed to by a pointer parameter (through `ind` number * of indirections) to a function, or the output value referred to by a reference parameter to - * a function, where the parameter has index `index`. + * a function, where the parameter has index `i`. * * Example: * ``` 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/lib/semmle/code/cpp/security/FileWrite.qll b/cpp/ql/lib/semmle/code/cpp/security/FileWrite.qll index dc421e8f3aee..ebf14341150f 100644 --- a/cpp/ql/lib/semmle/code/cpp/security/FileWrite.qll +++ b/cpp/ql/lib/semmle/code/cpp/security/FileWrite.qll @@ -21,7 +21,9 @@ class FileWrite extends Expr { Expr getDest() { fileWrite(this, _, result) } /** - * Gets the conversion character for this write, if it exists and is known. For example in the following code the write of `value1` has conversion character `"s"`, whereas the write of `value2` has no conversion specifier. + * Gets the conversion character from `source` for this write from , if it exists and is known. + * For example in the following code the write of `value1` has conversion character `"s"`, whereas + * the write of `value2` has no conversion specifier. * ``` * fprintf(file, "%s", value1); * stream << value2; 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..f1cdcaefc6bb 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 @@ -191,11 +191,16 @@ module BoostorgAsio { class SslContextClass extends Class { SslContextClass() { this.getQualifiedName() = "boost::asio::ssl::context" } - ConstructorCall getAContructorCall() { + ConstructorCall getAConstructorCall() { this.getAConstructor().getACallToThisFunction() = result and not result.getLocation().getFile().toString().matches("%/boost/asio/%") and result.fromSource() } + + /** + * DEPRECATED: Use `getAConstructorCall` instead. + */ + deprecated ConstructorCall getAContructorCall() { result = this.getAConstructorCall() } } /** @@ -368,7 +373,7 @@ module BoostorgAsio { */ default predicate isSink(DataFlow::Node sink) { exists(ConstructorCall cc, SslContextClass c, Expr e | e = sink.asExpr() | - c.getAContructorCall() = cc and + c.getAConstructorCall() = cc and cc.getArgument(0) = e ) } @@ -378,12 +383,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 } /** @@ -468,7 +473,7 @@ module BoostorgAsio { predicate isSource(DataFlow::Node source) { exists(SslContextClass c, ConstructorCall cc | cc = source.asExpr() and - c.getAContructorCall() = cc + c.getAConstructorCall() = cc ) } 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/Best Practices/Magic Constants/MagicConstants.qll b/cpp/ql/src/Best Practices/Magic Constants/MagicConstants.qll index 53e33ab4fa56..033840764830 100644 --- a/cpp/ql/src/Best Practices/Magic Constants/MagicConstants.qll +++ b/cpp/ql/src/Best Practices/Magic Constants/MagicConstants.qll @@ -164,12 +164,17 @@ predicate valueOccurrenceCount(string value, int n) { n > 20 } -predicate occurenceCount(Literal lit, string value, int n) { +predicate occurrenceCount(Literal lit, string value, int n) { valueOccurrenceCount(value, n) and value = lit.getValue() and nonTrivialValue(_, lit) } +/** + * DEPRECATED: Use `occurrenceCount` instead. + */ +deprecated predicate occurenceCount = occurrenceCount/3; + /* * Literals repeated frequently */ @@ -178,7 +183,7 @@ predicate check(Literal lit, string value, int n, File f) { // Check that the literal is nontrivial not trivial(lit) and // Check that it is repeated a number of times - occurenceCount(lit, value, n) and + occurrenceCount(lit, value, n) and n > 20 and f = lit.getFile() and // Exclude generated files diff --git a/cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll b/cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll index 863fd1e61203..2b68730fa58d 100644 --- a/cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll +++ b/cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll @@ -128,11 +128,18 @@ abstract class LeapYearFieldAccess extends YearFieldAccess { /** * Holds if the top-level binary operation includes an addition or subtraction operator with an operand specified by `valueToCheck`. */ - predicate additionalAdditionOrSubstractionCheckForLeapYear(int valueToCheck) { + predicate additionalAdditionOrSubtractionCheckForLeapYear(int valueToCheck) { additionalLogicalCheck(this, "+", valueToCheck) or additionalLogicalCheck(this, "-", valueToCheck) } + /** + * DEPRECATED: Use `additionalAdditionOrSubtractionCheckForLeapYear` instead. + */ + deprecated predicate additionalAdditionOrSubstractionCheckForLeapYear(int valueToCheck) { + this.additionalAdditionOrSubtractionCheckForLeapYear(valueToCheck) + } + /** * Holds if this object is used on a modulus 4 operation, which would likely indicate the start of a leap year check. */ @@ -180,13 +187,13 @@ class StructTmLeapYearFieldAccess extends LeapYearFieldAccess { this.additionalModulusCheckForLeapYear(100) and // tm_year represents years since 1900 ( - this.additionalAdditionOrSubstractionCheckForLeapYear(1900) + this.additionalAdditionOrSubtractionCheckForLeapYear(1900) or // some systems may use 2000 for 2-digit year conversions - this.additionalAdditionOrSubstractionCheckForLeapYear(2000) + this.additionalAdditionOrSubtractionCheckForLeapYear(2000) or // converting from/to Unix epoch - this.additionalAdditionOrSubstractionCheckForLeapYear(1970) + this.additionalAdditionOrSubtractionCheckForLeapYear(1970) ) } } diff --git a/cpp/ql/src/Likely Bugs/Protocols/TlsSettingsMisconfiguration.ql b/cpp/ql/src/Likely Bugs/Protocols/TlsSettingsMisconfiguration.ql index f5d1a09d04e9..e50a2d545431 100644 --- a/cpp/ql/src/Likely Bugs/Protocols/TlsSettingsMisconfiguration.ql +++ b/cpp/ql/src/Likely Bugs/Protocols/TlsSettingsMisconfiguration.ql @@ -14,7 +14,7 @@ import cpp import semmle.code.cpp.security.boostorg.asio.protocols predicate isSourceImpl(DataFlow::Node source, ConstructorCall cc) { - exists(BoostorgAsio::SslContextClass c | c.getAContructorCall() = cc and cc = source.asExpr()) + exists(BoostorgAsio::SslContextClass c | c.getAConstructorCall() = cc and cc = source.asExpr()) } predicate isSinkImpl(DataFlow::Node sink, FunctionCall fcSetOptions) { diff --git a/cpp/ql/src/Metrics/Internal/CallableExtents.ql b/cpp/ql/src/Metrics/Internal/CallableExtents.ql index 7a376c6da721..eef36e91bffe 100644 --- a/cpp/ql/src/Metrics/Internal/CallableExtents.ql +++ b/cpp/ql/src/Metrics/Internal/CallableExtents.ql @@ -15,8 +15,8 @@ import cpp class RangeFunction extends Function { /** * Holds if this function is at the specified location. - * The location spans column `startcolumn` of line `startline` to - * column `endcolumn` of line `endline` in file `filepath`. + * The location spans column `sc` of line `sl` to + * column `ec` of line `el` in file `path`. * For more information, see * [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/). */ 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/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/Security/CWE/CWE-570/IncorrectAllocationErrorHandling.ql b/cpp/ql/src/Security/CWE/CWE-570/IncorrectAllocationErrorHandling.ql index d4d908f8474b..c03ae995090c 100644 --- a/cpp/ql/src/Security/CWE/CWE-570/IncorrectAllocationErrorHandling.ql +++ b/cpp/ql/src/Security/CWE/CWE-570/IncorrectAllocationErrorHandling.ql @@ -41,7 +41,7 @@ predicate deleteMayThrow(DeleteOrDeleteArrayExpr deleteExpr) { } /** - * Holds if the function may throw an exception when called. That is, if the body of the function looks + * Holds if the function `f` may throw an exception when called. That is, if the body of the function looks * like it might throw an exception, and the function does not have a `noexcept` or `throw()` specifier. */ predicate functionMayThrow(Function f) { diff --git a/cpp/ql/src/definitions.ql b/cpp/ql/src/definitions.ql index c12277eaf23a..339b481f5f77 100644 --- a/cpp/ql/src/definitions.ql +++ b/cpp/ql/src/definitions.ql @@ -13,6 +13,6 @@ where def = definitionOf(e, kind) and // We need to exclude definitions for elements inside template instantiations, // as these often lead to multiple links to definitions from the same source location. - // LGTM does not support this behaviour. + // LGTM does not support this behavior. not e.isFromTemplateInstantiation(_) select e, def, kind 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/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/cpp/ql/src/experimental/Security/CWE/CWE-125/DangerousWorksWithMultibyteOrWideCharacters.ql b/cpp/ql/src/experimental/Security/CWE/CWE-125/DangerousWorksWithMultibyteOrWideCharacters.ql index 6529bf6cdf89..74ac8e6da661 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-125/DangerousWorksWithMultibyteOrWideCharacters.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-125/DangerousWorksWithMultibyteOrWideCharacters.ql @@ -46,7 +46,7 @@ predicate exprMayBeString(Expr exp) { ) } -/** Holds if expression is constant or operator call `sizeof`. */ +/** Holds if expression `exp` is constant or operator call `sizeof`. */ predicate argConstOrSizeof(Expr exp) { exp.getValue().toInt() > 1 or exp.(SizeofTypeOperator).getTypeOperand().getSize() > 1 diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-243/IncorrectChangingWorkingDirectory.ql b/cpp/ql/src/experimental/Security/CWE/CWE-243/IncorrectChangingWorkingDirectory.ql index ce5f4dd00f87..9d61418fd776 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-243/IncorrectChangingWorkingDirectory.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-243/IncorrectChangingWorkingDirectory.ql @@ -15,7 +15,7 @@ import cpp import semmle.code.cpp.commons.Exclusions -/** Holds if a `fc` function call is available before or after a `chdir` function call. */ +/** Holds if a `fcp` function call is available before or after a `chdir` function call. */ predicate inExistsChdir(FunctionCall fcp) { exists(FunctionCall fctmp | ( @@ -29,7 +29,7 @@ predicate inExistsChdir(FunctionCall fcp) { ) } -/** Holds if a `fc` function call is available before or after a function call containing a `chdir` call. */ +/** Holds if a `fcp` function call is available before or after a function call containing a `chdir` call. */ predicate outExistsChdir(FunctionCall fcp) { exists(FunctionCall fctmp | exists(FunctionCall fctmp2 | 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/external/DefectFilter.qll b/cpp/ql/src/external/DefectFilter.qll index ad786e9cbc96..a3719140741a 100644 --- a/cpp/ql/src/external/DefectFilter.qll +++ b/cpp/ql/src/external/DefectFilter.qll @@ -5,8 +5,8 @@ import cpp /** * Holds if `id` in the opaque identifier of a result reported by query `queryPath`, * such that `message` is the associated message and the location of the result spans - * column `startcolumn` of line `startline` to column `endcolumn` of line `endline` - * in file `filepath`. + * column `startcol` of line `startline` to column `endcol` of line `endline` + * in file `file`. * * For more information, see [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/). */ diff --git a/cpp/ql/src/external/MetricFilter.qll b/cpp/ql/src/external/MetricFilter.qll index 0315cd23c8d0..39475451b7a8 100644 --- a/cpp/ql/src/external/MetricFilter.qll +++ b/cpp/ql/src/external/MetricFilter.qll @@ -5,8 +5,8 @@ import cpp /** * Holds if `id` in the opaque identifier of a result reported by query `queryPath`, * such that `value` is the reported metric value and the location of the result spans - * column `startcolumn` of line `startline` to column `endcolumn` of line `endline` - * in file `filepath`. + * column `startcol` of line `startline` to column `endcol` of line `endline` + * in file `file`. * * For more information, see [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/). */ 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/lib/printAst.ql b/csharp/ql/lib/printAst.ql index 380f4b4024bb..85820796a881 100644 --- a/csharp/ql/lib/printAst.ql +++ b/csharp/ql/lib/printAst.ql @@ -18,8 +18,8 @@ external string selectedSourceFile(); class PrintAstConfigurationOverride extends PrintAstConfiguration { /** - * Holds if the location matches the selected file in the VS Code extension and - * the element is `fromSource`. + * Holds if the location `l` matches the selected file in the VS Code extension and + * the element is `e` is `fromSource`. */ override predicate shouldPrint(Element e, Location l) { super.shouldPrint(e, l) and diff --git a/csharp/ql/lib/semmle/code/csharp/Assignable.qll b/csharp/ql/lib/semmle/code/csharp/Assignable.qll index d4f8d9974f08..3c7170a6f846 100644 --- a/csharp/ql/lib/semmle/code/csharp/Assignable.qll +++ b/csharp/ql/lib/semmle/code/csharp/Assignable.qll @@ -583,7 +583,7 @@ module AssignableDefinitions { } /** - * Holds if the `ref` assignment to `aa` via call `c` is uncertain. + * Holds if the `ref` assignment to `arg` via call `c` is uncertain. */ // Not in the cached module `Cached`, as that would introduce a dependency // on the CFG construction, and effectively collapse too many stages into one diff --git a/csharp/ql/lib/semmle/code/csharp/Callable.qll b/csharp/ql/lib/semmle/code/csharp/Callable.qll index 6384f4582769..7e17f853913a 100644 --- a/csharp/ql/lib/semmle/code/csharp/Callable.qll +++ b/csharp/ql/lib/semmle/code/csharp/Callable.qll @@ -708,7 +708,7 @@ class TrueOperator extends UnaryOperator { * * Either an addition operator (`AddOperator`), a checked addition operator * (`CheckedAddOperator`) a subtraction operator (`SubOperator`), a checked - * substraction operator (`CheckedSubOperator`), a multiplication operator + * subtraction operator (`CheckedSubOperator`), a multiplication operator * (`MulOperator`), a checked multiplication operator (`CheckedMulOperator`), * a division operator (`DivOperator`), a checked division operator * (`CheckedDivOperator`), a remainder operator (`RemOperator`), an and diff --git a/csharp/ql/lib/semmle/code/csharp/Member.qll b/csharp/ql/lib/semmle/code/csharp/Member.qll index 3427d4ea0893..a196d3b3fc70 100644 --- a/csharp/ql/lib/semmle/code/csharp/Member.qll +++ b/csharp/ql/lib/semmle/code/csharp/Member.qll @@ -491,7 +491,7 @@ class Parameterizable extends Declaration, @parameterizable { final Parameter getARawParameter() { result = this.getRawParameter(_) } /** - * Gets the type of the parameter, possibly prefixed + * Gets the type of the `i`th parameter, possibly prefixed * with `out`, `ref`, or `params`, where appropriate. */ private string parameterTypeToString(int i) { 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/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll index d1490c849163..397225deb8c1 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll @@ -150,7 +150,7 @@ private module SourceVariableImpl { } /** - * Gets an `out`/`ref` definition of the same source variable as the `out`/`ref` + * Gets an `out`/`ref` definition of the same source variable `v` as the `out`/`ref` * definition `def`, belonging to the same call, at a position after `def`. */ OutRefDefinition getASameOutRefDefAfter(Ssa::SourceVariable v, OutRefDefinition def) { diff --git a/csharp/ql/lib/semmle/code/csharp/security/dataflow/UnsafeDeserializationQuery.qll b/csharp/ql/lib/semmle/code/csharp/security/dataflow/UnsafeDeserializationQuery.qll index 27f6ab6935f7..5b2bd407a5ce 100644 --- a/csharp/ql/lib/semmle/code/csharp/security/dataflow/UnsafeDeserializationQuery.qll +++ b/csharp/ql/lib/semmle/code/csharp/security/dataflow/UnsafeDeserializationQuery.qll @@ -874,7 +874,7 @@ private predicate isStrongTypeFsPicklerCall(MethodCall mc, Method m) { ( m instanceof FsPicklerSerializerClassDeserializeMethod or m instanceof FsPicklerSerializerClassDeserializeSequenceMethod or - m instanceof FsPicklerSerializerClasDeserializeSiftedMethod or + m instanceof FsPicklerSerializerClassDeserializeSiftedMethod or m instanceof FsPicklerSerializerClassUnPickleMethod or m instanceof FsPicklerSerializerClassUnPickleSiftedMethod or m instanceof CsPicklerSerializerClassDeserializeMethod or diff --git a/csharp/ql/lib/semmle/code/csharp/serialization/Deserializers.qll b/csharp/ql/lib/semmle/code/csharp/serialization/Deserializers.qll index aeb341222844..f7c6ade96eaf 100644 --- a/csharp/ql/lib/semmle/code/csharp/serialization/Deserializers.qll +++ b/csharp/ql/lib/semmle/code/csharp/serialization/Deserializers.qll @@ -560,9 +560,15 @@ class FsPicklerSerializerClassDeserializeSequenceMethod extends Method, UnsafeDe } } +/** + * DEPRECATED: Use `FsPicklerSerializerClassDeserializeSiftedMethod` instead. + */ +deprecated class FsPicklerSerializerClasDeserializeSiftedMethod = + FsPicklerSerializerClassDeserializeSiftedMethod; + /** `MBrace.FsPickler.FsPicklerSerializer.DeserializeSifted` method */ -class FsPicklerSerializerClasDeserializeSiftedMethod extends Method, UnsafeDeserializer { - FsPicklerSerializerClasDeserializeSiftedMethod() { +class FsPicklerSerializerClassDeserializeSiftedMethod extends Method, UnsafeDeserializer { + FsPicklerSerializerClassDeserializeSiftedMethod() { this.getDeclaringType().getBaseClass*() instanceof FsPicklerSerializerClass and this.hasUndecoratedName("DeserializeSifted") } diff --git a/csharp/ql/src/Bad Practices/Magic Constants/MagicConstants.qll b/csharp/ql/src/Bad Practices/Magic Constants/MagicConstants.qll index 73b82c14700a..8c3e0562d395 100644 --- a/csharp/ql/src/Bad Practices/Magic Constants/MagicConstants.qll +++ b/csharp/ql/src/Bad Practices/Magic Constants/MagicConstants.qll @@ -113,7 +113,7 @@ private predicate valueOccurrenceCount(string value, int n) { n > 20 } -private predicate occurenceCount(Literal lit, string value, int n) { +private predicate occurrenceCount(Literal lit, string value, int n) { valueOccurrenceCount(value, n) and value = lit.getValue() and relevantLiteral(lit, value) @@ -127,7 +127,7 @@ private predicate check(Literal lit, string value, int n, File f) { // Check that the literal is nontrivial not trivial(lit) and // Check that it is repeated a number of times - occurenceCount(lit, value, n) and + occurrenceCount(lit, value, n) and n > 20 and f = lit.getFile() } 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/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/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/go/ql/lib/semmle/go/dataflow/internal/DataFlowUtil.qll b/go/ql/lib/semmle/go/dataflow/internal/DataFlowUtil.qll index 2cd1bbcc7476..14ff455646c9 100644 --- a/go/ql/lib/semmle/go/dataflow/internal/DataFlowUtil.qll +++ b/go/ql/lib/semmle/go/dataflow/internal/DataFlowUtil.qll @@ -367,7 +367,7 @@ module BarrierGuard { } /** - * Holds if `guard` marks a point in the control-flow graph where this node + * Holds if `guard` marks a point in the control-flow graph where `g` * is known to validate `nd`, which is represented by `ap`. * * This predicate exists to enforce a good join order in `getAGuardedNode`. @@ -378,7 +378,7 @@ module BarrierGuard { } /** - * Holds if `guard` marks a point in the control-flow graph where this node + * Holds if `guard` marks a point in the control-flow graph where `g` * is known to validate `nd`. */ private predicate guards(Node g, ControlFlow::ConditionGuardNode guard, Node nd) { 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/experimental/quantum/JCA.qll b/java/ql/lib/experimental/quantum/JCA.qll index dc86c4637505..108835c2c94e 100644 --- a/java/ql/lib/experimental/quantum/JCA.qll +++ b/java/ql/lib/experimental/quantum/JCA.qll @@ -269,7 +269,7 @@ module JCAModel { } /** - * Data-flow configuration modelling flow from a cipher string literal to a cipher algorithm consumer. + * Data-flow configuration modeling flow from a cipher string literal to a cipher algorithm consumer. */ private module CipherAlgorithmStringToCipherConsumerConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node src) { src.asExpr() instanceof CipherStringLiteral } @@ -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 } @@ -1333,7 +1331,7 @@ module JCAModel { } /** - * Data-flow configuration modelling flow from a key agreement string literal to a key agreement algorithm consumer. + * Data-flow configuration modeling flow from a key agreement string literal to a key agreement algorithm consumer. */ private module KeyAgreementAlgorithmStringToConsumerConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node src) { src.asExpr() instanceof KeyAgreementStringLiteral } @@ -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/experimental/quantum/Language.qll b/java/ql/lib/experimental/quantum/Language.qll index 975a8ad8e1fe..e203d2a15873 100644 --- a/java/ql/lib/experimental/quantum/Language.qll +++ b/java/ql/lib/experimental/quantum/Language.qll @@ -113,7 +113,7 @@ private class ConstantDataSource extends Crypto::GenericConstantSourceInstance i } /** - * An instance of random number generation, modelled as the expression + * An instance of random number generation, modeled as the expression * tied to an output node (i.e., the result of the source of randomness) */ abstract class RandomnessInstance extends Crypto::RandomNumberGenerationInstance { diff --git a/java/ql/lib/printAst.ql b/java/ql/lib/printAst.ql index cd72403de90c..8ea0efaed5f1 100644 --- a/java/ql/lib/printAst.ql +++ b/java/ql/lib/printAst.ql @@ -18,8 +18,8 @@ external string selectedSourceFile(); class PrintAstConfigurationOverride extends PrintAstConfiguration { /** - * Holds if the location matches the selected file in the VS Code extension and - * the element is `fromSource`. + * Holds if the location `l` matches the selected file in the VS Code extension and + * the element `e` is `fromSource`. */ override predicate shouldPrint(Element e, Location l) { super.shouldPrint(e, l) and diff --git a/java/ql/lib/semmle/code/java/Concurrency.qll b/java/ql/lib/semmle/code/java/Concurrency.qll index 0e510db3443b..7322f16068c0 100644 --- a/java/ql/lib/semmle/code/java/Concurrency.qll +++ b/java/ql/lib/semmle/code/java/Concurrency.qll @@ -26,7 +26,8 @@ predicate locallySynchronizedOnThis(Expr e, RefType thisType) { } /** - * Holds if `e` is synchronized by a `synchronized` modifier on the enclosing (static) method. + * Holds if `e` is synchronized by a `synchronized` modifier on the enclosing (static) method + * declared in the type `classType`. */ predicate locallySynchronizedOnClass(Expr e, RefType classType) { exists(SynchronizedCallable c | c = e.getEnclosingCallable() | 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/java/ql/lib/semmle/code/java/Statement.qll b/java/ql/lib/semmle/code/java/Statement.qll index a4872a32c914..2aea8b006ae1 100644 --- a/java/ql/lib/semmle/code/java/Statement.qll +++ b/java/ql/lib/semmle/code/java/Statement.qll @@ -291,7 +291,7 @@ class TryStmt extends Stmt, @trystmt { CatchClause getACatchClause() { result.getParent() = this } /** - * Gets the `catch` clause at the specified (zero-based) position + * Gets the `catch` clause at the specified (zero-based) position `index` * in this `try` statement. */ CatchClause getCatchClause(int index) { @@ -305,7 +305,7 @@ class TryStmt extends Stmt, @trystmt { /** Gets a resource variable declaration, if any. */ LocalVariableDeclStmt getAResourceDecl() { result.getParent() = this and result.getIndex() <= -3 } - /** Gets the resource variable declaration at the specified position in this `try` statement. */ + /** Gets the resource variable declaration at the specified position `index` in this `try` statement. */ LocalVariableDeclStmt getResourceDecl(int index) { result = this.getAResourceDecl() and index = -3 - result.getIndex() @@ -314,7 +314,7 @@ class TryStmt extends Stmt, @trystmt { /** Gets a resource expression, if any. */ VarAccess getAResourceExpr() { result.getParent() = this and result.getIndex() <= -3 } - /** Gets the resource expression at the specified position in this `try` statement. */ + /** Gets the resource expression at the specified position `index` in this `try` statement. */ VarAccess getResourceExpr(int index) { result = this.getAResourceExpr() and index = -3 - result.getIndex() @@ -323,7 +323,7 @@ class TryStmt extends Stmt, @trystmt { /** Gets a resource in this `try` statement, if any. */ ExprParent getAResource() { result = this.getAResourceDecl() or result = this.getAResourceExpr() } - /** Gets the resource at the specified position in this `try` statement. */ + /** Gets the resource at the specified position `index` in this `try` statement. */ ExprParent getResource(int index) { result = this.getResourceDecl(index) or result = this.getResourceExpr(index) } diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/ContainerFlow.qll b/java/ql/lib/semmle/code/java/dataflow/internal/ContainerFlow.qll index f93139592269..5af246424772 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/ContainerFlow.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/ContainerFlow.qll @@ -470,7 +470,8 @@ private predicate enhancedForStmtStep(Node node1, Node node2, Type containerType } /** - * Holds if the step from `node1` to `node2` reads a value from an array. + * Holds if the step from `node1` to `node2` reads a value from an array, where + * the elements are of type `elemType`. * This covers ordinary array reads as well as array iteration through enhanced * `for` statements. */ diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll index ab2f7f89cb46..4b436edc6aa6 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll @@ -263,8 +263,8 @@ class Content extends TContent { /** * Holds if this element is at the specified location. - * The location spans column `startcolumn` of line `startline` to - * column `endcolumn` of line `endline` in file `filepath`. + * The location spans column `sc` of line `sl` to + * column `ec` of line `el` in file `path`. * For more information, see * [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/). */ @@ -362,8 +362,8 @@ class ContentSet instanceof Content { /** * Holds if this element is at the specified location. - * The location spans column `startcolumn` of line `startline` to - * column `endcolumn` of line `endline` in file `filepath`. + * The location spans column `sc` of line `sl` to + * column `ec` of line `el` in file `path`. * For more information, see * [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/). */ diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/rangeanalysis/SignAnalysisSpecific.qll b/java/ql/lib/semmle/code/java/dataflow/internal/rangeanalysis/SignAnalysisSpecific.qll index 8789661af7d2..6363b8f7ed35 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/rangeanalysis/SignAnalysisSpecific.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/rangeanalysis/SignAnalysisSpecific.qll @@ -204,7 +204,7 @@ private module Impl { /** Gets the character value of expression `e`. */ string getCharValue(Expr e) { result = e.(CharacterLiteral).getValue() } - /** Gets the constant `float` value of non-`ConstantIntegerExpr` expressions. */ + /** Gets the constant `float` value of non-`ConstantIntegerExpr` expression `e`. */ float getNonIntegerValue(Expr e) { result = e.(LongLiteral).getValue().toFloat() or result = e.(FloatLiteral).getValue().toFloat() or @@ -256,12 +256,12 @@ private module Impl { exists(EnhancedForStmt for | def = for.getVariable()) } - /** Returns the operand of the operation if `def` is a decrement. */ + /** Returns the operand of the operation if `e` is a decrement. */ Expr getDecrementOperand(Element e) { result = e.(PostDecExpr).getExpr() or result = e.(PreDecExpr).getExpr() } - /** Returns the operand of the operation if `def` is an increment. */ + /** Returns the operand of the operation if `e` is an increment. */ Expr getIncrementOperand(Element e) { result = e.(PostIncExpr).getExpr() or result = e.(PreIncExpr).getExpr() } 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/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/lib/semmle/code/java/security/CleartextStorageAndroidFilesystemQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidFilesystemQuery.qll index 98acdbc1c556..898673981483 100644 --- a/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidFilesystemQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidFilesystemQuery.qll @@ -55,7 +55,7 @@ private predicate isVarargs(Argument arg, DataFlow::ImplicitVarargsArray varargs arg.isVararg() and arg.getCall() = varargs.getCall() } -/** Holds if `store` closes `file`. */ +/** Holds if `closeCall` closes `file`. */ private predicate closesFile(DataFlow::Node file, Call closeCall) { closeCall.getCallee() instanceof CloseFileMethod and if closeCall.getCallee().isStatic() diff --git a/java/ql/lib/semmle/code/java/security/FileWritable.qll b/java/ql/lib/semmle/code/java/security/FileWritable.qll index d1833bf64d4d..a5efad31bcb1 100644 --- a/java/ql/lib/semmle/code/java/security/FileWritable.qll +++ b/java/ql/lib/semmle/code/java/security/FileWritable.qll @@ -89,7 +89,7 @@ private VarAccess getFileForPathConversion(Expr pathExpr) { } /** - * Holds if `fileAccess` is used in the `setWorldWritableExpr` to set the file to be world writable. + * Holds if `fileAccess` is used in the `setWorldWritable` to set the file to be world writable. */ private predicate fileSetWorldWritable(VarAccess fileAccess, Expr setWorldWritable) { // Calls to `File.setWritable(.., false)`. diff --git a/java/ql/lib/semmle/code/java/security/TempDirUtils.qll b/java/ql/lib/semmle/code/java/security/TempDirUtils.qll index 3d1623fa334c..21e289ef1ade 100644 --- a/java/ql/lib/semmle/code/java/security/TempDirUtils.qll +++ b/java/ql/lib/semmle/code/java/security/TempDirUtils.qll @@ -26,7 +26,8 @@ class MethodFileCreateTempFile extends Method { } /** - * Holds if `expDest` is some constructor call `new java.io.File(expSource)`, where the specific `File` constructor being used has `paramCount` parameters. + * Holds if `expSource` is an argument to a constructor call `exprDest` (constructor from `java.io.File`), where + * the specific `File` constructor being used has `paramCount` parameters. */ predicate isFileConstructorArgument(Expr expSource, Expr exprDest, int paramCount) { exists(ConstructorCall construtorCall | diff --git a/java/ql/lib/semmle/code/java/security/UnsafeDeserializationQuery.qll b/java/ql/lib/semmle/code/java/security/UnsafeDeserializationQuery.qll index ce0f649eff35..f5968898adcf 100644 --- a/java/ql/lib/semmle/code/java/security/UnsafeDeserializationQuery.qll +++ b/java/ql/lib/semmle/code/java/security/UnsafeDeserializationQuery.qll @@ -552,7 +552,7 @@ private DataFlow::Node getASafelyConfiguredParser() { } /** - * Holds if `parseMethodQualifierExpr` is a `jodd.json.JsonParser` instance that is configured unsafely + * Holds if `parserExpr` is a `jodd.json.JsonParser` instance that is configured unsafely * and which never appears to be configured safely. */ private predicate joddJsonParserConfiguredUnsafely(Expr parserExpr) { diff --git a/java/ql/src/Likely Bugs/Comparison/HashedButNoHash.ql b/java/ql/src/Likely Bugs/Comparison/HashedButNoHash.ql index f8b1cd85552a..bb370bead3b5 100644 --- a/java/ql/src/Likely Bugs/Comparison/HashedButNoHash.ql +++ b/java/ql/src/Likely Bugs/Comparison/HashedButNoHash.ql @@ -14,7 +14,7 @@ import java import Equality -/** A class that defines an `equals` method but no `hashCode` method. */ +/** Holds if `c` defines an `equals` method but no `hashCode` method. */ predicate eqNoHash(Class c) { exists(Method m | m = c.getAMethod() | m instanceof EqualsMethod and diff --git a/java/ql/src/Likely Bugs/Concurrency/NotifyWithoutSynch.ql b/java/ql/src/Likely Bugs/Concurrency/NotifyWithoutSynch.ql index 89dbedd02532..48f547575eea 100644 --- a/java/ql/src/Likely Bugs/Concurrency/NotifyWithoutSynch.ql +++ b/java/ql/src/Likely Bugs/Concurrency/NotifyWithoutSynch.ql @@ -78,7 +78,7 @@ private predicate synchronizedThisAccess(MethodCall ma, Type thisType) { /** * Auxiliary predicate for `unsynchronizedVarAccess`. Holds if - * there is an enclosing `synchronized` statement on the variable. + * there is an enclosing `synchronized` statement on the variable access `x`. */ predicate synchronizedVarAccess(VarAccess x) { exists(SynchronizedStmt s, VarAccess y | 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 diff --git a/java/ql/src/Violations of Best Practice/Magic Constants/MagicConstants.qll b/java/ql/src/Violations of Best Practice/Magic Constants/MagicConstants.qll index dc5b4eada8d9..c94a4fb58bf2 100644 --- a/java/ql/src/Violations of Best Practice/Magic Constants/MagicConstants.qll +++ b/java/ql/src/Violations of Best Practice/Magic Constants/MagicConstants.qll @@ -105,7 +105,7 @@ private predicate valueOccurrenceCount(string value, int n, string context) { n > 20 } -private predicate occurenceCount(Literal lit, string value, int n, string context) { +private predicate occurrenceCount(Literal lit, string value, int n, string context) { valueOccurrenceCount(value, n, context) and value = lit.getValue() and nonTrivialValue(_, lit, context) @@ -119,7 +119,7 @@ private predicate check(Literal lit, string value, int n, string context, Compil // Check that the literal is nontrivial not trivial(lit) and // Check that it is repeated a number of times - occurenceCount(lit, value, n, context) and + occurrenceCount(lit, value, n, context) and n > 20 and f = lit.getCompilationUnit() } diff --git a/java/ql/src/Violations of Best Practice/Naming Conventions/ConfusingOverloading.ql b/java/ql/src/Violations of Best Practice/Naming Conventions/ConfusingOverloading.ql index ca002fc654a6..ed492e5b6a1e 100644 --- a/java/ql/src/Violations of Best Practice/Naming Conventions/ConfusingOverloading.ql +++ b/java/ql/src/Violations of Best Practice/Naming Conventions/ConfusingOverloading.ql @@ -48,7 +48,7 @@ private predicate overloadedMethodsMostSpecific(Method n, Method m) { private predicate whitelist(string name) { name = "visit" } /** - * Method `m` has name `name`, number of parameters `numParams` + * Method `m` has name `name`, number of parameters `numParam` * and is declared in `t` or inherited from a supertype of `t`. */ pragma[nomagic] 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/java/ql/src/experimental/quantum/Examples/BrokenCrypto.ql b/java/ql/src/experimental/quantum/Examples/BrokenCrypto.ql index 7291ea554a58..1aee95152328 100644 --- a/java/ql/src/experimental/quantum/Examples/BrokenCrypto.ql +++ b/java/ql/src/experimental/quantum/Examples/BrokenCrypto.ql @@ -12,7 +12,7 @@ */ //THIS QUERY IS A REPLICA OF: https://github.com/github/codeql/blob/main/java/ql/src/Security/CWE/CWE-327/BrokenCryptoAlgorithm.ql -//but uses the **NEW MODELLING** +//but uses the **NEW MODELING** import experimental.quantum.Language /** diff --git a/java/ql/src/utils/flowtestcasegenerator/FlowTestCase.qll b/java/ql/src/utils/flowtestcasegenerator/FlowTestCase.qll index af0f501621a1..5ab1584f8668 100644 --- a/java/ql/src/utils/flowtestcasegenerator/FlowTestCase.qll +++ b/java/ql/src/utils/flowtestcasegenerator/FlowTestCase.qll @@ -253,7 +253,7 @@ class TestCase extends TTestCase { /** * Returns a call to `source()` wrapped in `newWith` methods as needed according to `input`. - * For example, if the input specification is `ArrayElement of MapValue of Argument[0]`, this + * For example, if the `stack` is `Argument[0].MapValue.ArrayElement`, this * will return `newWithMapValue(newWithArrayElement(source()))`. */ string getInput(SummaryComponentStack stack) { @@ -269,7 +269,7 @@ class TestCase extends TTestCase { /** * Returns `out` wrapped in `get` methods as needed according to `output`. - * For example, if the output specification is `ArrayElement of MapValue of Argument[0]`, this + * For example, if the `componentStack` is `Argument[0].MapValue.ArrayElement`, this * will return `getArrayElement(getMapValue(out))`. */ string getOutput(SummaryComponentStack componentStack) { diff --git a/javascript/ql/lib/semmle/javascript/dataflow/FlowSummary.qll b/javascript/ql/lib/semmle/javascript/dataflow/FlowSummary.qll index c4a6e12b2104..eb7160683a76 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/FlowSummary.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/FlowSummary.qll @@ -17,10 +17,10 @@ private import semmle.javascript.dataflow.internal.DataFlowPrivate * * - The relevant call sites cannot be matched by the access path syntax, and require the full power of CodeQL. * For example, complex overloading patterns might require more local reasoning at the call site. - * - The input/output behaviour cannot be described statically in the access path syntax, but the relevant access paths + * - The input/output behavior cannot be described statically in the access path syntax, but the relevant access paths * can be generated dynamically in CodeQL, based on the usages found in the codebase. * - * Subclasses should bind `this` to a unique identifier for the function being modelled. There is no special + * Subclasses should bind `this` to a unique identifier for the function being modeled. There is no special * interpreation of the `this` value, it should just not clash with the `this`-value used by other classes. * * For example, this models flow through calls such as `require("my-library").myFunction()`: 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/dataflow/internal/DataFlowPrivate.qll b/javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowPrivate.qll index 2fcc2acbd167..b7e955b94198 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowPrivate.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowPrivate.qll @@ -680,7 +680,7 @@ predicate neverSkipInPathGraph(Node node) { // Include the return-value expression node.asExpr() = any(Function f).getAReturnedExpr() or - // Include calls (which may have been modelled as steps) + // Include calls (which may have been modeled as steps) node.asExpr() instanceof InvokeExpr or // Include references to a variable @@ -1159,7 +1159,7 @@ private predicate legacyBarrier(DataFlow::Node node) { pragma[nomagic] private predicate isBlockedLegacyNode(Node node) { // Ignore captured variable nodes for those variables that are handled by the captured-variable library. - // Note that some variables, such as top-level variables, are still modelled with these nodes (which will result in jump steps). + // Note that some variables, such as top-level variables, are still modeled with these nodes (which will result in jump steps). exists(LocalVariable variable | node = TCapturedVariableNode(variable) and variable = any(VariableCaptureConfig::CapturedVariable v).asLocalVariable() diff --git a/javascript/ql/lib/semmle/javascript/dataflow/internal/VariableCapture.qll b/javascript/ql/lib/semmle/javascript/dataflow/internal/VariableCapture.qll index 75f21bab38ac..b3182dfc7b8e 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/internal/VariableCapture.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/internal/VariableCapture.qll @@ -172,7 +172,7 @@ module VariableCaptureConfig implements InputSig { predicate hasCfgNode(BasicBlock bb, int i) { none() } // Overridden in subclass - // note: langauge-specific + // note: language-specific js::DataFlow::Node getSource() { none() } // Overridden in subclass } 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/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsExtensions.qll b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsExtensions.qll index b86d7de457ee..49daf5c60489 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsExtensions.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsExtensions.qll @@ -4,7 +4,7 @@ /** * Holds if the value at `(type, path)` should be seen as a flow - * source of the given `kind`. + * source of the given `kind`. The `madId` is the data extension row number. * * The kind `remote` represents a general remote flow source. */ @@ -14,13 +14,13 @@ extensible predicate sourceModel( /** * Holds if the value at `(type, path)` should be seen as a sink - * of the given `kind`. + * of the given `kind`. The `madId` is the data extension row number. */ extensible predicate sinkModel(string type, string path, string kind, QlBuiltins::ExtensionId madId); /** * Holds if in calls to `(type, path)`, the value referred to by `input` - * can flow to the value referred to by `output`. + * can flow to the value referred to by `output`. The `madId` is the data extension row number. * * `kind` should be either `value` or `taint`, for value-preserving or taint-preserving steps, * respectively. diff --git a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsSpecific.qll b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsSpecific.qll index c61ecc138ef6..f0d751ad31b6 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsSpecific.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsSpecific.qll @@ -35,7 +35,7 @@ class Location = JS::Location; * Type names have form `package.type` or just `package` if referring to the package export * object. If `package` contains a `.` character it must be enclosed in single quotes, such as `'package'.type`. * - * A type name of form `(package)` may also be used when refering to the package export object. + * A type name of form `(package)` may also be used when referring to the package export object. * We allow this syntax as an alternative to the above, so models generated based on `EndpointNaming` look more consistent. * However, access paths are deliberately not parsed here, as we can not handle aliasing at this stage. * The model generator must explicitly generate the step between `(package)` and `(package).foo`, for example. diff --git a/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Arrays.qll b/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Arrays.qll index 00fed9c4f093..b56968377816 100644 --- a/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Arrays.qll +++ b/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Arrays.qll @@ -1,7 +1,7 @@ /** * Contains a summary for relevant methods on arrays. * - * Note that some of Array methods are modelled in `AmbiguousCoreMethods.qll`, and `toString` is special-cased elsewhere. + * Note that some of Array methods are modeled in `AmbiguousCoreMethods.qll`, and `toString` is special-cased elsewhere. */ private import javascript @@ -60,7 +60,7 @@ private predicate isForLoopVariable(Variable v) { private predicate isLikelyArrayIndex(Expr e) { // Require that 'e' is of type number and refers to a for-loop variable. - // TODO: This is here to mirror the old behaviour. Experiment with turning the 'and' into an 'or'. + // TODO: This is here to mirror the old behavior. Experiment with turning the 'and' into an 'or'. TTNumber() = unique(InferredType type | type = e.flow().analyze().getAType()) and isForLoopVariable(e.(VarAccess).getVariable()) or @@ -114,7 +114,7 @@ class ArrayConstructorSummary extends SummarizedCallable { /** * A call to `join` with a separator argument. * - * Calls without separators are modelled in `StringConcatenation.qll`. + * Calls without separators are modeled in `StringConcatenation.qll`. */ class Join extends SummarizedCallable { Join() { this = "Array#join" } diff --git a/javascript/ql/lib/semmle/javascript/internal/flow_summaries/AsyncAwait.qll b/javascript/ql/lib/semmle/javascript/internal/flow_summaries/AsyncAwait.qll index a39b0e6f43d7..246ac0f19d08 100644 --- a/javascript/ql/lib/semmle/javascript/internal/flow_summaries/AsyncAwait.qll +++ b/javascript/ql/lib/semmle/javascript/internal/flow_summaries/AsyncAwait.qll @@ -8,7 +8,7 @@ private import semmle.javascript.dataflow.internal.AdditionalFlowInternal private import semmle.javascript.dataflow.internal.DataFlowPrivate /** - * Steps modelling flow in an `async` function. + * Steps modeling flow in an `async` function. * * Note about promise-coercion and flattening: * - `await` preserves non-promise values, e.g. `await "foo"` is just `"foo"`. diff --git a/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Generators.qll b/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Generators.qll index e187b5751cfd..75815d00341d 100644 --- a/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Generators.qll +++ b/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Generators.qll @@ -7,7 +7,7 @@ private import semmle.javascript.dataflow.internal.DataFlowNode private import semmle.javascript.dataflow.internal.AdditionalFlowInternal /** - * Steps modelling flow out of a generator function: + * Steps modeling flow out of a generator function: * ```js * function* foo() { * yield x; // store 'x' in the return value's IteratorElement diff --git a/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Iterators.qll b/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Iterators.qll index e9937363c01d..6b1a182a49bd 100644 --- a/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Iterators.qll +++ b/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Iterators.qll @@ -1,5 +1,5 @@ /** - * Contains flow summaries and steps modelling flow through iterators. + * Contains flow summaries and steps modeling flow through iterators. */ private import javascript diff --git a/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Maps.qll b/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Maps.qll index 61cc1d148c6b..d9649d407c61 100644 --- a/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Maps.qll +++ b/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Maps.qll @@ -1,5 +1,5 @@ /** - * Contains flow summaries and steps modelling flow through `Map` objects. + * Contains flow summaries and steps modeling flow through `Map` objects. */ private import javascript diff --git a/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Promises.qll b/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Promises.qll index 1122c38320a5..7587ab11dc47 100644 --- a/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Promises.qll +++ b/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Promises.qll @@ -1,5 +1,5 @@ /** - * Contains flow summaries and steps modelling flow through `Promise` objects. + * Contains flow summaries and steps modeling flow through `Promise` objects. */ private import javascript diff --git a/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Sets.qll b/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Sets.qll index 34f7d222df8b..6b4f089b38ec 100644 --- a/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Sets.qll +++ b/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Sets.qll @@ -1,5 +1,5 @@ /** - * Contains flow summaries and steps modelling flow through `Set` objects. + * Contains flow summaries and steps modeling flow through `Set` objects. */ private import javascript diff --git a/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Strings.qll b/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Strings.qll index d18e21819653..bf9442219a75 100644 --- a/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Strings.qll +++ b/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Strings.qll @@ -1,5 +1,5 @@ /** - * Contains flow summaries and steps modelling flow through string methods. + * Contains flow summaries and steps modeling flow through string methods. */ private import javascript @@ -73,7 +73,7 @@ class StringSplit extends SummarizedCallable { * These are of special significance when tracking a tainted URL suffix, such as `window.location.href`, * because the first element of the resulting array should not be considered tainted. * - * This summary defaults to the same behaviour as the general `.split()` case, but it contains optional steps + * This summary defaults to the same behavior as the general `.split()` case, but it contains optional steps * and barriers named `tainted-url-suffix` that should be activated when tracking a tainted URL suffix. */ class StringSplitHashOrQuestionMark extends SummarizedCallable { diff --git a/javascript/ql/lib/semmle/javascript/security/TaintedUrlSuffixCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/TaintedUrlSuffixCustomizations.qll index d4bce73be197..841f830f2bf9 100644 --- a/javascript/ql/lib/semmle/javascript/security/TaintedUrlSuffixCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/TaintedUrlSuffixCustomizations.qll @@ -76,7 +76,7 @@ module TaintedUrlSuffix { // // x [tainted-url-suffix] --> x.split('#') [array element 1] [taint] // - // Technically we should also preverse tainted-url-suffix when entering the first array element of such + // Technically we should also preserve tainted-url-suffix when entering the first array element of such // a split, but this mostly leads to FPs since we currently don't track if the taint has been through URI-decoding. // (The query/fragment parts are often URI-decoded in practice, but not the other URL parts are not) state1.isTaintedUrlSuffix() and 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/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/lib/analysis/DefinitionTracking.qll b/python/ql/lib/analysis/DefinitionTracking.qll index 5a9811f62488..e015d0f70a97 100644 --- a/python/ql/lib/analysis/DefinitionTracking.qll +++ b/python/ql/lib/analysis/DefinitionTracking.qll @@ -83,7 +83,7 @@ private predicate ssa_phi_defn(PhiFunction phi, Definition defn) { ssa_variable_defn(phi.getAnInput(), defn) } -/** Holds if the ESSA defn `def` refers to (`value`, `cls`, `origin`) given the context `context`. */ +/** Holds if the ESSA defn `def` refers to (`value`, `cls`, `origin`) given the context `context`. */ private predicate ssa_defn_defn(EssaDefinition def, Definition defn) { ssa_phi_defn(def, defn) or diff --git a/python/ql/lib/experimental/cryptography/CryptoArtifact.qll b/python/ql/lib/experimental/cryptography/CryptoArtifact.qll index fc5c75a4e441..e8939c981113 100644 --- a/python/ql/lib/experimental/cryptography/CryptoArtifact.qll +++ b/python/ql/lib/experimental/cryptography/CryptoArtifact.qll @@ -95,7 +95,7 @@ abstract class CryptographicAlgorithm extends CryptographicArtifact { /** * Normalizes a raw name into a normalized name as found in `CryptoAlgorithmNames.qll`. * Subclassess should override for more api-specific normalization. - * By deafult, converts a raw name to upper-case with no hyphen, underscore, hash, or space. + * By default, converts a raw name to upper-case with no hyphen, underscore, hash, or space. */ bindingset[s] string normalizeName(string s) { 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/python/ql/lib/experimental/cryptography/modules/stdlib/HashlibModule.qll b/python/ql/lib/experimental/cryptography/modules/stdlib/HashlibModule.qll index 5b2586dc54a6..346512e9a2db 100644 --- a/python/ql/lib/experimental/cryptography/modules/stdlib/HashlibModule.qll +++ b/python/ql/lib/experimental/cryptography/modules/stdlib/HashlibModule.qll @@ -201,7 +201,7 @@ module KDF { // TODO: better modeling of scrypt /** - * Identifies key derivation fucntion hashlib.scrypt accesses. + * Identifies key derivation function hashlib.scrypt accesses. */ class HashlibScryptAlgorithm extends KeyDerivationAlgorithm, KeyDerivationOperation { HashlibScryptAlgorithm() { this = API::moduleImport("hashlib").getMember("scrypt").getACall() } diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll index b29be706c4fc..724ae82aa0dd 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll @@ -613,7 +613,7 @@ DataFlowType getNodeType(Node node) { // Extra flow //-------- /** - * Holds if `pred` can flow to `succ`, by jumping from one callable to + * Holds if `nodeFrom` can flow to `nodeTo`, by jumping from one callable to * another. Additional steps specified by the configuration are *not* * taken into account. */ @@ -634,7 +634,7 @@ predicate jumpStep(Node nodeFrom, Node nodeTo) { * the type-trackers as well, as that would make evaluation of type-tracking recursive * with the new jumpsteps. * - * Holds if `pred` can flow to `succ`, by jumping from one callable to + * Holds if `nodeFrom` can flow to `nodeTo`, by jumping from one callable to * another. Additional steps specified by the configuration are *not* * taken into account. */ @@ -657,7 +657,7 @@ predicate jumpStepSharedWithTypeTracker(Node nodeFrom, Node nodeTo) { * the type-trackers as well, as that would make evaluation of type-tracking recursive * with the new jumpsteps. * - * Holds if `pred` can flow to `succ`, by jumping from one callable to + * Holds if `nodeFrom` can flow to `nodeTo`, by jumping from one callable to * another. Additional steps specified by the configuration are *not* * taken into account. */ @@ -766,7 +766,7 @@ module Orm { abstract predicate storeStep(Node nodeFrom, Content c, Node nodeTo); /** - * Holds if `pred` can flow to `succ`, by jumping from one callable to + * Holds if `nodeFrom` can flow to `nodeTo`, by jumping from one callable to * another. Additional steps specified by the configuration are *not* * taken into account. */ 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/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModelsExtensions.qll b/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModelsExtensions.qll index b86d7de457ee..49daf5c60489 100644 --- a/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModelsExtensions.qll +++ b/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModelsExtensions.qll @@ -4,7 +4,7 @@ /** * Holds if the value at `(type, path)` should be seen as a flow - * source of the given `kind`. + * source of the given `kind`. The `madId` is the data extension row number. * * The kind `remote` represents a general remote flow source. */ @@ -14,13 +14,13 @@ extensible predicate sourceModel( /** * Holds if the value at `(type, path)` should be seen as a sink - * of the given `kind`. + * of the given `kind`. The `madId` is the data extension row number. */ extensible predicate sinkModel(string type, string path, string kind, QlBuiltins::ExtensionId madId); /** * Holds if in calls to `(type, path)`, the value referred to by `input` - * can flow to the value referred to by `output`. + * can flow to the value referred to by `output`. The `madId` is the data extension row number. * * `kind` should be either `value` or `taint`, for value-preserving or taint-preserving steps, * respectively. diff --git a/python/ql/lib/semmle/python/objects/ObjectInternal.qll b/python/ql/lib/semmle/python/objects/ObjectInternal.qll index a58b8b5f0a91..aa78caa2c9d7 100644 --- a/python/ql/lib/semmle/python/objects/ObjectInternal.qll +++ b/python/ql/lib/semmle/python/objects/ObjectInternal.qll @@ -174,9 +174,9 @@ class ObjectInternal extends TObject { abstract int length(); /** - * Holds if the object `function` is called when this object is called and `paramOffset` + * Holds if the object `function` is called when this object is called and `offset` * is the difference from the parameter position and the argument position. - * For a normal function `paramOffset` is 0. For classes and bound-methods it is 1. + * For a normal function `offset` is 0. For classes and bound-methods it is 1. * This is used to implement the `CallableValue` public API. */ predicate functionAndOffset(CallableObjectInternal function, int offset) { none() } diff --git a/python/ql/lib/semmle/python/types/FunctionObject.qll b/python/ql/lib/semmle/python/types/FunctionObject.qll index d52a885a832c..f64c02b9c6bf 100644 --- a/python/ql/lib/semmle/python/types/FunctionObject.qll +++ b/python/ql/lib/semmle/python/types/FunctionObject.qll @@ -46,9 +46,7 @@ abstract class FunctionObject extends Object { ControlFlowNode getACall() { result = this.theCallable().getACall() } /** Gets a call-site from where this function is called, given the `context` */ - ControlFlowNode getACall(Context caller_context) { - result = this.theCallable().getACall(caller_context) - } + ControlFlowNode getACall(Context context) { result = this.theCallable().getACall(context) } /** * Gets the `ControlFlowNode` that will be passed as the nth argument to `this` when called at `call`. 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/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 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/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/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/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. | 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/ApiGraphs.qll b/ruby/ql/lib/codeql/ruby/ApiGraphs.qll index 00537e375b1b..a74b0e081995 100644 --- a/ruby/ql/lib/codeql/ruby/ApiGraphs.qll +++ b/ruby/ql/lib/codeql/ruby/ApiGraphs.qll @@ -1,5 +1,5 @@ /** - * Provides an implementation of _API graphs_, which allow efficient modelling of how a given + * Provides an implementation of _API graphs_, which allow efficient modeling of how a given * value is used by the code base or how values produced by the code base are consumed by a library. * * See `API::Node` for more details. diff --git a/ruby/ql/lib/codeql/ruby/frameworks/core/Kernel.qll b/ruby/ql/lib/codeql/ruby/frameworks/core/Kernel.qll index 2746faebdc86..cef6cb4fa043 100644 --- a/ruby/ql/lib/codeql/ruby/frameworks/core/Kernel.qll +++ b/ruby/ql/lib/codeql/ruby/frameworks/core/Kernel.qll @@ -50,7 +50,7 @@ module Kernel { } /** - * Private methods in the `Kernel` module. + * Holds if `method` is a name of a private method in the `Kernel` module. * These can be be invoked on `self`, on `Kernel`, or using a low-level primitive like `send` or `instance_eval`. * ```ruby * puts "hello world" 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 b86d7de457ee..49daf5c60489 100644 --- a/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModelsExtensions.qll +++ b/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModelsExtensions.qll @@ -4,7 +4,7 @@ /** * Holds if the value at `(type, path)` should be seen as a flow - * source of the given `kind`. + * source of the given `kind`. The `madId` is the data extension row number. * * The kind `remote` represents a general remote flow source. */ @@ -14,13 +14,13 @@ extensible predicate sourceModel( /** * Holds if the value at `(type, path)` should be seen as a sink - * of the given `kind`. + * of the given `kind`. The `madId` is the data extension row number. */ extensible predicate sinkModel(string type, string path, string kind, QlBuiltins::ExtensionId madId); /** * Holds if in calls to `(type, path)`, the value referred to by `input` - * can flow to the value referred to by `output`. + * can flow to the value referred to by `output`. The `madId` is the data extension row number. * * `kind` should be either `value` or `taint`, for value-preserving or taint-preserving steps, * respectively. 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/ruby/ql/lib/codeql/ruby/regexp/internal/ParseRegExp.qll b/ruby/ql/lib/codeql/ruby/regexp/internal/ParseRegExp.qll index d1f96ec407ee..d35d9353bf13 100644 --- a/ruby/ql/lib/codeql/ruby/regexp/internal/ParseRegExp.qll +++ b/ruby/ql/lib/codeql/ruby/regexp/internal/ParseRegExp.qll @@ -194,7 +194,7 @@ abstract class RegExp extends Ast::StringlikeLiteral { } /** - * Holds if the character set starting at `charset_start` contains a character range + * Holds if the character set starting at `charsetStart` contains a character range * with lower bound found between `start` and `lowerEnd` * and upper bound found between `upperStart` and `end`. */ diff --git a/ruby/ql/lib/codeql/ruby/security/ImproperMemoizationQuery.qll b/ruby/ql/lib/codeql/ruby/security/ImproperMemoizationQuery.qll index 46fc231c6fc6..dab75f00b9e5 100644 --- a/ruby/ql/lib/codeql/ruby/security/ImproperMemoizationQuery.qll +++ b/ruby/ql/lib/codeql/ruby/security/ImproperMemoizationQuery.qll @@ -45,7 +45,7 @@ private class MemoCandidate extends Method { } /** - * Holds if parameter `p` of `m` is read in the right hand side of `assign`. + * Holds if parameter `p` of `m` is read in the right hand side of `a`. */ private predicate parameterUsedInMemoValue(Method m, Parameter p, MemoStmt a) { p = m.getAParameter() and @@ -54,7 +54,7 @@ private predicate parameterUsedInMemoValue(Method m, Parameter p, MemoStmt a) { } /** - * Holds if parameter `p` of `m` is read in the left hand side of `assign`. + * Holds if parameter `p` of `m` is read in the left hand side of `a`. */ private predicate parameterUsedInMemoKey(Method m, Parameter p, HashMemoStmt a) { p = m.getAParameter() and diff --git a/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll b/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll index 727f14bb94ab..47ae294492eb 100644 --- a/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll +++ b/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll @@ -196,7 +196,8 @@ private ExprCfgNode getALastEvalNode(ExprCfgNode e) { /** * Holds if a reverse local flow step should be added from the post-update node - * for `e` to the post-update node for the result. + * for `e` to the post-update node for the result. `preservesValue` is true + * if the step is value preserving. * * This is needed to allow for side-effects on compound expressions to propagate * to sub components. For example, in diff --git a/rust/ql/lib/codeql/rust/dataflow/internal/ModelsAsData.qll b/rust/ql/lib/codeql/rust/dataflow/internal/ModelsAsData.qll index 7f8df8d144ba..e5f6f09e17ad 100644 --- a/rust/ql/lib/codeql/rust/dataflow/internal/ModelsAsData.qll +++ b/rust/ql/lib/codeql/rust/dataflow/internal/ModelsAsData.qll @@ -52,6 +52,7 @@ private import codeql.rust.elements.internal.CallExprBaseImpl::Impl as CallExprB /** * Holds if in a call to the function with canonical path `path`, the value referred * to by `output` is a flow source of the given `kind`. + * The `madId` is the data extension row number. * * `output = "ReturnValue"` simply means the result of the call itself. * @@ -65,6 +66,7 @@ extensible predicate sourceModel( /** * Holds if in a call to the function with canonical path `path`, the value referred * to by `input` is a flow sink of the given `kind`. + * The `madId` is the data extension row number. * * For example, `input = Argument[0]` means the first argument of the call. * @@ -78,6 +80,7 @@ extensible predicate sinkModel( /** * Holds if in a call to the function with canonical path `path`, the value referred * to by `input` can flow to the value referred to by `output`. + * The `madId` is the data extension row number. * * `kind` should be either `value` or `taint`, for value-preserving or taint-preserving * steps, respectively. 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/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 } diff --git a/shared/quantum/codeql/quantum/experimental/Model.qll b/shared/quantum/codeql/quantum/experimental/Model.qll index d8b1402b5e88..d97b6af2b990 100644 --- a/shared/quantum/codeql/quantum/experimental/Model.qll +++ b/shared/quantum/codeql/quantum/experimental/Model.qll @@ -365,7 +365,7 @@ module CryptographyBase Input> { */ abstract class ArtifactConsumer extends ConsumerElement { /** - * Use `getAKnownArtifactSource() instead. The behaviour of these two predicates is equivalent. + * Use `getAKnownArtifactSource() instead. The behavior of these two predicates is equivalent. */ final override KnownElement getAKnownSource() { result = this.getAKnownArtifactSource() } @@ -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 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 diff --git a/swift/ql/lib/codeql/swift/elements/decl/internal/EnumDeclImpl.qll b/swift/ql/lib/codeql/swift/elements/decl/internal/EnumDeclImpl.qll index 9eff4077f933..3410ae305194 100644 --- a/swift/ql/lib/codeql/swift/elements/decl/internal/EnumDeclImpl.qll +++ b/swift/ql/lib/codeql/swift/elements/decl/internal/EnumDeclImpl.qll @@ -7,7 +7,7 @@ module Impl { /** * An enumeration declaration, for example: * ``` - * enum MyColours { + * enum MyColors { * case red * case green * case blue