Skip to content

Commit 8ea4182

Browse files
committed
Look for script injections in actions/github-script
1 parent e941218 commit 8ea4182

File tree

4 files changed

+69
-20
lines changed

4 files changed

+69
-20
lines changed

javascript/ql/lib/semmle/javascript/Actions.qll

Lines changed: 34 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,40 @@ module Actions {
244244
With getWith() { result = with }
245245
}
246246

247+
/**
248+
* Holds if `${{ e }}` is a GitHub Actions expression evaluated within this YAML string.
249+
* See https://docs.github.com/en/free-pro-team@latest/actions/reference/context-and-expression-syntax-for-github-actions.
250+
* Only finds simple expressions like `${{ github.event.comment.body }}`, where the expression contains only alphanumeric characters, underscores, dots, or dashes.
251+
* Does not identify more complicated expressions like `${{ fromJSON(env.time) }}`, or ${{ format('{{Hello {0}!}}', github.event.head_commit.author.name) }}
252+
*/
253+
string getASimpleReferenceExpression(YamlString node) {
254+
// We use `regexpFind` to obtain *all* matches of `${{...}}`,
255+
// not just the last (greedy match) or first (reluctant match).
256+
result =
257+
node.getValue()
258+
.regexpFind("\\$\\{\\{\\s*[A-Za-z0-9_\\[\\]\\*\\(\\)\\.\\-]+\\s*\\}\\}", _, _)
259+
.regexpCapture("\\$\\{\\{\\s*([A-Za-z0-9_\\[\\]\\*\\((\\)\\.\\-]+)\\s*\\}\\}", 1)
260+
}
261+
262+
/**
263+
* A `script:` field within an Actions `with:` specific to `actions/github-script` action.
264+
*
265+
* For example:
266+
* ```
267+
* uses: actions/github-script@v3
268+
* with:
269+
* script: console.log('${{ github.event.pull_request.head.sha }}')
270+
* ```
271+
*/
272+
class Script extends YamlNode, YamlString {
273+
With with;
274+
275+
Script() { with.lookup("script") = this }
276+
277+
/** Gets the `with` field this field belongs to. */
278+
With getWith() { result = with }
279+
}
280+
247281
/**
248282
* A `run` field within an Actions job step, which runs command-line programs using an operating system shell.
249283
* See https://docs.github.com/en/free-pro-team@latest/actions/reference/workflow-syntax-for-github-actions#jobsjob_idstepsrun.
@@ -255,20 +289,5 @@ module Actions {
255289

256290
/** Gets the step that executes this `run` command. */
257291
Step getStep() { result = step }
258-
259-
/**
260-
* Holds if `${{ e }}` is a GitHub Actions expression evaluated within this `run` command.
261-
* See https://docs.github.com/en/free-pro-team@latest/actions/reference/context-and-expression-syntax-for-github-actions.
262-
* Only finds simple expressions like `${{ github.event.comment.body }}`, where the expression contains only alphanumeric characters, underscores, dots, or dashes.
263-
* Does not identify more complicated expressions like `${{ fromJSON(env.time) }}`, or ${{ format('{{Hello {0}!}}', github.event.head_commit.author.name) }}
264-
*/
265-
string getASimpleReferenceExpression() {
266-
// We use `regexpFind` to obtain *all* matches of `${{...}}`,
267-
// not just the last (greedy match) or first (reluctant match).
268-
result =
269-
this.getValue()
270-
.regexpFind("\\$\\{\\{\\s*[A-Za-z0-9_\\[\\]\\*\\(\\)\\.\\-]+\\s*\\}\\}", _, _)
271-
.regexpCapture("\\$\\{\\{\\s*([A-Za-z0-9_\\[\\]\\*\\((\\)\\.\\-]+)\\s*\\}\\}", 1)
272-
}
273292
}
274293
}

javascript/ql/src/Security/CWE-094/ExpressionInjection.ql

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -103,10 +103,24 @@ private predicate isExternalUserControlledWorkflowRun(string context) {
103103
)
104104
}
105105

106-
from Actions::Run run, string context, Actions::On on
106+
from YamlNode node, string context, Actions::On on
107107
where
108-
run.getASimpleReferenceExpression() = context and
109-
run.getStep().getJob().getWorkflow().getOn() = on and
108+
(
109+
exists(Actions::Run run |
110+
node = run and
111+
Actions::getASimpleReferenceExpression(run) = context and
112+
run.getStep().getJob().getWorkflow().getOn() = on
113+
)
114+
or
115+
exists(Actions::Script script, Actions::Step step, Actions::Uses uses |
116+
node = script and
117+
script.getWith().getStep() = step and
118+
uses.getStep() = step and
119+
uses.getGitHubRepository() = "actions/github-script" and
120+
Actions::getASimpleReferenceExpression(script) = context and
121+
script.getWith().getStep().getJob().getWorkflow().getOn() = on
122+
)
123+
) and
110124
(
111125
exists(on.getNode("issues")) and
112126
isExternalUserControlledIssue(context)
@@ -138,6 +152,6 @@ where
138152
exists(on.getNode("workflow_run")) and
139153
isExternalUserControlledWorkflowRun(context)
140154
)
141-
select run,
155+
select node,
142156
"Potential injection from the " + context +
143157
" context, which may be controlled by an external user."

javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/comment_issue.yml

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,17 @@ jobs:
1212
steps:
1313
- run: echo '${{ github.event.comment.body }}'
1414
- run: echo '${{ github.event.issue.body }}'
15-
- run: echo '${{ github.event.issue.title }}'
15+
- run: echo '${{ github.event.issue.title }}'
16+
17+
echo-chamber3:
18+
runs-on: ubuntu-latest
19+
steps:
20+
- uses: actions/github-script@v3
21+
with:
22+
script: console.log('${{ github.event.comment.body }}')
23+
- uses: actions/github-script@v3
24+
with:
25+
script: console.log('${{ github.event.issue.body }}')
26+
- uses: actions/github-script@v3
27+
with:
28+
script: console.log('${{ github.event.issue.title }}')

javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/ExpressionInjection.expected

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
| .github/workflows/comment_issue.yml:13:12:13:50 | echo '$ ... ody }}' | Potential injection from the github.event.comment.body context, which may be controlled by an external user. |
33
| .github/workflows/comment_issue.yml:14:12:14:48 | echo '$ ... ody }}' | Potential injection from the github.event.issue.body context, which may be controlled by an external user. |
44
| .github/workflows/comment_issue.yml:15:12:15:49 | echo '$ ... tle }}' | Potential injection from the github.event.issue.title context, which may be controlled by an external user. |
5+
| .github/workflows/comment_issue.yml:22:17:22:63 | console ... dy }}') | Potential injection from the github.event.comment.body context, which may be controlled by an external user. |
6+
| .github/workflows/comment_issue.yml:25:17:25:61 | console ... dy }}') | Potential injection from the github.event.issue.body context, which may be controlled by an external user. |
7+
| .github/workflows/comment_issue.yml:28:17:28:62 | console ... le }}') | Potential injection from the github.event.issue.title context, which may be controlled by an external user. |
58
| .github/workflows/comment_issue_newline.yml:9:14:10:50 | \| | Potential injection from the github.event.comment.body context, which may be controlled by an external user. |
69
| .github/workflows/discussion.yml:7:12:7:54 | echo '$ ... tle }}' | Potential injection from the github.event.discussion.title context, which may be controlled by an external user. |
710
| .github/workflows/discussion.yml:8:12:8:53 | echo '$ ... ody }}' | Potential injection from the github.event.discussion.body context, which may be controlled by an external user. |

0 commit comments

Comments
 (0)