Skip to content

Commit 0456dcd

Browse files
author
Alvaro Muñoz
authored
Merge pull request #38 from github/expr_trigger_mapping
Ensure event sources are available for triggering events
2 parents 47a66e1 + f325d40 commit 0456dcd

9 files changed

Lines changed: 154 additions & 50 deletions

File tree

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

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,42 @@ private import internal.ExternalFlowExtensions as Extensions
22
private import codeql.actions.DataFlow
33
private import actions
44

5+
/**
6+
* MaD models for workflow details
7+
* Fields:
8+
* - path: Path to the workflow file
9+
* - trigger: Trigger for the workflow
10+
* - job: Job name
11+
* - secrets_source: Source of secrets
12+
* - permissions: Permissions for the workflow
13+
* - runner: Runner info for the workflow
14+
*/
515
predicate workflowDataModel(
6-
string path, string trigger, string job, string secrets_source, string permissions,
7-
string runner
16+
string path, string trigger, string job, string secrets_source, string permissions, string runner
817
) {
918
Extensions::workflowDataModel(path, trigger, job, secrets_source, permissions, runner)
1019
}
1120

12-
predicate repositoryDataModel(
13-
string visibility, string default_branch_name
14-
) {
21+
/**
22+
* MaD models for repository details
23+
* Fields:
24+
* - visibility: Visibility of the repository
25+
* - default_branch_name: Default branch name
26+
*/
27+
predicate repositoryDataModel(string visibility, string default_branch_name) {
1528
Extensions::repositoryDataModel(visibility, default_branch_name)
1629
}
1730

31+
/**
32+
* MaD models for context/trigger mapping
33+
* Fields:
34+
* - trigger: Trigger for the workflow
35+
* - context_prefix: Prefix for the context
36+
*/
37+
predicate contextTriggerDataModel(string trigger, string context_prefix) {
38+
Extensions::contextTriggerDataModel(trigger, context_prefix)
39+
}
40+
1841
/**
1942
* MaD sources
2043
* Fields:

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

Lines changed: 55 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,7 @@ private predicate branchEvent(string context) {
9595
// - They cannot contain a \
9696
// eg: zzz";echo${IFS}"hello";# would be a valid branch name
9797
"github\\.event\\.pull_request\\.head\\.repo\\.default_branch",
98-
"github\\.event\\.pull_request\\.head\\.ref", "github\\.head_ref",
99-
"github\\.event\\.workflow_run\\.head_branch",
98+
"github\\.event\\.pull_request\\.head\\.ref", "github\\.event\\.workflow_run\\.head_branch",
10099
"github\\.event\\.workflow_run\\.pull_requests\\[[0-9]+\\]\\.head\\.ref",
101100
"github\\.event\\.merge_group\\.head_ref",
102101
]
@@ -165,7 +164,8 @@ private predicate pathEvent(string context) {
165164
reg =
166165
[
167166
// filename
168-
"github\\.event\\.workflow\\.path",
167+
"github\\.event\\.workflow\\.path", "github\\.event\\.workflow_run\\.path",
168+
"github\\.event\\.workflow_run\\.referenced_workflows\\.path",
169169
]
170170
|
171171
Utils::normalizeExpr(context).regexpMatch(Utils::wrapRegexp(reg))
@@ -197,11 +197,33 @@ private predicate jsonEvent(string context) {
197197
)
198198
}
199199

200-
class EventSource extends RemoteFlowSource {
200+
class GitHubSource extends RemoteFlowSource {
201201
string flag;
202202

203-
EventSource() {
204-
exists(Expression e, string context | this.asExpr() = e and context = e.getExpression() |
203+
GitHubSource() {
204+
exists(Expression e, string context, string context_prefix |
205+
this.asExpr() = e and
206+
context = e.getExpression() and
207+
Utils::normalizeExpr(context) = "github.head_ref" and
208+
contextTriggerDataModel(e.getEnclosingWorkflow().getATriggerEvent().getName(), context_prefix) and
209+
Utils::normalizeExpr(context).matches("%" + context_prefix + "%") and
210+
flag = "branch"
211+
)
212+
}
213+
214+
override string getSourceType() { result = flag }
215+
}
216+
217+
class GitHubEventSource extends RemoteFlowSource {
218+
string flag;
219+
220+
GitHubEventSource() {
221+
exists(Expression e, string context, string context_prefix |
222+
this.asExpr() = e and
223+
context = e.getExpression() and
224+
contextTriggerDataModel(e.getEnclosingWorkflow().getATriggerEvent().getName(), context_prefix) and
225+
Utils::normalizeExpr(context).matches("%" + context_prefix + "%")
226+
|
205227
titleEvent(context) and flag = "title"
206228
or
207229
urlEvent(context) and flag = "url"
@@ -217,8 +239,33 @@ class EventSource extends RemoteFlowSource {
217239
usernameEvent(context) and flag = "username"
218240
or
219241
pathEvent(context) and flag = "filename"
220-
or
221-
jsonEvent(context) and flag = "json"
242+
)
243+
}
244+
245+
override string getSourceType() { result = flag }
246+
}
247+
248+
class GitHubEventJsonSource extends RemoteFlowSource {
249+
string flag;
250+
251+
GitHubEventJsonSource() {
252+
exists(Expression e, string context |
253+
this.asExpr() = e and
254+
context = e.getExpression() and
255+
(
256+
jsonEvent(context) and
257+
(
258+
exists(string context_prefix |
259+
contextTriggerDataModel(e.getEnclosingWorkflow().getATriggerEvent().getName(),
260+
context_prefix) and
261+
Utils::normalizeExpr(context).matches("%" + context_prefix + "%")
262+
)
263+
or
264+
contextTriggerDataModel(e.getEnclosingWorkflow().getATriggerEvent().getName(), _) and
265+
Utils::normalizeExpr(context).regexpMatch(".*\\bgithub.event\\b.*")
266+
)
267+
) and
268+
flag = "json"
222269
)
223270
}
224271

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,19 @@ extensible predicate sinkModel(
2323
string action, string version, string input, string kind, string provenance
2424
);
2525

26+
/**
27+
* Holds if workflow data model exists for the given parameters.
28+
*/
2629
extensible predicate workflowDataModel(
27-
string path, string trigger, string job, string secrets_source, string permissions,
28-
string runner
30+
string path, string trigger, string job, string secrets_source, string permissions, string runner
2931
);
3032

31-
extensible predicate repositoryDataModel(
32-
string visibility, string default_branch_name
33-
);
33+
/**
34+
* Holds if repository data model exists for the given parameters.
35+
*/
36+
extensible predicate repositoryDataModel(string visibility, string default_branch_name);
37+
38+
/**
39+
* Holds if context/trigger mapping exists for the given parameters.
40+
*/
41+
extensible predicate contextTriggerDataModel(string trigger, string context_prefix);

‎ql/lib/ext/workflow-models/workflow-models.yml‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,51 @@ extensions:
77
pack: githubsecuritylab/actions-all
88
extensible: workflowDataModel
99
data: []
10+
- addsTo:
11+
pack: githubsecuritylab/actions-all
12+
extensible: contextTriggerDataModel
13+
data:
14+
# This predicate maps triggering events with the github event context available for that event
15+
- ["commit_comment", "github.event.comment"]
16+
- ["discussion", "github.event.discussion"]
17+
- ["discussion_comment", "github.event.comment"]
18+
- ["discussion_comment", "github.event.discussion"]
19+
- ["issues", "github.event.issue"]
20+
- ["issue_comment", "github.event.issue"]
21+
- ["issue_comment", "github.event.comment"]
22+
- ["gollum", "github.event.pages"]
23+
- ["merge_group", "github.event.merge_group"]
24+
- ["pull_request", "github.event.pull_request"]
25+
- ["pull_request", "github.head_ref"]
26+
- ["pull_request_comment", "github.event.comment"]
27+
- ["pull_request_comment", "github.event.pull_request"]
28+
- ["pull_request_comment", "github.head_ref"]
29+
- ["pull_request_review", "github.event.pull_request"]
30+
- ["pull_request_review", "github.event.review"]
31+
- ["pull_request_review", "github.head_ref"]
32+
- ["pull_request_review_comment", "github.event.comment"]
33+
- ["pull_request_review_comment", "github.event.pull_request"]
34+
- ["pull_request_review_comment", "github.event.review"]
35+
- ["pull_request_review_comment", "github.head_ref"]
36+
- ["pull_request_target", "github.event.pull_request"]
37+
- ["pull_request_target", "github.head_ref"]
38+
- ["push", "github.event.commits"]
39+
- ["push", "github.event.head_commit"]
40+
- ["repository_dispatch", "github.event.client_payload"]
41+
- ["workflow_dispatch", "github.event.inputs"]
42+
- ["workflow_run", "github.event.workflow"]
43+
- ["workflow_run", "github.event.workflow_run"]
44+
# workflow_call receives the same event payload as the calling workflow
45+
- ["workflow_call", "github.event.client_payload"]
46+
- ["workflow_call", "github.event.comment"]
47+
- ["workflow_call", "github.event.commits"]
48+
- ["workflow_call", "github.event.discussion"]
49+
- ["workflow_call", "github.event.head_commit"]
50+
- ["workflow_call", "github.event.inputs"]
51+
- ["workflow_call", "github.event.issue"]
52+
- ["workflow_call", "github.event.merge_group"]
53+
- ["workflow_call", "github.event.pages"]
54+
- ["workflow_call", "github.event.pull_request"]
55+
- ["workflow_call", "github.event.review"]
56+
- ["workflow_call", "github.event.workflow"]
57+
- ["workflow_call", "github.event.workflow_run"]

‎ql/test/query-tests/Security/CWE-094/.github/workflows/pull_request_target.yml‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ jobs:
44
echo-chamber:
55
runs-on: ubuntu-latest
66
steps:
7-
- run: echo '${{ github.event.issue.title }}' # not defined for this trigger, but we will still report it
8-
- run: echo '${{ github.event.issue.body }}' # not defined for this trigger, but we will still report it
7+
- run: echo '${{ github.event.issue.title }}' # not defined for this trigger, so we should not report it
8+
- run: echo '${{ github.event.issue.body }}' # not defined for this trigger, so we should not report it
99
- run: echo '${{ github.event.pull_request.title }}'
1010
- run: echo '${{ github.event.pull_request.body }}'
1111
- run: echo '${{ github.event.pull_request.head.label }}'
@@ -14,3 +14,4 @@ jobs:
1414
- run: echo '${{ github.event.pull_request.head.repo.homepage }}'
1515
- run: echo '${{ github.event.pull_request.head.ref }}'
1616
- run: echo '${{ github.head_ref }}'
17+

‎ql/test/query-tests/Security/CWE-094/.github/workflows/self_needs.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ jobs:
1313
- id: source
1414
uses: mad9000/actions-find-and-replace-string@3
1515
with:
16-
source: ${{ github.event['head_commit']['message'] }}
16+
source: ${{ github.event['comment']['body'] }}
1717
find: 'foo'
1818
replace: ''
1919
- run: ${{ steps.source.outputs.value }}

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ edges
5050
| .github/workflows/self_needs.yml:11:20:11:52 | steps.source.outputs.value | .github/workflows/self_needs.yml:11:7:12:4 | Job outputs node [job_output] |
5151
| .github/workflows/self_needs.yml:13:9:19:6 | Uses Step: source [value] | .github/workflows/self_needs.yml:11:20:11:52 | steps.source.outputs.value |
5252
| .github/workflows/self_needs.yml:13:9:19:6 | Uses Step: source [value] | .github/workflows/self_needs.yml:19:15:19:47 | steps.source.outputs.value |
53-
| .github/workflows/self_needs.yml:16:20:16:64 | github.event['head_commit']['message'] | .github/workflows/self_needs.yml:13:9:19:6 | Uses Step: source [value] |
53+
| .github/workflows/self_needs.yml:16:20:16:57 | github.event['comment']['body'] | .github/workflows/self_needs.yml:13:9:19:6 | Uses Step: source [value] |
5454
| .github/workflows/simple1.yml:8:9:14:6 | Uses Step: summary [value] | .github/workflows/simple1.yml:16:18:16:49 | steps.summary.outputs.value |
5555
| .github/workflows/simple1.yml:11:20:11:58 | github.event.head_commit.message | .github/workflows/simple1.yml:8:9:14:6 | Uses Step: summary [value] |
5656
| .github/workflows/simple2.yml:14:9:18:6 | Uses Step: source | .github/workflows/simple2.yml:22:20:22:64 | steps.source.outputs.all_changed_files |
@@ -183,8 +183,6 @@ nodes
183183
| .github/workflows/pull_request_review_comment.yml:12:19:12:69 | github.event.pull_request.head.repo.homepage | semmle.label | github.event.pull_request.head.repo.homepage |
184184
| .github/workflows/pull_request_review_comment.yml:13:19:13:59 | github.event.pull_request.head.ref | semmle.label | github.event.pull_request.head.ref |
185185
| .github/workflows/pull_request_review_comment.yml:14:19:14:50 | github.event.comment.body | semmle.label | github.event.comment.body |
186-
| .github/workflows/pull_request_target.yml:7:19:7:49 | github.event.issue.title | semmle.label | github.event.issue.title |
187-
| .github/workflows/pull_request_target.yml:8:19:8:48 | github.event.issue.body | semmle.label | github.event.issue.body |
188186
| .github/workflows/pull_request_target.yml:9:19:9:56 | github.event.pull_request.title | semmle.label | github.event.pull_request.title |
189187
| .github/workflows/pull_request_target.yml:10:19:10:55 | github.event.pull_request.body | semmle.label | github.event.pull_request.body |
190188
| .github/workflows/pull_request_target.yml:11:19:11:61 | github.event.pull_request.head.label | semmle.label | github.event.pull_request.head.label |
@@ -206,7 +204,7 @@ nodes
206204
| .github/workflows/self_needs.yml:11:7:12:4 | Job outputs node [job_output] | semmle.label | Job outputs node [job_output] |
207205
| .github/workflows/self_needs.yml:11:20:11:52 | steps.source.outputs.value | semmle.label | steps.source.outputs.value |
208206
| .github/workflows/self_needs.yml:13:9:19:6 | Uses Step: source [value] | semmle.label | Uses Step: source [value] |
209-
| .github/workflows/self_needs.yml:16:20:16:64 | github.event['head_commit']['message'] | semmle.label | github.event['head_commit']['message'] |
207+
| .github/workflows/self_needs.yml:16:20:16:57 | github.event['comment']['body'] | semmle.label | github.event['comment']['body'] |
210208
| .github/workflows/self_needs.yml:19:15:19:47 | steps.source.outputs.value | semmle.label | steps.source.outputs.value |
211209
| .github/workflows/self_needs.yml:20:15:20:51 | needs.test1.outputs.job_output | semmle.label | needs.test1.outputs.job_output |
212210
| .github/workflows/simple1.yml:8:9:14:6 | Uses Step: summary [value] | semmle.label | Uses Step: summary [value] |
@@ -254,12 +252,10 @@ nodes
254252
| .github/workflows/workflow_run.yml:14:19:14:77 | github.event.workflow_run.head_commit.committer.name | semmle.label | github.event.workflow_run.head_commit.committer.name |
255253
| .github/workflows/workflow_run.yml:15:19:15:62 | github.event.workflow_run.head_branch | semmle.label | github.event.workflow_run.head_branch |
256254
| .github/workflows/workflow_run.yml:16:19:16:78 | github.event.workflow_run.head_repository.description | semmle.label | github.event.workflow_run.head_repository.description |
257-
| action1/action.yml:14:19:14:50 | github.event.comment.body | semmle.label | github.event.comment.body |
258255
subpaths
259256
#select
260257
| .github/workflows/changed-files.yml:20:24:20:76 | steps.changed-files1.outputs.all_changed_files | .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 | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/changed-files.yml:20:24:20:76 | steps.changed-files1.outputs.all_changed_files | ${{ steps.changed-files1.outputs.all_changed_files }} |
261258
| .github/workflows/changed-files.yml:40:24:40:76 | steps.changed-files3.outputs.all_changed_files | .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 | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/changed-files.yml:40:24:40:76 | steps.changed-files3.outputs.all_changed_files | ${{ steps.changed-files3.outputs.all_changed_files }} |
262259
| .github/workflows/changed-files.yml:58:24:58:76 | steps.changed-files5.outputs.all_changed_files | .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 | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/changed-files.yml:58:24:58:76 | steps.changed-files5.outputs.all_changed_files | ${{ steps.changed-files5.outputs.all_changed_files }} |
263260
| .github/workflows/changelog.yml:58:26:58:39 | env.log | .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog.yml:58:26:58:39 | env.log | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/changelog.yml:58:26:58:39 | env.log | ${{ env.log }} |
264261
| .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | .github/workflows/changelog_from_prt.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | ${{ env.log }} |
265-
| action1/action.yml:14:19:14:50 | github.event.comment.body | action1/action.yml:14:19:14:50 | github.event.comment.body | action1/action.yml:14:19:14:50 | github.event.comment.body | Potential code injection in $@, which may be controlled by an external user. | action1/action.yml:14:19:14:50 | github.event.comment.body | ${{ github.event.comment.body }} |

0 commit comments

Comments
 (0)