Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 7 additions & 9 deletions actions/ql/lib/codeql/actions/ast/internal/Ast.qll
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

/**
Expand Down Expand Up @@ -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)
)
}
}
Expand Down
8 changes: 4 additions & 4 deletions actions/ql/lib/codeql/actions/dataflow/ExternalFlow.qll
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
)
)
}
Expand Down
15 changes: 7 additions & 8 deletions actions/ql/lib/codeql/actions/dataflow/FlowSources.qll
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -148,7 +148,6 @@ class GhCLICommandSource extends RemoteFlowSource, CommandSource {
class GitHubEventPathSource extends RemoteFlowSource, CommandSource {
string cmd;
string flag;
string access_path;
Run run;

// Examples
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ abstract class ArgumentInjectionSink extends DataFlow::Node {
*/
class ArgumentInjectionFromEnvVarSink extends ArgumentInjectionSink {
string command;
string argument;

ArgumentInjectionFromEnvVarSink() {
exists(Run run, string var |
Expand All @@ -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, _)
)
}

Expand All @@ -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, _)
)
}

Expand Down
47 changes: 24 additions & 23 deletions actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,6 @@ class LegitLabsDownloadArtifactActionStep extends UntrustedArtifactDownloadStep,
}

class ActionsGitHubScriptDownloadStep extends UntrustedArtifactDownloadStep, UsesStep {
string script;

ActionsGitHubScriptDownloadStep() {
// eg:
// - uses: actions/github-script@v6
Expand All @@ -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() {
Expand All @@ -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/"
)
}
}

Expand Down Expand Up @@ -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/"
)
}
}

Expand Down Expand Up @@ -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
Expand Down
5 changes: 1 addition & 4 deletions actions/ql/lib/codeql/actions/security/ControlChecks.qll
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 2 additions & 10 deletions actions/ql/lib/codeql/actions/security/EnvVarInjectionQuery.qll
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
)
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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())
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
4 changes: 2 additions & 2 deletions cpp/ql/lib/experimental/cryptography/modules/OpenSSL.qll
Original file line number Diff line number Diff line change
Expand Up @@ -652,14 +652,14 @@
* 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 {

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase. Warning

Acronyms in EVP_PKEY_CTX_Ptr_Source_to_KeyGenOperationWithNoSizeConfig should be PascalCase/camelCase.
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<EVP_PKEY_CTX_Ptr_Source_to_KeyGenOperationWithNoSize>;
DataFlow::Global<EVP_PKEY_CTX_Ptr_Source_to_KeyGenOperationWithNoSizeConfig>;

/**
* UNKNOWN key sizes to general purpose key generation functions (i.e., that take in no key size and assume
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -378,12 +378,12 @@ module BoostorgAsio {
* Constructs a standard data flow computation for protocol values to the first argument
* of a context constructor.
*/
module SslContextCallGlobal<SslContextCallConfigSig Config> {
private module C implements DataFlow::ConfigSig {
import Config
module SslContextCallGlobal<SslContextCallConfigSig SslConfig> {
private module Config implements DataFlow::ConfigSig {
import SslConfig
}

import DataFlow::Global<C>
import DataFlow::Global<Config>
}

/**
Expand Down
2 changes: 1 addition & 1 deletion cpp/ql/lib/semmle/code/cpp/valuenumbering/HashCons.qll
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
8 changes: 4 additions & 4 deletions cpp/ql/src/Security/CWE/CWE-190/IntegerOverflowTainted.ql
Original file line number Diff line number Diff line change
Expand Up @@ -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() }
Expand Down
Loading
Loading