Skip to content

Commit ac1c206

Browse files
committed
Encapsulate github-script
1 parent d80c541 commit ac1c206

File tree

2 files changed

+31
-17
lines changed

2 files changed

+31
-17
lines changed

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

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -368,13 +368,32 @@ module Actions {
368368
* script: console.log('${{ github.event.pull_request.head.sha }}')
369369
* ```
370370
*/
371-
class Script extends YamlNode, YamlString {
372-
With with;
371+
class GitHubScript extends YamlNode, YamlString {
372+
GitHubScriptWith with;
373373

374-
Script() { with.lookup("script") = this }
374+
GitHubScript() { with.lookup("script") = this }
375375

376376
/** Gets the `with` field this field belongs to. */
377-
With getWith() { result = with }
377+
GitHubScriptWith getWith() { result = with }
378+
}
379+
380+
/**
381+
* A step that uses `actions/github-script` action.
382+
*/
383+
class GitHubScriptStep extends Step {
384+
GitHubScriptStep() { this.getUses().getGitHubRepository() = "actions/github-script" }
385+
}
386+
387+
/**
388+
* A `with:` field sibling to `uses: actions/github-script`.
389+
*/
390+
class GitHubScriptWith extends YamlNode, YamlMapping {
391+
GitHubScriptStep step;
392+
393+
GitHubScriptWith() { step.lookup("with") = this }
394+
395+
/** Gets the step this field belongs to. */
396+
GitHubScriptStep getStep() { result = step }
378397
}
379398

380399
/**

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

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -133,17 +133,12 @@ predicate isRunInjectable(Actions::Run run, string injection, string context) {
133133
* Holds if the `actions/github-script` contains any expression interpolation `${{ e }}`.
134134
* Sets `context` to the initial untrusted value assignment in case of `${{ env... }}` interpolation
135135
*/
136-
predicate isScriptInjectable(Actions::Script script, string injection, string context) {
137-
exists(Actions::Step step, Actions::Uses uses |
138-
script.getWith().getStep() = step and
139-
uses.getStep() = step and
140-
uses.getGitHubRepository() = "actions/github-script" and
141-
Actions::getASimpleReferenceExpression(script) = injection and
142-
(
143-
injection = context
144-
or
145-
isEnvInterpolationTainted(injection, context)
146-
)
136+
predicate isScriptInjectable(Actions::GitHubScript script, string injection, string context) {
137+
Actions::getASimpleReferenceExpression(script) = injection and
138+
(
139+
injection = context
140+
or
141+
isEnvInterpolationTainted(injection, context)
147142
)
148143
}
149144

@@ -158,7 +153,7 @@ where
158153
run.getStep().getRuns() = runs
159154
)
160155
or
161-
exists(Actions::Script script |
156+
exists(Actions::GitHubScript script |
162157
node = script and
163158
script.getWith().getStep().getRuns() = runs and
164159
isScriptInjectable(script, injection, context)
@@ -184,7 +179,7 @@ where
184179
run.getStep().getJob().getWorkflow().getOn() = on
185180
)
186181
or
187-
exists(Actions::Script script |
182+
exists(Actions::GitHubScript script |
188183
node = script and
189184
script.getWith().getStep().getJob().getWorkflow().getOn() = on and
190185
isScriptInjectable(script, injection, context)

0 commit comments

Comments
 (0)