diff --git a/ql/lib/codeql/actions/Ast.qll b/ql/lib/codeql/actions/Ast.qll index 2bbf5c8ac0d..881daf13336 100644 --- a/ql/lib/codeql/actions/Ast.qll +++ b/ql/lib/codeql/actions/Ast.qll @@ -2,7 +2,7 @@ private import codeql.actions.ast.internal.Actions private import codeql.Locations /** - * Base class for the AST tree. Based on YamlNode from the Yaml library. + * Base class for thejAST tree. Based on YamlNode from the Yaml library. */ class AstNode instanceof YamlNode { AstNode getParentNode() { result = super.getParentNode() } @@ -14,20 +14,16 @@ class AstNode instanceof YamlNode { string getAPrimaryQlClass() { result = super.getAPrimaryQlClass() } Location getLocation() { result = super.getLocation() } -} - -/** - * A statement is a group of expressions and/or statements that you design to carry out a task or an action. - * Any statement that can return a value is automatically qualified to be used as an expression. - */ -class Statement extends AstNode { - /** Gets the workflow that this job is a part of. */ - WorkflowStmt getEnclosingWorkflowStmt() { this = result.getAChildNode*() } /** - * Gets a environment variable expression by name in the scope of the current step. + * Gets the enclosing workflow statement. */ - Expression getEnvExpr(string name) { + Workflow getEnclosingWorkflow() { this = result.getAChildNode*() } + + /** + * Gets a environment variable expression by name in the scope of the current node. + */ + EnvExpr getEnvExpr(string name) { exists(Actions::Env env | env.(YamlMapping).maps(any(YamlScalar s | s.getValue() = name), result) | @@ -40,37 +36,32 @@ class Statement extends AstNode { } } -/** - * An expression is any word or group of words or symbols that is a value. In programming, an expression is a value, or anything that executes and ends up being a value. - */ -class Expression extends Statement { } - /** * A composite action */ -class CompositeActionStmt extends Statement instanceof Actions::CompositeAction { - RunsStmt getRunsStmt() { result = super.getRuns() } +class CompositeAction extends AstNode instanceof Actions::CompositeAction { + Runs getRuns() { result = super.getRuns() } - InputsStmt getInputsStmt() { result = this.(YamlMapping).lookup("inputs") } + Inputs getInputs() { result = this.(YamlMapping).lookup("inputs") } - OutputsStmt getOutputsStmt() { result = this.(YamlMapping).lookup("outputs") } + Outputs getOutputs() { result = this.(YamlMapping).lookup("outputs") } } -class RunsStmt extends Statement instanceof Actions::Runs { - StepStmt getAStepStmt() { result = super.getSteps().getElementNode(_) } +class Runs extends AstNode instanceof Actions::Runs { + Step getAStep() { result = super.getSteps().getElementNode(_) } - StepStmt getStepStmt(int i) { result = super.getSteps().getElementNode(i) } + Step getStep(int i) { result = super.getSteps().getElementNode(i) } } /** * A Github Actions Workflow */ -class WorkflowStmt extends Statement instanceof Actions::Workflow { +class Workflow extends AstNode instanceof Actions::Workflow { string getName() { result = super.getName() } - JobStmt getAJobStmt() { result = super.getJob(_) } + Job getAJob() { result = super.getJob(_) } - JobStmt getJobStmt(string id) { result = super.getJob(id) } + Job getJob(string id) { result = super.getJob(id) } predicate hasTriggerEvent(string trigger) { exists(YamlNode n | n = super.getOn().(YamlMappingLikeNode).getNode(trigger)) @@ -80,27 +71,25 @@ class WorkflowStmt extends Statement instanceof Actions::Workflow { exists(YamlNode n | n = super.getOn().(YamlMappingLikeNode).getNode(result)) } - Statement getPermissionsStmt() { result = this.(YamlMapping).lookup("permissions") } + Permissions getPermissions() { result = this.(YamlMapping).lookup("permissions") } - StrategyStmt getStrategyStmt() { result = this.(YamlMapping).lookup("strategy") } + Strategy getStrategy() { result = this.(YamlMapping).lookup("strategy") } } -class ReusableWorkflowStmt extends WorkflowStmt { +class ReusableWorkflow extends Workflow { YamlValue workflow_call; - ReusableWorkflowStmt() { - this.(Actions::Workflow).getOn().getNode("workflow_call") = workflow_call - } + ReusableWorkflow() { this.(Actions::Workflow).getOn().getNode("workflow_call") = workflow_call } - InputsStmt getInputsStmt() { result = workflow_call.(YamlMapping).lookup("inputs") } + Inputs getInputs() { result = workflow_call.(YamlMapping).lookup("inputs") } - OutputsStmt getOutputsStmt() { result = workflow_call.(YamlMapping).lookup("outputs") } + Outputs getOutputs() { result = workflow_call.(YamlMapping).lookup("outputs") } } -class InputsStmt extends Statement instanceof YamlMapping { +class Inputs extends AstNode instanceof YamlMapping { YamlMapping parent; - InputsStmt() { parent.lookup("inputs") = this } + Inputs() { parent.lookup("inputs") = this } /** * Gets a specific input expression (YamlMapping) by name. @@ -111,10 +100,10 @@ class InputsStmt extends Statement instanceof YamlMapping { } } -class OutputsStmt extends Statement instanceof YamlMapping { +class Outputs extends AstNode instanceof YamlMapping { YamlMapping parent; - OutputsStmt() { parent.lookup("outputs") = this } + Outputs() { parent.lookup("outputs") = this } /** * Gets a specific output expression (YamlMapping) by name. @@ -127,10 +116,16 @@ class OutputsStmt extends Statement instanceof YamlMapping { string getAnOutputName() { this.(YamlMapping).maps(any(YamlString s | s.getValue() = result), _) } } -class StrategyStmt extends Statement instanceof YamlMapping { +class Permissions extends AstNode instanceof YamlMapping { YamlMapping parent; - StrategyStmt() { parent.lookup("strategy") = this } + Permissions() { parent.lookup("permissions") = this } +} + +class Strategy extends AstNode instanceof YamlMapping { + YamlMapping parent; + + Strategy() { parent.lookup("strategy") = this } /** * Gets a specific matric expression (YamlMapping) by name. @@ -144,31 +139,10 @@ class StrategyStmt extends Statement instanceof YamlMapping { } } -class InputExpr extends Expression instanceof YamlString { - InputExpr() { exists(InputsStmt inputs | inputs.(YamlMapping).maps(this, _)) } -} - -class OutputExpr extends Expression instanceof YamlString { - OutputExpr() { - exists(OutputsStmt outputs | - outputs.(YamlMapping).lookup(_).(YamlMapping).lookup("value") = this or - outputs.(YamlMapping).lookup(_) = this - ) - } -} - -class MatrixVariableExpr extends Expression instanceof YamlString { - MatrixVariableExpr() { - exists(StrategyStmt outputs | - outputs.(YamlMapping).lookup("matrix").(YamlMapping).lookup(_) = this - ) - } -} - /** * A Job is a collection of steps that run in an execution environment. */ -class JobStmt extends Statement instanceof Actions::Job { +class Job extends AstNode instanceof Actions::Job { /** * Gets the ID of this job, as a string. * This is the job's key within the `jobs` mapping. @@ -176,20 +150,20 @@ class JobStmt extends Statement instanceof Actions::Job { string getId() { result = super.getId() } /** Gets the step at the given index within this job. */ - StepStmt getStepStmt(int index) { result = super.getStep(index) } + Step getStep(int index) { result = super.getStep(index) } /** Gets any steps that are defined within this job. */ - StepStmt getAStepStmt() { result = super.getStep(_) } + Step getAStep() { result = super.getStep(_) } /** * Gets a needed job. * eg: * - needs: [job1, job2] */ - JobStmt getNeededJob() { + Job getNeededJob() { exists(Actions::Needs needs | needs.getJob() = this and - result = needs.getANeededJob().(JobStmt) + result = needs.getANeededJob() ) } @@ -199,7 +173,7 @@ class JobStmt extends Statement instanceof Actions::Job { * out1: ${steps.foo.bar} * out2: ${steps.foo.baz} */ - OutputsStmt getOutputsStmt() { result = this.(Actions::Job).lookup("outputs") } + Outputs getOutputs() { result = this.(Actions::Job).lookup("outputs") } /** * Reusable workflow jobs may have Uses children @@ -209,42 +183,42 @@ class JobStmt extends Statement instanceof Actions::Job { * with: * arg1: value1 */ - JobUsesExpr getUsesExpr() { result.getJobStmt() = this } + JobUses getUses() { result.getJob() = this } predicate usesReusableWorkflow() { this.(YamlMapping).maps(any(YamlString s | s.getValue() = "uses"), _) } - IfStmt getIfStmt() { result = super.getIf() } + If getIf() { result = super.getIf() } - Statement getPermissionsStmt() { result = this.(YamlMapping).lookup("permissions") } + Permissions getPermissions() { result = this.(YamlMapping).lookup("permissions") } - StrategyStmt getStrategyStmt() { result = this.(YamlMapping).lookup("strategy") } + Strategy getStrategy() { result = this.(YamlMapping).lookup("strategy") } } /** * A Step is a single task that can be executed as part of a job. */ -class StepStmt extends Statement instanceof Actions::Step { +class Step extends AstNode instanceof Actions::Step { string getId() { result = super.getId() } - JobStmt getJobStmt() { result = super.getJob() } + Job getJob() { result = super.getJob() } - IfStmt getIfStmt() { result = super.getIf() } + If getIf() { result = super.getIf() } } /** * An If node representing a conditional statement. */ -class IfStmt extends Statement { +class If extends AstNode { YamlMapping parent; - IfStmt() { + If() { (parent instanceof Actions::Step or parent instanceof Actions::Job) and parent.lookup("if") = this } - Statement getEnclosingStatement() { result = parent } + AstNode getEnclosingNode() { result = parent } string getCondition() { result = this.(YamlScalar).getValue() } } @@ -252,7 +226,7 @@ class IfStmt extends Statement { /** * Abstract class representing a call to a 3rd party action or reusable workflow. */ -abstract class UsesExpr extends Expression { +abstract class Uses extends AstNode { abstract string getCallee(); abstract string getVersion(); @@ -263,10 +237,10 @@ abstract class UsesExpr extends Expression { /** * A Uses step represents a call to an action that is defined in a GitHub repository. */ -class StepUsesExpr extends StepStmt, UsesExpr { +class StepUses extends Step, Uses { Actions::Uses uses; - StepUsesExpr() { uses.getStep() = this } + StepUses() { uses.getStep() = this } override string getCallee() { result = uses.getGitHubRepository() } @@ -288,12 +262,10 @@ class StepUsesExpr extends StepStmt, UsesExpr { /** * A Uses step represents a call to an action that is defined in a GitHub repository. */ -class JobUsesExpr extends UsesExpr instanceof YamlMapping { - JobUsesExpr() { - this instanceof JobStmt and this.maps(any(YamlString s | s.getValue() = "uses"), _) - } +class JobUses extends Uses instanceof YamlMapping { + JobUses() { this instanceof Job and this.maps(any(YamlString s | s.getValue() = "uses"), _) } - JobStmt getJobStmt() { result = this } + Job getJob() { result = this } /** * Gets a regular expression that parses an `owner/repo@version` reference within a `uses` field in an Actions job step. @@ -336,10 +308,10 @@ class JobUsesExpr extends UsesExpr instanceof YamlMapping { /** * A Run step represents the evaluation of a provided script */ -class RunExpr extends StepStmt, Expression { +class Run extends Step { Actions::Run scriptExpr; - RunExpr() { scriptExpr.getStep() = this } + Run() { scriptExpr.getStep() = this } Expression getScriptExpr() { result = scriptExpr } @@ -347,24 +319,59 @@ class RunExpr extends StepStmt, Expression { } /** - * Evaluation of a workflow expression ${{}}. + * An AST node associated with a Reusable Workflow input. */ -class ExprAccessExpr extends Expression instanceof YamlString { - string expr; - - ExprAccessExpr() { expr = Actions::getASimpleReferenceExpression(this) } - - string getExpression() { result = expr } - - JobStmt getJobStmt() { result.getAChildNode*() = this } +class InputExpr extends AstNode { + InputExpr() { exists(Inputs inputs | inputs.(YamlMapping).maps(this, _)) } } /** - * A context access expression. + * An AST node holding an Env var value. + */ +class EnvExpr extends AstNode { + EnvExpr() { exists(Actions::Env env | env.(YamlMapping).lookup(_) = this) } +} + +/** + * An AST node holding a job or workflow output var. + */ +class OutputExpr extends AstNode { + OutputExpr() { + exists(Outputs outputs | + outputs.(YamlMapping).lookup(_).(YamlMapping).lookup("value") = this or + outputs.(YamlMapping).lookup(_) = this + ) + } +} + +/** + * An AST node holding a matrix var. + */ +class MatrixVariableExpr extends AstNode { + MatrixVariableExpr() { + exists(Strategy outputs | outputs.(YamlMapping).lookup("matrix").(YamlMapping).lookup(_) = this) + } +} + +/** + * Evaluation of a workflow expression ${{}}. + */ +class Expression extends AstNode instanceof YamlString { + string expr; + + Expression() { expr = Actions::getASimpleReferenceExpression(this) } + + string getExpression() { result = expr } + + Job getJob() { result.getAChildNode*() = this } +} + +/** + * A ${{}} expression accessing a context variable. * https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability */ -class CtxAccessExpr extends ExprAccessExpr { - CtxAccessExpr() { +class ContextExpression extends Expression { + ContextExpression() { expr.regexpMatch([ stepsCtxRegex(), needsCtxRegex(), jobsCtxRegex(), envCtxRegex(), inputsCtxRegex(), matrixCtxRegex() @@ -373,7 +380,7 @@ class CtxAccessExpr extends ExprAccessExpr { abstract string getFieldName(); - abstract Expression getRefExpr(); + abstract AstNode getTarget(); } private string stepsCtxRegex() { @@ -406,11 +413,11 @@ private string wrapRegexp(string regex) { * https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability * e.g. `${{ steps.changed-files.outputs.all_changed_files }}` */ -class StepsCtxAccessExpr extends CtxAccessExpr { +class StepsExpression extends ContextExpression { string stepId; string fieldName; - StepsCtxAccessExpr() { + StepsExpression() { expr.regexpMatch(stepsCtxRegex()) and stepId = expr.regexpCapture(stepsCtxRegex(), 1) and fieldName = expr.regexpCapture(stepsCtxRegex(), 2) @@ -418,9 +425,9 @@ class StepsCtxAccessExpr extends CtxAccessExpr { override string getFieldName() { result = fieldName } - override Expression getRefExpr() { + override AstNode getTarget() { this.getLocation().getFile() = result.getLocation().getFile() and - result.(StepStmt).getId() = stepId + result.(Step).getId() = stepId } } @@ -429,12 +436,12 @@ class StepsCtxAccessExpr extends CtxAccessExpr { * https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability * e.g. `${{ needs.job1.outputs.foo}}` */ -class NeedsCtxAccessExpr extends CtxAccessExpr { - JobStmt job; +class NeedsExpression extends ContextExpression { + Job job; string jobId; string fieldName; - NeedsCtxAccessExpr() { + NeedsExpression() { expr.regexpMatch(needsCtxRegex()) and jobId = expr.regexpCapture(needsCtxRegex(), 1) and fieldName = expr.regexpCapture(needsCtxRegex(), 2) and @@ -445,14 +452,14 @@ class NeedsCtxAccessExpr extends CtxAccessExpr { override string getFieldName() { result = fieldName } - override Expression getRefExpr() { + override AstNode getTarget() { job.getLocation().getFile() = this.getLocation().getFile() and ( // regular jobs - job.getOutputsStmt() = result + job.getOutputs() = result or // reusable workflow calling jobs - job.getUsesExpr() = result + job.getUses() = result ) } } @@ -462,11 +469,11 @@ class NeedsCtxAccessExpr extends CtxAccessExpr { * https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability * e.g. `${{ jobs.job1.outputs.foo}}` (within reusable workflows) */ -class JobsCtxAccessExpr extends CtxAccessExpr { +class JobsExpression extends ContextExpression { string jobId; string fieldName; - JobsCtxAccessExpr() { + JobsExpression() { expr.regexpMatch(jobsCtxRegex()) and jobId = expr.regexpCapture(jobsCtxRegex(), 1) and fieldName = expr.regexpCapture(jobsCtxRegex(), 2) @@ -474,11 +481,11 @@ class JobsCtxAccessExpr extends CtxAccessExpr { override string getFieldName() { result = fieldName } - override Expression getRefExpr() { - exists(JobStmt job | + override AstNode getTarget() { + exists(Job job | job.getId() = jobId and job.getLocation().getFile() = this.getLocation().getFile() and - job.getOutputsStmt() = result + job.getOutputs() = result ) } } @@ -488,21 +495,23 @@ class JobsCtxAccessExpr extends CtxAccessExpr { * https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability * e.g. `${{ inputs.foo }}` */ -class InputsCtxAccessExpr extends CtxAccessExpr { +class InputsExpression extends ContextExpression { string fieldName; - InputsCtxAccessExpr() { + InputsExpression() { expr.regexpMatch(inputsCtxRegex()) and fieldName = expr.regexpCapture(inputsCtxRegex(), 1) } override string getFieldName() { result = fieldName } - override Expression getRefExpr() { + override AstNode getTarget() { result.getLocation().getFile() = this.getLocation().getFile() and - exists(ReusableWorkflowStmt w | w.getInputsStmt().getInputExpr(fieldName) = result) - or - exists(CompositeActionStmt a | a.getInputsStmt().getInputExpr(fieldName) = result) + ( + exists(ReusableWorkflow w | w.getInputs().getInputExpr(fieldName) = result) + or + exists(CompositeAction a | a.getInputs().getInputExpr(fieldName) = result) + ) } } @@ -511,18 +520,18 @@ class InputsCtxAccessExpr extends CtxAccessExpr { * https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability * e.g. `${{ env.foo }}` */ -class EnvCtxAccessExpr extends CtxAccessExpr { +class EnvExpression extends ContextExpression { string fieldName; - EnvCtxAccessExpr() { + EnvExpression() { expr.regexpMatch(envCtxRegex()) and fieldName = expr.regexpCapture(envCtxRegex(), 1) } override string getFieldName() { result = fieldName } - override Expression getRefExpr() { - exists(Statement s | + override AstNode getTarget() { + exists(AstNode s | s.getEnvExpr(fieldName) = result and s.getAChildNode*() = this ) @@ -534,24 +543,24 @@ class EnvCtxAccessExpr extends CtxAccessExpr { * https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability * e.g. `${{ matrix.foo }}` */ -class MatrixCtxAccessExpr extends CtxAccessExpr { +class MatrixExpression extends ContextExpression { string fieldName; - MatrixCtxAccessExpr() { + MatrixExpression() { expr.regexpMatch(matrixCtxRegex()) and fieldName = expr.regexpCapture(matrixCtxRegex(), 1) } override string getFieldName() { result = fieldName } - override Expression getRefExpr() { - exists(WorkflowStmt w | - w.getStrategyStmt().getMatrixVariableExpr(fieldName) = result and + override AstNode getTarget() { + exists(Workflow w | + w.getStrategy().getMatrixVariableExpr(fieldName) = result and w.getAChildNode*() = this ) or - exists(JobStmt j | - j.getStrategyStmt().getMatrixVariableExpr(fieldName) = result and + exists(Job j | + j.getStrategy().getMatrixVariableExpr(fieldName) = result and j.getAChildNode*() = this ) } diff --git a/ql/lib/codeql/actions/controlflow/BasicBlocks.qll b/ql/lib/codeql/actions/controlflow/BasicBlocks.qll index cdc7b0cf24f..af5e0f62552 100644 --- a/ql/lib/codeql/actions/controlflow/BasicBlocks.qll +++ b/ql/lib/codeql/actions/controlflow/BasicBlocks.qll @@ -442,4 +442,3 @@ class ConditionBlock extends BasicBlock { */ predicate controls(BasicBlock controlled, BooleanSuccessor s) { controls(this, controlled, s) } } - diff --git a/ql/lib/codeql/actions/controlflow/internal/Cfg.qll b/ql/lib/codeql/actions/controlflow/internal/Cfg.qll index b8137172b8c..2bc86723493 100644 --- a/ql/lib/codeql/actions/controlflow/internal/Cfg.qll +++ b/ql/lib/codeql/actions/controlflow/internal/Cfg.qll @@ -83,9 +83,9 @@ module Completion { module CfgScope { abstract class CfgScope extends AstNode { } - class WorkflowScope extends CfgScope instanceof WorkflowStmt { } + class WorkflowScope extends CfgScope instanceof Workflow { } - class CompositeActionScope extends CfgScope instanceof CompositeActionStmt { } + class CompositeActionScope extends CfgScope instanceof CompositeAction { } } private module Implementation implements CfgShared::InputSig { @@ -119,13 +119,13 @@ private module Implementation implements CfgShared::InputSig { int maxSplits() { result = 0 } predicate scopeFirst(CfgScope scope, AstNode e) { - first(scope.(WorkflowStmt), e) or - first(scope.(CompositeActionStmt), e) + first(scope.(Workflow), e) or + first(scope.(CompositeAction), e) } predicate scopeLast(CfgScope scope, AstNode e, Completion c) { - last(scope.(WorkflowStmt), e, c) or - last(scope.(CompositeActionStmt), e, c) + last(scope.(Workflow), e, c) or + last(scope.(CompositeAction), e, c) } predicate successorTypeIsSimple(SuccessorType t) { t instanceof NormalSuccessor } @@ -143,14 +143,14 @@ private import CfgImpl private import Completion private import CfgScope -private class CompositeActionTree extends StandardPreOrderTree instanceof CompositeActionStmt { +private class CompositeActionTree extends StandardPreOrderTree instanceof CompositeAction { override ControlFlowTree getChildNode(int i) { result = - rank[i](Expression child, Location l | + rank[i](AstNode child, Location l | ( - child = this.(CompositeActionStmt).getInputsStmt() or - child = this.(CompositeActionStmt).getOutputsStmt() or - child = this.(CompositeActionStmt).getRunsStmt() + child = this.(CompositeAction).getInputs() or + child = this.(CompositeAction).getOutputs() or + child = this.(CompositeAction).getRuns() ) and l = child.getLocation() | @@ -161,21 +161,21 @@ private class CompositeActionTree extends StandardPreOrderTree instanceof Compos } } -private class RunsTree extends StandardPreOrderTree instanceof RunsStmt { - override ControlFlowTree getChildNode(int i) { result = super.getStepStmt(i) } +private class RunsTree extends StandardPreOrderTree instanceof Runs { + override ControlFlowTree getChildNode(int i) { result = super.getStep(i) } } -private class WorkflowTree extends StandardPreOrderTree instanceof WorkflowStmt { +private class WorkflowTree extends StandardPreOrderTree instanceof Workflow { override ControlFlowTree getChildNode(int i) { - if this instanceof ReusableWorkflowStmt + if this instanceof ReusableWorkflow then result = - rank[i](Expression child, Location l | + rank[i](AstNode child, Location l | ( - child = this.(ReusableWorkflowStmt).getInputsStmt() or - child = this.(ReusableWorkflowStmt).getOutputsStmt() or - child = this.(ReusableWorkflowStmt).getStrategyStmt() or - child = this.(ReusableWorkflowStmt).getAJobStmt() + child = this.(ReusableWorkflow).getInputs() or + child = this.(ReusableWorkflow).getOutputs() or + child = this.(ReusableWorkflow).getStrategy() or + child = this.(ReusableWorkflow).getAJob() ) and l = child.getLocation() | @@ -185,10 +185,10 @@ private class WorkflowTree extends StandardPreOrderTree instanceof WorkflowStmt ) else result = - rank[i](Expression child, Location l | + rank[i](AstNode child, Location l | ( - child = super.getAJobStmt() or - child = super.getStrategyStmt() + child = super.getAJob() or + child = super.getStrategy() ) and l = child.getLocation() | @@ -199,10 +199,10 @@ private class WorkflowTree extends StandardPreOrderTree instanceof WorkflowStmt } } -private class InputsTree extends StandardPreOrderTree instanceof InputsStmt { +private class InputsTree extends StandardPreOrderTree instanceof Inputs { override ControlFlowTree getChildNode(int i) { result = - rank[i](Expression child, Location l | + rank[i](AstNode child, Location l | child = super.getInputExpr(_) and l = child.getLocation() | child @@ -212,12 +212,10 @@ private class InputsTree extends StandardPreOrderTree instanceof InputsStmt { } } -private class InputExprTree extends LeafTree instanceof InputExpr { } - -private class OutputsTree extends StandardPreOrderTree instanceof OutputsStmt { +private class OutputsTree extends StandardPreOrderTree instanceof Outputs { override ControlFlowTree getChildNode(int i) { result = - rank[i](Expression child, Location l | + rank[i](AstNode child, Location l | child = super.getOutputExpr(_) and l = child.getLocation() | child @@ -227,12 +225,10 @@ private class OutputsTree extends StandardPreOrderTree instanceof OutputsStmt { } } -private class OutputExprTree extends LeafTree instanceof OutputExpr { } - -private class StrategyTree extends StandardPreOrderTree instanceof StrategyStmt { +private class StrategyTree extends StandardPreOrderTree instanceof Strategy { override ControlFlowTree getChildNode(int i) { result = - rank[i](Expression child, Location l | + rank[i](AstNode child, Location l | child = super.getMatrixVariableExpr(_) and l = child.getLocation() | child @@ -242,17 +238,15 @@ private class StrategyTree extends StandardPreOrderTree instanceof StrategyStmt } } -private class MatrixVariableExprTree extends LeafTree instanceof MatrixVariableExpr { } - -private class JobTree extends StandardPreOrderTree instanceof JobStmt { +private class JobTree extends StandardPreOrderTree instanceof Job { override ControlFlowTree getChildNode(int i) { result = - rank[i](Expression child, Location l | + rank[i](AstNode child, Location l | ( - child = super.getAStepStmt() or - child = super.getOutputsStmt() or - child = super.getStrategyStmt() or - child = super.getUsesExpr() + child = super.getAStep() or + child = super.getOutputs() or + child = super.getStrategy() or + child = super.getUses() ) and l = child.getLocation() | @@ -263,12 +257,10 @@ private class JobTree extends StandardPreOrderTree instanceof JobStmt { } } -private class UsesExprTree extends LeafTree instanceof UsesExpr { } - -private class UsesTree extends StandardPreOrderTree instanceof UsesExpr { +private class UsesTree extends StandardPreOrderTree instanceof Uses { override ControlFlowTree getChildNode(int i) { result = - rank[i](Expression child, Location l | + rank[i](AstNode child, Location l | (child = super.getArgumentExpr(_) or child = super.getEnvExpr(_)) and l = child.getLocation() | @@ -279,11 +271,10 @@ private class UsesTree extends StandardPreOrderTree instanceof UsesExpr { } } -private class RunTree extends StandardPreOrderTree instanceof RunExpr { - //override ControlFlowTree getChildNode(int i) { result = super.getScriptExpr() and i = 0 } +private class RunTree extends StandardPreOrderTree instanceof Run { override ControlFlowTree getChildNode(int i) { result = - rank[i](Expression child, Location l | + rank[i](AstNode child, Location l | (child = super.getEnvExpr(_) or child = super.getScriptExpr()) and l = child.getLocation() | @@ -294,4 +285,16 @@ private class RunTree extends StandardPreOrderTree instanceof RunExpr { } } -private class ExprAccessTree extends LeafTree instanceof ExprAccessExpr { } +private class UsesLeaf extends LeafTree instanceof Uses { } + +private class InputExprTree extends LeafTree instanceof InputExpr { } + +private class OutputExprTree extends LeafTree instanceof OutputExpr { } + +private class MatrixVariableExprTree extends LeafTree instanceof MatrixVariableExpr { } + +private class EnvExprTree extends LeafTree instanceof EnvExpr { } + +private class ExprAccessTree extends LeafTree instanceof ContextExpression { } + +private class AstNodeLeaf extends LeafTree instanceof Expression { } diff --git a/ql/lib/codeql/actions/dataflow/ExternalFlow.qll b/ql/lib/codeql/actions/dataflow/ExternalFlow.qll index 594b6017729..479078fe18b 100644 --- a/ql/lib/codeql/actions/dataflow/ExternalFlow.qll +++ b/ql/lib/codeql/actions/dataflow/ExternalFlow.qll @@ -42,7 +42,7 @@ predicate sinkModel(string action, string version, string input, string kind) { predicate externallyDefinedSource( DataFlow::Node source, string sourceType, string fieldName, string trigger ) { - exists(UsesExpr uses, string action, string version, string kind | + exists(Uses uses, string action, string version, string kind | sourceModel(action, version, fieldName, trigger, kind) and uses.getCallee() = action.toLowerCase() and ( @@ -65,7 +65,7 @@ predicate externallyDefinedSource( predicate externallyDefinedStoreStep( DataFlow::Node pred, DataFlow::Node succ, DataFlow::ContentSet c ) { - exists(UsesExpr uses, string action, string version, string input, string output | + exists(Uses uses, string action, string version, string input, string output | summaryModel(action, version, input, output, "taint") and c = any(DataFlow::FieldContent ct | ct.getName() = output.replaceAll("output.", "")) and uses.getCallee() = action.toLowerCase() and @@ -87,7 +87,7 @@ predicate externallyDefinedStoreStep( } predicate externallyDefinedSink(DataFlow::ExprNode sink, string kind) { - exists(UsesExpr uses, string action, string version, string input | + exists(Uses uses, string action, string version, string input | ( if input.trim().matches("env.%") then sink.asExpr() = uses.getEnvExpr(input.trim().replaceAll("env.", "")) diff --git a/ql/lib/codeql/actions/dataflow/FlowSources.qll b/ql/lib/codeql/actions/dataflow/FlowSources.qll index 0e82498bfc1..c30c963afdb 100644 --- a/ql/lib/codeql/actions/dataflow/FlowSources.qll +++ b/ql/lib/codeql/actions/dataflow/FlowSources.qll @@ -114,7 +114,7 @@ private class EventSource extends RemoteFlowSource { string trigger; EventSource() { - exists(ExprAccessExpr e, string context | this.asExpr() = e and context = e.getExpression() | + exists(Expression e, string context | this.asExpr() = e and context = e.getExpression() | trigger = ["issues", "issue_comment"] and isExternalUserControlledIssue(context) or trigger = ["pull_request_target", "pull_request_review", "pull_request_review_comment"] and @@ -158,9 +158,9 @@ private class ExternallyDefinedSource extends RemoteFlowSource { * An input for a Composite Action */ private class CompositeActionInputSource extends RemoteFlowSource { - CompositeActionStmt c; + CompositeAction c; - CompositeActionInputSource() { c.getInputsStmt().getInputExpr(_) = this.asExpr() } + CompositeActionInputSource() { c.getInputs().getInputExpr(_) = this.asExpr() } override string getSourceType() { result = "Composite action input" } diff --git a/ql/lib/codeql/actions/dataflow/FlowSteps.qll b/ql/lib/codeql/actions/dataflow/FlowSteps.qll index bc0c782e9ff..64df342ae9b 100644 --- a/ql/lib/codeql/actions/dataflow/FlowSteps.qll +++ b/ql/lib/codeql/actions/dataflow/FlowSteps.qll @@ -34,7 +34,7 @@ class AdditionalTaintStep extends Unit { * echo "foo=$(echo $BODY)" >> "$GITHUB_OUTPUT" */ predicate runEnvToScriptStoreStep(DataFlow::Node pred, DataFlow::Node succ, DataFlow::ContentSet c) { - exists(RunExpr r, string varName, string output | + exists(Run r, string varName, string output | c = any(DataFlow::FieldContent ct | ct.getName() = output.replaceAll("output\\.", "")) and r.getEnvExpr(varName) = pred.asExpr() and exists(string script, string line | diff --git a/ql/lib/codeql/actions/dataflow/internal/DataFlowPrivate.qll b/ql/lib/codeql/actions/dataflow/internal/DataFlowPrivate.qll index b9aafb8ec94..62975959b39 100644 --- a/ql/lib/codeql/actions/dataflow/internal/DataFlowPrivate.qll +++ b/ql/lib/codeql/actions/dataflow/internal/DataFlowPrivate.qll @@ -54,21 +54,22 @@ DataFlowType getNodeType(Node node) { any() } predicate nodeIsHidden(Node node) { none() } class DataFlowExpr extends Cfg::Node { - DataFlowExpr() { this.getAstNode() instanceof Expression } + DataFlowExpr() { any() } + //DataFlowExpr() { this.getAstNode() instanceof Expression } } /** * A call corresponds to a Uses steps where a 3rd party action or a reusable workflow get called */ class DataFlowCall instanceof Cfg::Node { - DataFlowCall() { super.getAstNode() instanceof UsesExpr } + DataFlowCall() { super.getAstNode() instanceof Uses } /** Gets a textual representation of this element. */ string toString() { result = super.toString() } Location getLocation() { result = super.getLocation() } - string getName() { result = super.getAstNode().(UsesExpr).getCallee() } + string getName() { result = super.getAstNode().(Uses).getCallee() } DataFlowCallable getEnclosingCallable() { result = super.getScope() } } @@ -82,11 +83,11 @@ class DataFlowCallable instanceof Cfg::CfgScope { Location getLocation() { result = super.getLocation() } string getName() { - if this instanceof ReusableWorkflowStmt - then result = this.(ReusableWorkflowStmt).getLocation().getFile().getRelativePath() + if this instanceof ReusableWorkflow + then result = this.(ReusableWorkflow).getLocation().getFile().getRelativePath() else - if this instanceof CompositeActionStmt - then result = this.(CompositeActionStmt).getLocation().getFile().getRelativePath() + if this instanceof CompositeAction + then result = this.(CompositeAction).getLocation().getFile().getRelativePath() else none() } } @@ -134,9 +135,9 @@ predicate typeStrongerThan(DataFlowType t1, DataFlowType t2) { none() } newtype TContent = TFieldContent(string name) { // We only use field flow for steps and jobs outputs, not for accessing other context fields such as env, matrix or inputs - name = any(StepsCtxAccessExpr a).getFieldName() or - name = any(NeedsCtxAccessExpr a).getFieldName() or - name = any(JobsCtxAccessExpr a).getFieldName() + name = any(StepsExpression a).getFieldName() or + name = any(NeedsExpression a).getFieldName() or + name = any(JobsExpression a).getFieldName() } predicate forceHighPrecision(Content c) { c instanceof FieldContent } @@ -149,14 +150,14 @@ ContentApprox getContentApprox(Content c) { result = c } * Made a string to match the ArgumentPosition type. */ class ParameterPosition extends string { - ParameterPosition() { exists(any(ReusableWorkflowStmt w).getInputsStmt().getInputExpr(this)) } + ParameterPosition() { exists(any(ReusableWorkflow w).getInputs().getInputExpr(this)) } } /** * Made a string to match `With:` keys in the AST */ class ArgumentPosition extends string { - ArgumentPosition() { exists(any(UsesExpr e).getArgumentExpr(this)) } + ArgumentPosition() { exists(any(Uses e).getArgumentExpr(this)) } } /** @@ -172,11 +173,11 @@ predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) { ppos = * field name. */ predicate stepsCtxLocalStep(Node nodeFrom, Node nodeTo) { - exists(UsesExpr astFrom, StepsCtxAccessExpr astTo | + exists(Uses astFrom, StepsExpression astTo | externallyDefinedSource(nodeFrom, _, "output." + astTo.getFieldName(), _) and astFrom = nodeFrom.asExpr() and astTo = nodeTo.asExpr() and - astTo.getRefExpr() = astFrom + astTo.getTarget() = astFrom ) } @@ -189,11 +190,11 @@ predicate stepsCtxLocalStep(Node nodeFrom, Node nodeTo) { * field name. */ predicate needsCtxLocalStep(Node nodeFrom, Node nodeTo) { - exists(UsesExpr astFrom, NeedsCtxAccessExpr astTo | + exists(Uses astFrom, NeedsExpression astTo | externallyDefinedSource(nodeFrom, _, "output." + astTo.getFieldName(), _) and astFrom = nodeFrom.asExpr() and astTo = nodeTo.asExpr() and - astTo.getRefExpr() = astFrom + astTo.getTarget() = astFrom ) } @@ -202,10 +203,10 @@ predicate needsCtxLocalStep(Node nodeFrom, Node nodeTo) { * e.g. ${{ inputs.foo }} */ predicate inputsCtxLocalStep(Node nodeFrom, Node nodeTo) { - exists(Expression astFrom, InputsCtxAccessExpr astTo | + exists(AstNode astFrom, InputsExpression astTo | astFrom = nodeFrom.asExpr() and astTo = nodeTo.asExpr() and - astTo.getRefExpr() = astFrom + astTo.getTarget() = astFrom ) } @@ -214,10 +215,10 @@ predicate inputsCtxLocalStep(Node nodeFrom, Node nodeTo) { * e.g. ${{ matrix.foo }} */ predicate matrixCtxLocalStep(Node nodeFrom, Node nodeTo) { - exists(Expression astFrom, MatrixCtxAccessExpr astTo | + exists(AstNode astFrom, MatrixExpression astTo | astFrom = nodeFrom.asExpr() and astTo = nodeTo.asExpr() and - astTo.getRefExpr() = astFrom + astTo.getTarget() = astFrom ) } @@ -226,12 +227,12 @@ predicate matrixCtxLocalStep(Node nodeFrom, Node nodeTo) { * e.g. ${{ env.foo }} */ predicate envCtxLocalStep(Node nodeFrom, Node nodeTo) { - exists(Expression astFrom, EnvCtxAccessExpr astTo | + exists(Expression astFrom, EnvExpression astTo | astFrom = nodeFrom.asExpr() and astTo = nodeTo.asExpr() and ( externallyDefinedSource(nodeFrom, _, "env." + astTo.getFieldName(), _) or - astTo.getRefExpr() = astFrom + astTo.getTarget() = astFrom ) ) } @@ -266,17 +267,17 @@ predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) { localFlowStep(nodeFr predicate jumpStep(Node nodeFrom, Node nodeTo) { none() } /** - * Holds if a CtxAccessExpr reads a field from a job (needs/jobs), step (steps) output via a read of `c` (fieldname) + * Holds if a Expression reads a field from a job (needs/jobs), step (steps) output via a read of `c` (fieldname) */ predicate ctxFieldReadStep(Node node1, Node node2, ContentSet c) { - exists(CtxAccessExpr access | + exists(ContextExpression access | ( - access instanceof NeedsCtxAccessExpr or - access instanceof StepsCtxAccessExpr or - access instanceof JobsCtxAccessExpr + access instanceof NeedsExpression or + access instanceof StepsExpression or + access instanceof JobsExpression ) and c = any(FieldContent ct | ct.getName() = access.getFieldName()) and - node1.asExpr() = access.getRefExpr() and + node1.asExpr() = access.getTarget() and node2.asExpr() = access ) } @@ -294,7 +295,7 @@ predicate readStep(Node node1, ContentSet c, Node node2) { ctxFieldReadStep(node * using the output variable name as the access path */ predicate fieldStoreStep(Node node1, Node node2, ContentSet c) { - exists(OutputsStmt out, string fieldName | + exists(Outputs out, string fieldName | node1.asExpr() = out.getOutputExpr(fieldName) and node2.asExpr() = out and c = any(FieldContent ct | ct.getName() = fieldName) diff --git a/ql/lib/codeql/actions/dataflow/internal/DataFlowPublic.qll b/ql/lib/codeql/actions/dataflow/internal/DataFlowPublic.qll index 5fe3c741735..a8434cdb603 100644 --- a/ql/lib/codeql/actions/dataflow/internal/DataFlowPublic.qll +++ b/ql/lib/codeql/actions/dataflow/internal/DataFlowPublic.qll @@ -52,11 +52,11 @@ class ParameterNode extends ExprNode { ParameterNode() { this.asExpr() = input and - input = any(InputsStmt s).getInputExpr(_) + input = any(Inputs s).getInputExpr(_) } predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) { - input = c.(ReusableWorkflowStmt).getInputsStmt().getInputExpr(pos) + input = c.(ReusableWorkflow).getInputs().getInputExpr(pos) } override string toString() { result = "input " + input.toString() } @@ -81,12 +81,12 @@ class CallNode extends ExprNode { * An argument to a Uses step (call). */ class ArgumentNode extends ExprNode { - ArgumentNode() { this.getCfgNode().getAstNode() = any(UsesExpr e).getArgumentExpr(_) } + ArgumentNode() { this.getCfgNode().getAstNode() = any(Uses e).getArgumentExpr(_) } predicate argumentOf(DataFlowCall call, ArgumentPosition pos) { this.getCfgNode() = call.(Cfg::Node).getASuccessor+() and call.(Cfg::Node).getAstNode() = - any(UsesExpr e | e.getArgumentExpr(pos) = this.getCfgNode().getAstNode()) + any(Uses e | e.getArgumentExpr(pos) = this.getCfgNode().getAstNode()) } } @@ -94,11 +94,11 @@ class ArgumentNode extends ExprNode { * Reusable workflow output nodes */ class ReturnNode extends ExprNode { - private OutputsStmt outputs; + private Outputs outputs; ReturnNode() { this.asExpr() = outputs and - outputs = any(ReusableWorkflowStmt s).getOutputsStmt() + outputs = any(ReusableWorkflow s).getOutputs() } ReturnKind getKind() { result = TNormalReturn() } diff --git a/ql/src/Debug/partial.ql b/ql/src/Debug/partial.ql index c0a694455dc..fbdf9ca7daa 100644 --- a/ql/src/Debug/partial.ql +++ b/ql/src/Debug/partial.ql @@ -15,7 +15,7 @@ import PartialFlow::PartialPathGraph private module MyConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource and - source.getLocation().getFile().getBaseName() = "matrix.yml" + source.getLocation().getFile().getBaseName() = "argus_case_study.yml" } predicate isSink(DataFlow::Node sink) { none() } diff --git a/ql/src/Security/CWE-020/CompositeActionsSinks.ql b/ql/src/Security/CWE-020/CompositeActionsSinks.ql index 525307bcc28..5bff6abc7bb 100644 --- a/ql/src/Security/CWE-020/CompositeActionsSinks.ql +++ b/ql/src/Security/CWE-020/CompositeActionsSinks.ql @@ -18,14 +18,14 @@ import codeql.actions.dataflow.ExternalFlow private class ExpressionInjectionSink extends DataFlow::Node { ExpressionInjectionSink() { - exists(RunExpr e | e.getScriptExpr() = this.asExpr()) or + exists(Run e | e.getScriptExpr() = this.asExpr()) or externallyDefinedSink(this, "expression-injection") } } private module MyConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { - exists(CompositeActionStmt c | c.getInputsStmt().getInputExpr(_) = source.asExpr()) + exists(CompositeAction c | c.getInputs().getInputExpr(_) = source.asExpr()) } predicate isSink(DataFlow::Node sink) { sink instanceof ExpressionInjectionSink } diff --git a/ql/src/Security/CWE-020/CompositeActionsSources.ql b/ql/src/Security/CWE-020/CompositeActionsSources.ql index b3eb6d348a8..12703a6cff2 100644 --- a/ql/src/Security/CWE-020/CompositeActionsSources.ql +++ b/ql/src/Security/CWE-020/CompositeActionsSources.ql @@ -20,11 +20,11 @@ private module MyConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource and not source instanceof DataFlow::ParameterNode and - exists(CompositeActionStmt c | c.getAChildNode*() = source.asExpr()) + exists(CompositeAction c | c.getAChildNode*() = source.asExpr()) } predicate isSink(DataFlow::Node sink) { - exists(CompositeActionStmt c | c.getOutputsStmt().getOutputExpr(_) = sink.asExpr()) + exists(CompositeAction c | c.getOutputs().getOutputExpr(_) = sink.asExpr()) } predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet set) { diff --git a/ql/src/Security/CWE-020/CompositeActionsSummaries.ql b/ql/src/Security/CWE-020/CompositeActionsSummaries.ql index b451d9d1bda..e5933a73b36 100644 --- a/ql/src/Security/CWE-020/CompositeActionsSummaries.ql +++ b/ql/src/Security/CWE-020/CompositeActionsSummaries.ql @@ -18,11 +18,11 @@ import codeql.actions.dataflow.ExternalFlow private module MyConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { - exists(CompositeActionStmt c | c.getInputsStmt().getInputExpr(_) = source.asExpr()) + exists(CompositeAction c | c.getInputs().getInputExpr(_) = source.asExpr()) } predicate isSink(DataFlow::Node sink) { - exists(CompositeActionStmt c | c.getOutputsStmt().getOutputExpr(_) = sink.asExpr()) + exists(CompositeAction c | c.getOutputs().getOutputExpr(_) = sink.asExpr()) } } diff --git a/ql/src/Security/CWE-020/ReusableWorkflowsSinks.ql b/ql/src/Security/CWE-020/ReusableWorkflowsSinks.ql index 9317b900158..1e1f942b200 100644 --- a/ql/src/Security/CWE-020/ReusableWorkflowsSinks.ql +++ b/ql/src/Security/CWE-020/ReusableWorkflowsSinks.ql @@ -18,14 +18,14 @@ import codeql.actions.dataflow.ExternalFlow private class ExpressionInjectionSink extends DataFlow::Node { ExpressionInjectionSink() { - exists(RunExpr e | e.getScriptExpr() = this.asExpr()) or + exists(Run e | e.getScriptExpr() = this.asExpr()) or externallyDefinedSink(this, "expression-injection") } } private module MyConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { - exists(ReusableWorkflowStmt w | w.getInputsStmt().getInputExpr(_) = source.asExpr()) + exists(ReusableWorkflow w | w.getInputs().getInputExpr(_) = source.asExpr()) } predicate isSink(DataFlow::Node sink) { sink instanceof ExpressionInjectionSink } diff --git a/ql/src/Security/CWE-020/ReusableWorkflowsSources.ql b/ql/src/Security/CWE-020/ReusableWorkflowsSources.ql index eeea688b273..7bcea3d45b0 100644 --- a/ql/src/Security/CWE-020/ReusableWorkflowsSources.ql +++ b/ql/src/Security/CWE-020/ReusableWorkflowsSources.ql @@ -20,11 +20,11 @@ private module MyConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource and not source instanceof DataFlow::ParameterNode and - exists(ReusableWorkflowStmt w | w.getAChildNode*() = source.asExpr()) + exists(ReusableWorkflow w | w.getAChildNode*() = source.asExpr()) } predicate isSink(DataFlow::Node sink) { - exists(ReusableWorkflowStmt w | w.getOutputsStmt().getOutputExpr(_) = sink.asExpr()) + exists(ReusableWorkflow w | w.getOutputs().getOutputExpr(_) = sink.asExpr()) } predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet set) { diff --git a/ql/src/Security/CWE-020/ReusableWorkflowsSummaries.ql b/ql/src/Security/CWE-020/ReusableWorkflowsSummaries.ql index 3949488e129..5ac0c299929 100644 --- a/ql/src/Security/CWE-020/ReusableWorkflowsSummaries.ql +++ b/ql/src/Security/CWE-020/ReusableWorkflowsSummaries.ql @@ -18,11 +18,11 @@ import codeql.actions.dataflow.ExternalFlow private module MyConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { - exists(ReusableWorkflowStmt w | w.getInputsStmt().getInputExpr(_) = source.asExpr()) + exists(ReusableWorkflow w | w.getInputs().getInputExpr(_) = source.asExpr()) } predicate isSink(DataFlow::Node sink) { - exists(ReusableWorkflowStmt w | w.getOutputsStmt().getOutputExpr(_) = sink.asExpr()) + exists(ReusableWorkflow w | w.getOutputs().getOutputExpr(_) = sink.asExpr()) } } diff --git a/ql/src/Security/CWE-094/CriticalExpressionInjection.ql b/ql/src/Security/CWE-094/CriticalExpressionInjection.ql index a6baf060c9d..63f1a7a9d3a 100644 --- a/ql/src/Security/CWE-094/CriticalExpressionInjection.ql +++ b/ql/src/Security/CWE-094/CriticalExpressionInjection.ql @@ -19,7 +19,7 @@ import codeql.actions.dataflow.ExternalFlow private class ExpressionInjectionSink extends DataFlow::Node { ExpressionInjectionSink() { - exists(RunExpr e | e.getScriptExpr() = this.asExpr()) or + exists(Run e | e.getScriptExpr() = this.asExpr()) or externallyDefinedSink(this, "expression-injection") } } @@ -40,8 +40,7 @@ where source .getNode() .asExpr() - .(Statement) - .getEnclosingWorkflowStmt() + .getEnclosingWorkflow() .hasTriggerEvent(source.getNode().(RemoteFlowSource).getATriggerEvent()) select sink.getNode(), source, sink, "Potential expression injection, which may be controlled by an external user." diff --git a/ql/src/Security/CWE-094/ExpressionInjection.ql b/ql/src/Security/CWE-094/ExpressionInjection.ql index c34fcb74bbc..b13bf88abe6 100644 --- a/ql/src/Security/CWE-094/ExpressionInjection.ql +++ b/ql/src/Security/CWE-094/ExpressionInjection.ql @@ -19,7 +19,7 @@ import codeql.actions.dataflow.ExternalFlow private class ExpressionInjectionSink extends DataFlow::Node { ExpressionInjectionSink() { - exists(RunExpr e | e.getScriptExpr() = this.asExpr()) or + exists(Run e | e.getScriptExpr() = this.asExpr()) or externallyDefinedSink(this, "expression-injection") } } diff --git a/ql/src/Security/CWE-275/MissingActionsPermissions.ql b/ql/src/Security/CWE-275/MissingActionsPermissions.ql index a4cecf18b78..9373bf808e3 100644 --- a/ql/src/Security/CWE-275/MissingActionsPermissions.ql +++ b/ql/src/Security/CWE-275/MissingActionsPermissions.ql @@ -13,11 +13,11 @@ import actions -from WorkflowStmt workflow, JobStmt job +from Workflow workflow, Job job where - job = workflow.getAJobStmt() and + job = workflow.getAJob() and ( - not exists(workflow.getPermissionsStmt()) and - not exists(job.getPermissionsStmt()) + not exists(workflow.getPermissions()) and + not exists(job.getPermissions()) ) select job, "Actions Job or Workflow does not set permissions" diff --git a/ql/src/Security/CWE-829/UnpinnedActionsTag.ql b/ql/src/Security/CWE-829/UnpinnedActionsTag.ql index 3c951a4e0b0..34bcbd7b060 100644 --- a/ql/src/Security/CWE-829/UnpinnedActionsTag.ql +++ b/ql/src/Security/CWE-829/UnpinnedActionsTag.ql @@ -21,10 +21,10 @@ private predicate isTrustedOrg(string repo) { exists(string org | org in ["actions", "github", "advanced-security"] | repo.matches(org + "/%")) } -from StepUsesExpr uses, string repo, string version, WorkflowStmt workflow, string name +from StepUses uses, string repo, string version, Workflow workflow, string name where uses.getCallee() = repo and - uses.getEnclosingWorkflowStmt() = workflow and + uses.getEnclosingWorkflow() = workflow and ( workflow.getName() = name or diff --git a/ql/src/Security/CWE-829/UntrustedCheckout.ql b/ql/src/Security/CWE-829/UntrustedCheckout.ql index 3c745b5d84a..ed96d5f07c1 100644 --- a/ql/src/Security/CWE-829/UntrustedCheckout.ql +++ b/ql/src/Security/CWE-829/UntrustedCheckout.ql @@ -18,34 +18,34 @@ import actions /** * An If node that contains an `actor` check */ -class ActorCheckStmt extends IfStmt { - ActorCheckStmt() { this.getCondition().regexpMatch(".*github\\.(triggering_)?actor.*") } +class ActorCheck extends If { + ActorCheck() { this.getCondition().regexpMatch(".*github\\.(triggering_)?actor.*") } } /** * An If node that contains a `label` check */ -class LabelCheckStmt extends IfStmt { - LabelCheckStmt() { +class LabelCheck extends If { + LabelCheck() { this.getCondition().regexpMatch(".*github\\.event\\.pull_request\\.labels.*") or this.getCondition().regexpMatch(".*github\\.event\\.label\\.name.*") } } -from WorkflowStmt w, JobStmt job, StepUsesExpr checkoutStep +from Workflow w, Job job, StepUses checkoutStep where w.hasTriggerEvent("pull_request_target") and - w.getAJobStmt() = job and - job.getAStepStmt() = checkoutStep and + w.getAJob() = job and + job.getAStep() = checkoutStep and checkoutStep.getCallee() = "actions/checkout" and checkoutStep .getArgumentExpr("ref") - .(ExprAccessExpr) + .(Expression) .getExpression() .matches([ "%github.event.pull_request.head.ref%", "%github.event.pull_request.head.sha%", "%github.event.pull_request.number%", "%github.event.number%", "%github.head_ref%" ]) and - not exists(ActorCheckStmt check | job.getIfStmt() = check or checkoutStep.getIfStmt() = check) and - not exists(LabelCheckStmt check | job.getIfStmt() = check or checkoutStep.getIfStmt() = check) + not exists(ActorCheck check | job.getIf() = check or checkoutStep.getIf() = check) and + not exists(LabelCheck check | job.getIf() = check or checkoutStep.getIf() = check) select checkoutStep, "Potential unsafe checkout of untrusted pull request on 'pull_request_target'." diff --git a/ql/test/library-tests/test.expected b/ql/test/library-tests/test.expected index 4007e6454ea..ffbbed2bac1 100644 --- a/ql/test/library-tests/test.expected +++ b/ql/test/library-tests/test.expected @@ -90,18 +90,11 @@ stepUsesNodes | .github/workflows/test.yml:19:9:26:6 | name: R ... d files | jobUsesNodes usesSteps -| .github/workflows/test.yml:11:9:15:6 | uses: a ... kout@v4 | fetch-depth | .github/workflows/test.yml:13:24:13:24 | 0 | -| .github/workflows/test.yml:19:9:26:6 | name: R ... d files | find | .github/workflows/test.yml:24:17:24:21 | "foo" | -| .github/workflows/test.yml:19:9:26:6 | name: R ... d files | replace | .github/workflows/test.yml:25:20:25:21 | "" | | .github/workflows/test.yml:19:9:26:6 | name: R ... d files | source | .github/workflows/test.yml:23:19:23:63 | ${{ ste ... iles }} | -runSteps1 +runSteps | .github/workflows/test.yml:26:9:28:6 | id: simplesink1 | echo ${{ steps.source.outputs.all_changed_files }} | | .github/workflows/test.yml:28:9:31:2 | id: simplesink2 | ${{ github.event.pull_request.head.ref }} | | .github/workflows/test.yml:39:9:40:53 | id: sink | echo ${{needs.job1.outputs.job_output}} | -runSteps2 -| .github/workflows/test.yml:26:9:28:6 | id: simplesink1 | .github/workflows/test.yml:27:14:27:63 | echo ${ ... iles }} | -| .github/workflows/test.yml:28:9:31:2 | id: simplesink2 | .github/workflows/test.yml:29:14:29:54 | ${{ git ... .ref }} | -| .github/workflows/test.yml:39:9:40:53 | id: sink | .github/workflows/test.yml:40:14:40:52 | echo ${ ... utput}} | runStepChildren | .github/workflows/test.yml:26:9:28:6 | id: simplesink1 | .github/workflows/test.yml:26:9:26:10 | id | | .github/workflows/test.yml:26:9:28:6 | id: simplesink1 | .github/workflows/test.yml:26:13:26:23 | simplesink1 | @@ -115,21 +108,6 @@ runStepChildren | .github/workflows/test.yml:39:9:40:53 | id: sink | .github/workflows/test.yml:39:13:39:16 | sink | | .github/workflows/test.yml:39:9:40:53 | id: sink | .github/workflows/test.yml:40:9:40:11 | run | | .github/workflows/test.yml:39:9:40:53 | id: sink | .github/workflows/test.yml:40:14:40:52 | echo ${ ... utput}} | -varAccesses -| .github/workflows/test.yml:8:19:8:49 | ${{ ste ... alue }} | steps.step.outputs.value | -| .github/workflows/test.yml:23:19:23:63 | ${{ ste ... iles }} | steps.source.outputs.all_changed_files | -| .github/workflows/test.yml:27:14:27:63 | echo ${ ... iles }} | steps.source.outputs.all_changed_files | -| .github/workflows/test.yml:29:14:29:54 | ${{ git ... .ref }} | github.event.pull_request.head.ref | -| .github/workflows/test.yml:34:9:34:23 | ${{ always() }} | always() | -| .github/workflows/test.yml:40:14:40:52 | echo ${ ... utput}} | needs.job1.outputs.job_output | -orphanVarAccesses -nonOrphanVarAccesses -| .github/workflows/test.yml:8:19:8:49 | ${{ ste ... alue }} | steps.step.outputs.value | .github/workflows/test.yml:8:7:10:4 | job_out ... alue }} | -| .github/workflows/test.yml:23:19:23:63 | ${{ ste ... iles }} | steps.source.outputs.all_changed_files | .github/workflows/test.yml:23:11:26:6 | source: ... iles }} | -| .github/workflows/test.yml:27:14:27:63 | echo ${ ... iles }} | steps.source.outputs.all_changed_files | .github/workflows/test.yml:26:9:28:6 | id: simplesink1 | -| .github/workflows/test.yml:29:14:29:54 | ${{ git ... .ref }} | github.event.pull_request.head.ref | .github/workflows/test.yml:28:9:31:2 | id: simplesink2 | -| .github/workflows/test.yml:34:9:34:23 | ${{ always() }} | always() | .github/workflows/test.yml:32:5:40:53 | runs-on ... -latest | -| .github/workflows/test.yml:40:14:40:52 | echo ${ ... utput}} | needs.job1.outputs.job_output | .github/workflows/test.yml:39:9:40:53 | id: sink | parentNodes | .github/workflows/test.yml:1:1:1:2 | on | .github/workflows/test.yml:1:1:40:53 | on: push | | .github/workflows/test.yml:1:5:1:8 | push | .github/workflows/test.yml:1:1:40:53 | on: push | @@ -200,6 +178,8 @@ parentNodes | .github/workflows/test.yml:40:9:40:11 | run | .github/workflows/test.yml:39:9:40:53 | id: sink | | .github/workflows/test.yml:40:14:40:52 | echo ${ ... utput}} | .github/workflows/test.yml:39:9:40:53 | id: sink | cfgNodes +dfNodes +exprNodes | .github/workflows/test.yml:1:1:40:53 | enter on: push | | .github/workflows/test.yml:1:1:40:53 | exit on: push | | .github/workflows/test.yml:1:1:40:53 | exit on: push (normal) | @@ -218,44 +198,15 @@ cfgNodes | .github/workflows/test.yml:32:5:40:53 | runs-on ... -latest | | .github/workflows/test.yml:39:9:40:53 | id: sink | | .github/workflows/test.yml:40:14:40:52 | echo ${ ... utput}} | -dfNodes -| .github/workflows/test.yml:1:1:40:53 | on: push | -| .github/workflows/test.yml:5:5:31:2 | runs-on ... -latest | -| .github/workflows/test.yml:8:7:10:4 | job_out ... alue }} | -| .github/workflows/test.yml:8:19:8:49 | ${{ ste ... alue }} | -| .github/workflows/test.yml:11:9:15:6 | uses: a ... kout@v4 | -| .github/workflows/test.yml:15:9:19:6 | name: G ... d files | -| .github/workflows/test.yml:19:9:26:6 | name: R ... d files | -| .github/workflows/test.yml:23:19:23:63 | ${{ ste ... iles }} | -| .github/workflows/test.yml:26:9:28:6 | id: simplesink1 | -| .github/workflows/test.yml:27:14:27:63 | echo ${ ... iles }} | -| .github/workflows/test.yml:28:9:31:2 | id: simplesink2 | -| .github/workflows/test.yml:29:14:29:54 | ${{ git ... .ref }} | -| .github/workflows/test.yml:32:5:40:53 | runs-on ... -latest | -| .github/workflows/test.yml:39:9:40:53 | id: sink | -| .github/workflows/test.yml:40:14:40:52 | echo ${ ... utput}} | -exprNodes -| .github/workflows/test.yml:1:1:40:53 | on: push | -| .github/workflows/test.yml:5:5:31:2 | runs-on ... -latest | -| .github/workflows/test.yml:8:7:10:4 | job_out ... alue }} | -| .github/workflows/test.yml:8:19:8:49 | ${{ ste ... alue }} | -| .github/workflows/test.yml:11:9:15:6 | uses: a ... kout@v4 | -| .github/workflows/test.yml:15:9:19:6 | name: G ... d files | -| .github/workflows/test.yml:19:9:26:6 | name: R ... d files | -| .github/workflows/test.yml:23:19:23:63 | ${{ ste ... iles }} | -| .github/workflows/test.yml:26:9:28:6 | id: simplesink1 | -| .github/workflows/test.yml:27:14:27:63 | echo ${ ... iles }} | -| .github/workflows/test.yml:28:9:31:2 | id: simplesink2 | -| .github/workflows/test.yml:29:14:29:54 | ${{ git ... .ref }} | -| .github/workflows/test.yml:32:5:40:53 | runs-on ... -latest | -| .github/workflows/test.yml:39:9:40:53 | id: sink | -| .github/workflows/test.yml:40:14:40:52 | echo ${ ... utput}} | argumentNodes | .github/workflows/test.yml:23:19:23:63 | ${{ ste ... iles }} | usesIds | .github/workflows/test.yml:15:9:19:6 | name: G ... d files | source | | .github/workflows/test.yml:19:9:26:6 | name: R ... d files | step | nodeLocations +| .github/workflows/test.yml:1:1:40:53 | enter on: push | .github/workflows/test.yml:1:1:40:53 | .github/workflows/test.yml@1:1:40:53 | +| .github/workflows/test.yml:1:1:40:53 | exit on: push | .github/workflows/test.yml:1:1:40:53 | .github/workflows/test.yml@1:1:40:53 | +| .github/workflows/test.yml:1:1:40:53 | exit on: push (normal) | .github/workflows/test.yml:1:1:40:53 | .github/workflows/test.yml@1:1:40:53 | | .github/workflows/test.yml:1:1:40:53 | on: push | .github/workflows/test.yml:1:1:40:53 | .github/workflows/test.yml@1:1:40:53 | | .github/workflows/test.yml:5:5:31:2 | runs-on ... -latest | .github/workflows/test.yml:5:5:31:2 | .github/workflows/test.yml@5:5:31:2 | | .github/workflows/test.yml:8:7:10:4 | job_out ... alue }} | .github/workflows/test.yml:8:7:10:4 | .github/workflows/test.yml@8:7:10:4 | diff --git a/ql/test/library-tests/test.ql b/ql/test/library-tests/test.ql index 168987284c3..7524e31f050 100644 --- a/ql/test/library-tests/test.ql +++ b/ql/test/library-tests/test.ql @@ -9,49 +9,39 @@ query predicate files(File f) { any() } query predicate yamlNodes(YamlNode n) { any() } -query predicate jobNodes(JobStmt s) { any() } +query predicate jobNodes(Job s) { any() } -query predicate stepNodes(StepStmt s) { any() } +query predicate stepNodes(Step s) { any() } -query predicate allUsesNodes(UsesExpr s) { any() } +query predicate allUsesNodes(Uses s) { any() } -query predicate stepUsesNodes(StepUsesExpr s) { any() } +query predicate stepUsesNodes(StepUses s) { any() } -query predicate jobUsesNodes(JobUsesExpr s) { any() } +query predicate jobUsesNodes(JobUses s) { any() } -query predicate usesSteps(UsesExpr call, string argname, Expression arg) { +query predicate usesSteps(Uses call, string argname, Expression arg) { call.getArgumentExpr(argname) = arg } -query predicate runSteps1(RunExpr run, string body) { run.getScript() = body } +query predicate runSteps(Run run, string body) { run.getScript() = body } -query predicate runSteps2(RunExpr run, Expression bodyExpr) { run.getScriptExpr() = bodyExpr } - -query predicate runStepChildren(RunExpr run, AstNode child) { child.getParentNode() = run } - -query predicate varAccesses(ExprAccessExpr ea, string expr) { expr = ea.getExpression() } - -query predicate orphanVarAccesses(ExprAccessExpr va, string var) { - var = va.getExpression() and - not exists(AstNode n | n = va.getParentNode()) -} - -query predicate nonOrphanVarAccesses(ExprAccessExpr va, string var, AstNode parent) { - var = va.getExpression() and - parent = va.getParentNode() -} +query predicate runStepChildren(Run run, AstNode child) { child.getParentNode() = run } query predicate parentNodes(AstNode child, AstNode parent) { child.getParentNode() = parent } -query predicate cfgNodes(Cfg::Node n) { any() } +query predicate cfgNodes(Cfg::Node n) { + n.getLocation().getFile().getBaseName() = "argus_case_study.yml" +} //any() } -query predicate dfNodes(DataFlow::Node e) { any() } +query predicate dfNodes(DataFlow::Node e) { + e.getLocation().getFile().getBaseName() = "argus_case_study.yml" +} //any() } -query predicate exprNodes(DataFlow::ExprNode e) { any() } +query predicate exprNodes(DataFlow::Node e) { any() } query predicate argumentNodes(DataFlow::ArgumentNode e) { any() } -query predicate usesIds(StepUsesExpr s, string a) { s.getId() = a } +query predicate usesIds(StepUses s, string a) { s.getId() = a } query predicate nodeLocations(DataFlow::Node n, Location l) { n.getLocation() = l } @@ -67,4 +57,4 @@ query predicate summaries(string action, string version, string input, string ou query predicate calls(DataFlow::CallNode call, string callee) { callee = call.getCallee() } -query predicate needs(DataFlow::ExprNode e) { e.asExpr() instanceof NeedsCtxAccessExpr } +query predicate needs(DataFlow::Node e) { e.asExpr() instanceof NeedsExpression }