Skip to content

Commit b685a8d

Browse files
author
Alvaro Muñoz
authored
Merge pull request #86 from github/analyze_reusable_workflows
Cross remote Reusable Workflow analysis
2 parents a1e44bc + d44e7ae commit b685a8d

File tree

10 files changed

+147
-23
lines changed

10 files changed

+147
-23
lines changed

‎ql/lib/codeql/actions/Helper.qll‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,10 +252,26 @@ predicate inPrivilegedExternallyTriggerableJob(AstNode node) {
252252
)
253253
}
254254

255+
predicate calledByPrivilegedExternallyTriggerableJob(AstNode node) {
256+
exists(ReusableWorkflow rw, ExternalJob caller, Job callee |
257+
callee = node.getEnclosingJob() and
258+
rw.getACaller() = caller and
259+
rw.getAJob() = callee and
260+
caller.isPrivilegedExternallyTriggerable()
261+
)
262+
or
263+
exists(LocalJob caller |
264+
caller = node.getEnclosingCompositeAction().getACallerJob() and
265+
caller.isPrivilegedExternallyTriggerable()
266+
)
267+
}
268+
255269
predicate inPrivilegedContext(AstNode node) {
256270
inPrivilegedCompositeAction(node)
257271
or
258272
inPrivilegedExternallyTriggerableJob(node)
273+
or
274+
calledByPrivilegedExternallyTriggerableJob(node)
259275
}
260276

261277
predicate inNonPrivilegedCompositeAction(AstNode node) {

‎ql/lib/codeql/actions/dataflow/internal/DataFlowPrivate.qll‎

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,23 @@ class DataFlowCall instanceof Cfg::Node {
8989
Location getLocation() { result = this.(Cfg::Node).getLocation() }
9090
}
9191

92+
string getRepoRoot() {
93+
exists(Workflow w |
94+
w.getLocation().getFile().getRelativePath().indexOf("/.github/workflows") > 0 and
95+
result =
96+
w.getLocation()
97+
.getFile()
98+
.getRelativePath()
99+
.prefix(w.getLocation().getFile().getRelativePath().indexOf("/.github/workflows") + 1) and
100+
// exclude workflow_enum reusable workflows directory root
101+
not result.indexOf(".github/reusable_workflows/") > -1
102+
or
103+
not w.getLocation().getFile().getRelativePath().indexOf("/.github/workflows") > 0 and
104+
not w.getLocation().getFile().getRelativePath().indexOf(".github/reusable_workflows") > -1 and
105+
result = ""
106+
)
107+
}
108+
92109
/**
93110
* A Cfg scope that can be called
94111
*/
@@ -97,28 +114,7 @@ class DataFlowCallable instanceof Cfg::CfgScope {
97114

98115
string getName() {
99116
if this instanceof ReusableWorkflow
100-
then
101-
//result = this.(ReusableWorkflow).getLocation().getFile().getRelativePath()
102-
result =
103-
this.(ReusableWorkflow)
104-
.getLocation()
105-
.getFile()
106-
.getRelativePath()
107-
.suffix(this.(ReusableWorkflow)
108-
.getLocation()
109-
.getFile()
110-
.getRelativePath()
111-
.indexOf("/.github/workflows") + 1) or
112-
result =
113-
this.(ReusableWorkflow)
114-
.getLocation()
115-
.getFile()
116-
.getRelativePath()
117-
.suffix(this.(ReusableWorkflow)
118-
.getLocation()
119-
.getFile()
120-
.getRelativePath()
121-
.indexOf(".github/workflows"))
117+
then result = this.(ReusableWorkflow).getLocation().getFile().getRelativePath() // or
122118
else
123119
if this instanceof CompositeAction
124120
then
@@ -154,7 +150,13 @@ class NormalReturn extends ReturnKind, TNormalReturn {
154150
}
155151

156152
/** Gets a viable implementation of the target of the given `Call`. */
157-
DataFlowCallable viableCallable(DataFlowCall c) { c.getName() = result.getName() }
153+
DataFlowCallable viableCallable(DataFlowCall c) {
154+
c.getName() = result.getName() or
155+
c.getName() = result.getName().replaceAll(getRepoRoot(), "") or
156+
// special case for reusable workflows downloaded by the workflow_enum action
157+
c.getName() =
158+
result.getName().replaceAll(getRepoRoot(), "").replaceAll(".github/reusable_workflows/", "")
159+
}
158160

159161
/**
160162
* Gets a node that can read the value returned from `call` with return kind

‎ql/test/query-tests/Security/CWE-094/CodeInjectionCritical.expected‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
edges
2+
| .github/actions/action5/action.yml:4:3:4:7 | input taint | .github/actions/action5/action.yml:26:19:26:37 | inputs.taint | provenance | |
23
| .github/workflows/argus_case_study.yml:15:9:24:6 | Uses Step: remove_quotations [replaced] | .github/workflows/argus_case_study.yml:27:33:27:77 | steps.remove_quotations.outputs.replaced | provenance | |
34
| .github/workflows/argus_case_study.yml:17:25:17:53 | github.event.issue.title | .github/workflows/argus_case_study.yml:22:20:22:39 | env.ISSUE_TITLE | provenance | |
45
| .github/workflows/argus_case_study.yml:22:20:22:39 | env.ISSUE_TITLE | .github/workflows/argus_case_study.yml:15:9:24:6 | Uses Step: remove_quotations [replaced] | provenance | |
@@ -29,6 +30,7 @@ edges
2930
| .github/workflows/changed-files.yml:15:9:18:6 | Uses Step: changed-files1 | .github/workflows/changed-files.yml:20:24:20:76 | steps.changed-files1.outputs.all_changed_files | provenance | |
3031
| .github/workflows/changed-files.yml:33:9:38:6 | Uses Step: changed-files3 | .github/workflows/changed-files.yml:40:24:40:76 | steps.changed-files3.outputs.all_changed_files | provenance | |
3132
| .github/workflows/changed-files.yml:53:9:56:6 | Uses Step: changed-files5 | .github/workflows/changed-files.yml:58:24:58:76 | steps.changed-files5.outputs.all_changed_files | provenance | |
33+
| .github/workflows/composite-action-caller-3.yml:12:19:12:50 | github.event.comment.body | .github/actions/action5/action.yml:4:3:4:7 | input taint | provenance | |
3234
| .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] | .github/workflows/cross3.yml:39:31:39:75 | steps.remove_quotations.outputs.replaced | provenance | |
3335
| .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] | .github/workflows/cross3.yml:57:29:57:73 | steps.remove_quotations.outputs.replaced | provenance | |
3436
| .github/workflows/cross3.yml:32:18:32:53 | github.event.commits[0].message | .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] | provenance | |
@@ -126,7 +128,9 @@ nodes
126128
| .github/actions/action1/action.yml:7:19:7:55 | github.event.pull_request.body | semmle.label | github.event.pull_request.body |
127129
| .github/actions/action3/action.yml:9:19:9:55 | github.event.pull_request.body | semmle.label | github.event.pull_request.body |
128130
| .github/actions/action4/action.yml:7:19:7:55 | github.event.pull_request.body | semmle.label | github.event.pull_request.body |
131+
| .github/actions/action5/action.yml:4:3:4:7 | input taint | semmle.label | input taint |
129132
| .github/actions/action5/action.yml:16:19:16:55 | github.event.pull_request.body | semmle.label | github.event.pull_request.body |
133+
| .github/actions/action5/action.yml:26:19:26:37 | inputs.taint | semmle.label | inputs.taint |
130134
| .github/workflows/argus_case_study.yml:15:9:24:6 | Uses Step: remove_quotations [replaced] | semmle.label | Uses Step: remove_quotations [replaced] |
131135
| .github/workflows/argus_case_study.yml:17:25:17:53 | github.event.issue.title | semmle.label | github.event.issue.title |
132136
| .github/workflows/argus_case_study.yml:22:20:22:39 | env.ISSUE_TITLE | semmle.label | env.ISSUE_TITLE |
@@ -179,6 +183,7 @@ nodes
179183
| .github/workflows/comment_issue_newline.yml:10:25:10:56 | github.event.comment.body | semmle.label | github.event.comment.body |
180184
| .github/workflows/comment_issue_newline.yml:11:24:11:51 | github.event.issue.body | semmle.label | github.event.issue.body |
181185
| .github/workflows/comment_issue_newline.yml:12:24:12:55 | github.event.comment.body | semmle.label | github.event.comment.body |
186+
| .github/workflows/composite-action-caller-3.yml:12:19:12:50 | github.event.comment.body | semmle.label | github.event.comment.body |
182187
| .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] | semmle.label | Uses Step: remove_quotations [replaced] |
183188
| .github/workflows/cross3.yml:32:18:32:53 | github.event.commits[0].message | semmle.label | github.event.commits[0].message |
184189
| .github/workflows/cross3.yml:39:31:39:75 | steps.remove_quotations.outputs.replaced | semmle.label | steps.remove_quotations.outputs.replaced |
@@ -385,6 +390,7 @@ subpaths
385390
#select
386391
| .github/actions/action1/action.yml:7:19:7:55 | github.event.pull_request.body | .github/actions/action1/action.yml:7:19:7:55 | github.event.pull_request.body | .github/actions/action1/action.yml:7:19:7:55 | github.event.pull_request.body | Potential code injection in $@, which may be controlled by an external user. | .github/actions/action1/action.yml:7:19:7:55 | github.event.pull_request.body | ${{ github.event.pull_request.body }} |
387392
| .github/actions/action5/action.yml:16:19:16:55 | github.event.pull_request.body | .github/actions/action5/action.yml:16:19:16:55 | github.event.pull_request.body | .github/actions/action5/action.yml:16:19:16:55 | github.event.pull_request.body | Potential code injection in $@, which may be controlled by an external user. | .github/actions/action5/action.yml:16:19:16:55 | github.event.pull_request.body | ${{ github.event.pull_request.body }} |
393+
| .github/actions/action5/action.yml:26:19:26:37 | inputs.taint | .github/workflows/composite-action-caller-3.yml:12:19:12:50 | github.event.comment.body | .github/actions/action5/action.yml:26:19:26:37 | inputs.taint | Potential code injection in $@, which may be controlled by an external user. | .github/actions/action5/action.yml:26:19:26:37 | inputs.taint | ${{ inputs.taint }} |
388394
| .github/workflows/argus_case_study.yml:27:33:27:77 | steps.remove_quotations.outputs.replaced | .github/workflows/argus_case_study.yml:17:25:17:53 | github.event.issue.title | .github/workflows/argus_case_study.yml:27:33:27:77 | steps.remove_quotations.outputs.replaced | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/argus_case_study.yml:27:33:27:77 | steps.remove_quotations.outputs.replaced | ${{steps.remove_quotations.outputs.replaced}} |
389395
| .github/workflows/artifactpoisoning1.yml:27:67:27:92 | steps.pr.outputs.id | .github/workflows/artifactpoisoning1.yml:14:9:20:6 | Uses Step | .github/workflows/artifactpoisoning1.yml:27:67:27:92 | steps.pr.outputs.id | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/artifactpoisoning1.yml:27:67:27:92 | steps.pr.outputs.id | ${{ steps.pr.outputs.id }} |
390396
| .github/workflows/artifactpoisoning2.yml:22:17:22:42 | steps.pr.outputs.id | .github/workflows/artifactpoisoning2.yml:13:9:19:6 | Uses Step: pr | .github/workflows/artifactpoisoning2.yml:22:17:22:42 | steps.pr.outputs.id | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/artifactpoisoning2.yml:22:17:22:42 | steps.pr.outputs.id | ${{ steps.pr.outputs.id }} |

‎ql/test/query-tests/Security/CWE-094/CodeInjectionMedium.expected‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
edges
2+
| .github/actions/action5/action.yml:4:3:4:7 | input taint | .github/actions/action5/action.yml:26:19:26:37 | inputs.taint | provenance | |
23
| .github/workflows/argus_case_study.yml:15:9:24:6 | Uses Step: remove_quotations [replaced] | .github/workflows/argus_case_study.yml:27:33:27:77 | steps.remove_quotations.outputs.replaced | provenance | |
34
| .github/workflows/argus_case_study.yml:17:25:17:53 | github.event.issue.title | .github/workflows/argus_case_study.yml:22:20:22:39 | env.ISSUE_TITLE | provenance | |
45
| .github/workflows/argus_case_study.yml:22:20:22:39 | env.ISSUE_TITLE | .github/workflows/argus_case_study.yml:15:9:24:6 | Uses Step: remove_quotations [replaced] | provenance | |
@@ -29,6 +30,7 @@ edges
2930
| .github/workflows/changed-files.yml:15:9:18:6 | Uses Step: changed-files1 | .github/workflows/changed-files.yml:20:24:20:76 | steps.changed-files1.outputs.all_changed_files | provenance | |
3031
| .github/workflows/changed-files.yml:33:9:38:6 | Uses Step: changed-files3 | .github/workflows/changed-files.yml:40:24:40:76 | steps.changed-files3.outputs.all_changed_files | provenance | |
3132
| .github/workflows/changed-files.yml:53:9:56:6 | Uses Step: changed-files5 | .github/workflows/changed-files.yml:58:24:58:76 | steps.changed-files5.outputs.all_changed_files | provenance | |
33+
| .github/workflows/composite-action-caller-3.yml:12:19:12:50 | github.event.comment.body | .github/actions/action5/action.yml:4:3:4:7 | input taint | provenance | |
3234
| .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] | .github/workflows/cross3.yml:39:31:39:75 | steps.remove_quotations.outputs.replaced | provenance | |
3335
| .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] | .github/workflows/cross3.yml:57:29:57:73 | steps.remove_quotations.outputs.replaced | provenance | |
3436
| .github/workflows/cross3.yml:32:18:32:53 | github.event.commits[0].message | .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] | provenance | |
@@ -126,7 +128,9 @@ nodes
126128
| .github/actions/action1/action.yml:7:19:7:55 | github.event.pull_request.body | semmle.label | github.event.pull_request.body |
127129
| .github/actions/action3/action.yml:9:19:9:55 | github.event.pull_request.body | semmle.label | github.event.pull_request.body |
128130
| .github/actions/action4/action.yml:7:19:7:55 | github.event.pull_request.body | semmle.label | github.event.pull_request.body |
131+
| .github/actions/action5/action.yml:4:3:4:7 | input taint | semmle.label | input taint |
129132
| .github/actions/action5/action.yml:16:19:16:55 | github.event.pull_request.body | semmle.label | github.event.pull_request.body |
133+
| .github/actions/action5/action.yml:26:19:26:37 | inputs.taint | semmle.label | inputs.taint |
130134
| .github/workflows/argus_case_study.yml:15:9:24:6 | Uses Step: remove_quotations [replaced] | semmle.label | Uses Step: remove_quotations [replaced] |
131135
| .github/workflows/argus_case_study.yml:17:25:17:53 | github.event.issue.title | semmle.label | github.event.issue.title |
132136
| .github/workflows/argus_case_study.yml:22:20:22:39 | env.ISSUE_TITLE | semmle.label | env.ISSUE_TITLE |
@@ -179,6 +183,7 @@ nodes
179183
| .github/workflows/comment_issue_newline.yml:10:25:10:56 | github.event.comment.body | semmle.label | github.event.comment.body |
180184
| .github/workflows/comment_issue_newline.yml:11:24:11:51 | github.event.issue.body | semmle.label | github.event.issue.body |
181185
| .github/workflows/comment_issue_newline.yml:12:24:12:55 | github.event.comment.body | semmle.label | github.event.comment.body |
186+
| .github/workflows/composite-action-caller-3.yml:12:19:12:50 | github.event.comment.body | semmle.label | github.event.comment.body |
182187
| .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] | semmle.label | Uses Step: remove_quotations [replaced] |
183188
| .github/workflows/cross3.yml:32:18:32:53 | github.event.commits[0].message | semmle.label | github.event.commits[0].message |
184189
| .github/workflows/cross3.yml:39:31:39:75 | steps.remove_quotations.outputs.replaced | semmle.label | steps.remove_quotations.outputs.replaced |

‎ql/test/query-tests/Security/CWE-829/.github/reusable_workflows/TestOrg/TestRepo/.github/workflows/reusable.yml‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
name: Test
2+
3+
on:
4+
workflow_call:
5+
inputs:
6+
branch:
7+
type: string
8+
default: "**"
9+
10+
defaults:
11+
run:
12+
shell: bash
13+
14+
jobs:
15+
test:
16+
name: Checkout
17+
runs-on: ubuntu-latest
18+
19+
permissions:
20+
contents: write
21+
pull-requests: write
22+
steps:
23+
- uses: actions/checkout@v2
24+
with:
25+
ref: ${{ inputs.branch }}
26+
- run: |
27+
npm install
28+
npm run lint
29+

‎ql/test/query-tests/Security/CWE-829/.github/workflows/reusable_caller1.yaml‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
name: assets-test
2+
3+
on:
4+
pull_request_target:
5+
6+
jobs:
7+
check-execution-context:
8+
uses: TestOrg/TestRepo/.github/workflows/reusable.yml@main
9+
with:
10+
branch: ${{ github.event.pull_request.head.ref }}
11+

‎ql/test/query-tests/Security/CWE-829/.github/workflows/reusable_caller2.yaml‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
name: assets-test
2+
3+
on:
4+
pull_request:
5+
6+
jobs:
7+
check-execution-context:
8+
uses: TestOrg/TestRepo/.github/workflows/reusable.yml@main
9+
with:
10+
branch: ${{ github.event.pull_request.head.ref }}
11+

‎ql/test/query-tests/Security/CWE-829/.github/workflows/reusable_caller3.yaml‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
name: assets-test
2+
3+
on:
4+
pull_request:
5+
6+
jobs:
7+
check-execution-context:
8+
uses: ./.github/workflows/reusable_local.yml
9+
with:
10+
branch: ${{ github.event.pull_request.head.ref }}
11+

‎ql/test/query-tests/Security/CWE-829/.github/workflows/reusable_local.yml‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
name: Test
2+
3+
on:
4+
workflow_call:
5+
inputs:
6+
branch:
7+
type: string
8+
default: "**"
9+
10+
defaults:
11+
run:
12+
shell: bash
13+
14+
jobs:
15+
test:
16+
name: Checkout
17+
runs-on: ubuntu-latest
18+
19+
permissions:
20+
contents: write
21+
pull-requests: write
22+
steps:
23+
- uses: actions/checkout@v2
24+
with:
25+
ref: ${{ inputs.branch }}
26+
- run: |
27+
npm install
28+
npm run lint
29+

0 commit comments

Comments
 (0)