Skip to content

Commit 6b83afe

Browse files
author
Alvaro Muñoz
authored
Merge pull request #9 from GitHubSecurityLab/content_set
feat(field-flow): enhance dataflow tracking
2 parents 32b1d77 + e6b4676 commit 6b83afe

17 files changed

+379
-340
lines changed

ql/lib/codeql/actions/Ast.qll

+106-42
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,8 @@ class OutputsStmt extends Statement instanceof YamlMapping {
9595
this.(YamlMapping).lookup(name).(YamlMapping).lookup("value") = result or
9696
this.(YamlMapping).lookup(name) = result
9797
}
98+
99+
string getAnOutputName() { this.(YamlMapping).maps(any(YamlString s | s.getValue() = result), _) }
98100
}
99101

100102
class InputExpr extends Expression instanceof YamlString {
@@ -158,6 +160,10 @@ class JobStmt extends Statement instanceof Actions::Job {
158160
* arg1: value1
159161
*/
160162
JobUsesExpr getUsesExpr() { result.getJobStmt() = this }
163+
164+
predicate usesReusableWorkflow() {
165+
this.(YamlMapping).maps(any(YamlString s | s.getValue() = "uses"), _)
166+
}
161167
}
162168

163169
/**
@@ -353,106 +359,164 @@ class ExprAccessExpr extends Expression instanceof YamlString {
353359
string getExpression() { result = expr }
354360

355361
JobStmt getJobStmt() { result.getAChildNode*() = this }
362+
}
363+
364+
/**
365+
* A context access expression.
366+
* https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability
367+
*/
368+
class CtxAccessExpr extends ExprAccessExpr {
369+
CtxAccessExpr() {
370+
expr.regexpMatch([
371+
stepsCtxRegex(), needsCtxRegex(), jobsCtxRegex(), envCtxRegex(), inputsCtxRegex()
372+
])
373+
}
374+
375+
abstract string getFieldName();
356376

357377
abstract Expression getRefExpr();
358378
}
359379

380+
private string stepsCtxRegex() { result = "steps\\.([A-Za-z0-9_-]+)\\.outputs\\.([A-Za-z0-9_-]+)" }
381+
382+
private string needsCtxRegex() { result = "needs\\.([A-Za-z0-9_-]+)\\.outputs\\.([A-Za-z0-9_-]+)" }
383+
384+
private string jobsCtxRegex() { result = "jobs\\.([A-Za-z0-9_-]+)\\.outputs\\.([A-Za-z0-9_-]+)" }
385+
386+
private string envCtxRegex() { result = "env\\.([A-Za-z0-9_-]+)" }
387+
388+
private string inputsCtxRegex() { result = "inputs\\.([A-Za-z0-9_-]+)" }
389+
360390
/**
361-
* Holds for an ExprAccessExpr accesing the `steps` context.
391+
* Holds for an expression accesing the `steps` context.
362392
* https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability
363393
* e.g. `${{ steps.changed-files.outputs.all_changed_files }}`
364394
*/
365-
class StepOutputAccessExpr extends ExprAccessExpr {
395+
class StepsCtxAccessExpr extends CtxAccessExpr {
366396
string stepId;
367-
string varName;
397+
string fieldName;
368398

369-
StepOutputAccessExpr() {
370-
stepId =
371-
this.getExpression().regexpCapture("steps\\.([A-Za-z0-9_-]+)\\.outputs\\.[A-Za-z0-9_-]+", 1) and
372-
varName =
373-
this.getExpression().regexpCapture("steps\\.[A-Za-z0-9_-]+\\.outputs\\.([A-Za-z0-9_-]+)", 1)
399+
StepsCtxAccessExpr() {
400+
expr.regexpMatch(stepsCtxRegex()) and
401+
stepId = expr.regexpCapture(stepsCtxRegex(), 1) and
402+
fieldName = expr.regexpCapture(stepsCtxRegex(), 2)
374403
}
375404

405+
override string getFieldName() { result = fieldName }
406+
376407
override Expression getRefExpr() {
377408
this.getLocation().getFile() = result.getLocation().getFile() and
378409
result.(StepStmt).getId() = stepId
379410
}
380411
}
381412

382413
/**
383-
* Holds for an ExprAccessExpr accesing the `needs` or `job` contexts.
414+
* Holds for an expression accesing the `needs` context.
415+
* https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability
416+
* e.g. `${{ needs.job1.outputs.foo}}`
417+
*/
418+
class NeedsCtxAccessExpr extends CtxAccessExpr {
419+
JobStmt job;
420+
string jobId;
421+
string fieldName;
422+
423+
NeedsCtxAccessExpr() {
424+
expr.regexpMatch(needsCtxRegex()) and
425+
jobId = expr.regexpCapture(needsCtxRegex(), 1) and
426+
fieldName = expr.regexpCapture(needsCtxRegex(), 2) and
427+
job.getId() = jobId
428+
}
429+
430+
predicate usesReusableWorkflow() { job.usesReusableWorkflow() }
431+
432+
override string getFieldName() { result = fieldName }
433+
434+
override Expression getRefExpr() {
435+
job.getLocation().getFile() = this.getLocation().getFile() and
436+
(
437+
// regular jobs
438+
job.getOutputStmt().getOutputExpr(fieldName) = result
439+
or
440+
// jobs calling reusable workflows
441+
job.getUsesExpr() = result
442+
)
443+
}
444+
}
445+
446+
/**
447+
* Holds for an expression accesing the `jobs` context.
384448
* https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability
385-
* e.g. `${{ needs.job1.outputs.foo}}` or `${{ jobs.job1.outputs.foo}}` (for reusable workflows)
449+
* e.g. `${{ jobs.job1.outputs.foo}}` (within reusable workflows)
386450
*/
387-
class JobOutputAccessExpr extends ExprAccessExpr {
451+
class JobsCtxAccessExpr extends CtxAccessExpr {
388452
string jobId;
389-
string varName;
390-
391-
JobOutputAccessExpr() {
392-
jobId =
393-
this.getExpression()
394-
.regexpCapture("(needs|jobs)\\.([A-Za-z0-9_-]+)\\.outputs\\.[A-Za-z0-9_-]+", 2) and
395-
varName =
396-
this.getExpression()
397-
.regexpCapture("(needs|jobs)\\.[A-Za-z0-9_-]+\\.outputs\\.([A-Za-z0-9_-]+)", 2)
453+
string fieldName;
454+
455+
JobsCtxAccessExpr() {
456+
expr.regexpMatch(jobsCtxRegex()) and
457+
jobId = expr.regexpCapture(jobsCtxRegex(), 1) and
458+
fieldName = expr.regexpCapture(jobsCtxRegex(), 2)
398459
}
399460

461+
override string getFieldName() { result = fieldName }
462+
400463
override Expression getRefExpr() {
401464
exists(JobStmt job |
402465
job.getId() = jobId and
403466
job.getLocation().getFile() = this.getLocation().getFile() and
404-
(
405-
// A Job can have multiple outputs, so we need to check both
406-
// jobs.<job_id>.outputs.<output_name>
407-
job.getOutputStmt().getOutputExpr(varName) = result
408-
or
409-
// jobs.<job_id>.uses (variables returned from the reusable workflow
410-
job.getUsesExpr() = result
411-
)
467+
job.getOutputStmt().getOutputExpr(fieldName) = result
412468
)
413469
}
414470
}
415471

416472
/**
417-
* Holds for an ExprAccessExpr accesing the `inputs` context.
473+
* Holds for an expression the `inputs` context.
418474
* https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability
419475
* e.g. `${{ inputs.foo }}`
420476
*/
421-
class InputAccessExpr extends ExprAccessExpr {
422-
string paramName;
477+
class InputsCtxAccessExpr extends CtxAccessExpr {
478+
string fieldName;
423479

424-
InputAccessExpr() {
425-
paramName = this.getExpression().regexpCapture("inputs\\.([A-Za-z0-9_-]+)", 1)
480+
InputsCtxAccessExpr() {
481+
expr.regexpMatch(inputsCtxRegex()) and
482+
fieldName = expr.regexpCapture(inputsCtxRegex(), 1)
426483
}
427484

485+
override string getFieldName() { result = fieldName }
486+
428487
override Expression getRefExpr() {
429488
exists(ReusableWorkflowStmt w |
430489
w.getLocation().getFile() = this.getLocation().getFile() and
431-
w.getInputsStmt().getInputExpr(paramName) = result
490+
w.getInputsStmt().getInputExpr(fieldName) = result
432491
)
433492
or
434493
exists(CompositeActionStmt a |
435494
a.getLocation().getFile() = this.getLocation().getFile() and
436-
a.getInputsStmt().getInputExpr(paramName) = result
495+
a.getInputsStmt().getInputExpr(fieldName) = result
437496
)
438497
}
439498
}
440499

441500
/**
442-
* Holds for an ExprAccessExpr accesing the `env` context.
501+
* Holds for an expression accesing the `env` context.
443502
* https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability
444503
* e.g. `${{ env.foo }}`
445504
*/
446-
class EnvAccessExpr extends ExprAccessExpr {
447-
string varName;
505+
class EnvCtxAccessExpr extends CtxAccessExpr {
506+
string fieldName;
507+
508+
EnvCtxAccessExpr() {
509+
expr.regexpMatch(envCtxRegex()) and
510+
fieldName = expr.regexpCapture(envCtxRegex(), 1)
511+
}
448512

449-
EnvAccessExpr() { varName = this.getExpression().regexpCapture("env\\.([A-Za-z0-9_-]+)", 1) }
513+
override string getFieldName() { result = fieldName }
450514

451515
override Expression getRefExpr() {
452-
exists(JobUsesExpr s | s.getEnvExpr(varName) = result)
516+
exists(JobUsesExpr s | s.getEnvExpr(fieldName) = result)
453517
or
454-
exists(StepUsesExpr s | s.getEnvExpr(varName) = result)
518+
exists(StepUsesExpr s | s.getEnvExpr(fieldName) = result)
455519
or
456-
exists(RunExpr s | s.getEnvExpr(varName) = result)
520+
exists(RunExpr s | s.getEnvExpr(fieldName) = result)
457521
}
458522
}

ql/lib/codeql/actions/ast/internal/Actions.qll

+4-2
Original file line numberDiff line numberDiff line change
@@ -294,8 +294,10 @@ module Actions {
294294
/** Gets the owner and name of the repository where the Action comes from, e.g. `actions/checkout` in `actions/checkout@v2`. */
295295
string getGitHubRepository() {
296296
result =
297-
this.getValue().regexpCapture(usesParser(), 1) + "/" +
298-
this.getValue().regexpCapture(usesParser(), 2)
297+
(
298+
this.getValue().regexpCapture(usesParser(), 1) + "/" +
299+
this.getValue().regexpCapture(usesParser(), 2)
300+
).toLowerCase()
299301
}
300302

301303
/** Gets the version reference used when checking out the Action, e.g. `v2` in `actions/checkout@v2`. */

ql/lib/codeql/actions/dataflow/ExternalFlow.qll

+67-9
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,31 @@ private import internal.ExternalFlowExtensions as Extensions
22
import codeql.actions.DataFlow
33
import actions
44

5-
/** Holds if a source model exists for the given parameters. */
5+
/**
6+
* MaD sources
7+
* Fields:
8+
* - action: Fully-qualified action name (NWO)
9+
* - version: Either '*' or a specific SHA/Tag
10+
* - output arg: To node (prefixed with either `env.` or `output.`)
11+
* - trigger: Triggering event under which this model introduces tainted data. Use `*` for any event.
12+
*/
613
predicate sourceModel(string action, string version, string output, string trigger, string kind) {
714
Extensions::sourceModel(action, version, output, trigger, kind)
815
}
916

10-
/** Holds if a sink model exists for the given parameters. */
17+
/**
18+
* MaD summaries
19+
* Fields:
20+
* - action: Fully-qualified action name (NWO)
21+
* - version: Either '*' or a specific SHA/Tag
22+
* - input arg: From node (prefixed with either `env.` or `input.`)
23+
* - output arg: To node (prefixed with either `env.` or `output.`)
24+
* - kind: Either 'Taint' or 'Value'
25+
*/
1126
predicate summaryModel(string action, string version, string input, string output, string kind) {
1227
Extensions::summaryModel(action, version, input, output, kind)
1328
}
1429

15-
/** Holds if a sink model exists for the given parameters. */
16-
predicate sinkModel(string action, string version, string input, string kind) {
17-
Extensions::sinkModel(action, version, input, kind)
18-
}
19-
2030
/**
2131
* MaD sinks
2232
* Fields:
@@ -25,15 +35,63 @@ predicate sinkModel(string action, string version, string input, string kind) {
2535
* - input arg: sink node (prefixed with either `env.` or `input.`)
2636
* - kind: sink kind
2737
*/
28-
predicate sinkNode(DataFlow::ExprNode sink, string kind) {
38+
predicate sinkModel(string action, string version, string input, string kind) {
39+
Extensions::sinkModel(action, version, input, kind)
40+
}
41+
42+
predicate externallyDefinedSource(DataFlow::Node source, string sourceType, string fieldName) {
43+
exists(UsesExpr uses, string action, string version, string trigger, string kind |
44+
sourceModel(action, version, fieldName, trigger, kind) and
45+
uses.getCallee() = action.toLowerCase() and
46+
(
47+
if version.trim() = "*"
48+
then uses.getVersion() = any(string v)
49+
else uses.getVersion() = version.trim()
50+
) and
51+
(
52+
if fieldName.trim().matches("env.%")
53+
then source.asExpr() = uses.getEnvExpr(fieldName.trim().replaceAll("env\\.", ""))
54+
else
55+
if fieldName.trim().matches("output.%")
56+
then
57+
// 'output.' is the default qualifier
58+
source.asExpr() = uses
59+
else none()
60+
) and
61+
sourceType = kind
62+
)
63+
}
64+
65+
predicate externallyDefinedSummary(DataFlow::Node pred, DataFlow::Node succ, DataFlow::ContentSet c) {
66+
exists(UsesExpr uses, string action, string version, string input, string output |
67+
c = any(DataFlow::FieldContent ct | ct.getName() = output.replaceAll("output\\.", "")) and
68+
summaryModel(action, version, input, output, "taint") and
69+
uses.getCallee() = action.toLowerCase() and
70+
(
71+
if version.trim() = "*"
72+
then uses.getVersion() = any(string v)
73+
else uses.getVersion() = version.trim()
74+
) and
75+
(
76+
if input.trim().matches("env.%")
77+
then pred.asExpr() = uses.getEnvExpr(input.trim().replaceAll("env\\.", ""))
78+
else
79+
// 'input.' is the default qualifier
80+
pred.asExpr() = uses.getArgumentExpr(input.trim().replaceAll("input\\.", ""))
81+
) and
82+
succ.asExpr() = uses
83+
)
84+
}
85+
86+
predicate externallyDefinedSink(DataFlow::ExprNode sink, string kind) {
2987
exists(UsesExpr uses, string action, string version, string input |
3088
(
3189
if input.trim().matches("env.%")
3290
then sink.asExpr() = uses.getEnvExpr(input.trim().replaceAll("input\\.", ""))
3391
else sink.asExpr() = uses.getArgumentExpr(input.trim())
3492
) and
3593
sinkModel(action, version, input, kind) and
36-
uses.getCallee() = action and
94+
uses.getCallee() = action.toLowerCase() and
3795
(
3896
if version.trim() = "*"
3997
then uses.getVersion() = any(string v)

ql/lib/codeql/actions/dataflow/FlowSources.qll

+5-31
Original file line numberDiff line numberDiff line change
@@ -126,40 +126,14 @@ private class EventSource extends RemoteFlowSource {
126126
}
127127

128128
/**
129-
* MaD sources
130-
* Fields:
131-
* - action: Fully-qualified action name (NWO)
132-
* - version: Either '*' or a specific SHA/Tag
133-
* - output arg: To node (prefixed with either `env.` or `output.`)
134-
* - trigger: Triggering event under which this model introduces tainted data. Use `*` for any event.
129+
* A Source of untrusted data defined in a MaD specification
135130
*/
136131
private class ExternallyDefinedSource extends RemoteFlowSource {
137-
string soutceType;
138-
139-
ExternallyDefinedSource() {
140-
exists(
141-
UsesExpr uses, string action, string version, string output, string trigger, string kind
142-
|
143-
sourceModel(action, version, output, trigger, kind) and
144-
uses.getCallee() = action and
145-
(
146-
if version.trim() = "*"
147-
then uses.getVersion() = any(string v)
148-
else uses.getVersion() = version.trim()
149-
) and
150-
(
151-
if output.trim().matches("env.%")
152-
then this.asExpr() = uses.getEnvExpr(output.trim().replaceAll("output\\.", ""))
153-
else
154-
// 'output.' is the default qualifier
155-
// TODO: Taint just the specified output
156-
this.asExpr() = uses
157-
) and
158-
soutceType = kind
159-
)
160-
}
132+
string sourceType;
133+
134+
ExternallyDefinedSource() { externallyDefinedSource(this, sourceType, _) }
161135

162-
override string getSourceType() { result = soutceType }
136+
override string getSourceType() { result = sourceType }
163137
}
164138

165139
/**

0 commit comments

Comments
 (0)