Skip to content

Commit 78b7399

Browse files
tspascoaltspascoal
committed
Actions: Add workflow_dispatch and workflow_call input sources for code injection
Model workflow_dispatch string inputs and workflow_call string inputs as remote flow sources. Add a new low-severity CodeInjection query for workflow_call inputs. The low severity CodeInjection query is explicitily added to the security-extended suite
1 parent fb8b569 commit 78b7399

File tree

13 files changed

+1068
-0
lines changed

13 files changed

+1068
-0
lines changed

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

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ private import codeql.actions.security.ArtifactPoisoningQuery
22
private import codeql.actions.security.UntrustedCheckoutQuery
33
private import codeql.actions.config.Config
44
private import codeql.actions.dataflow.ExternalFlow
5+
private import codeql.actions.ast.internal.Ast
6+
private import codeql.actions.ast.internal.Yaml
57

68
/**
79
* A data flow source.
@@ -363,3 +365,68 @@ class OctokitRequestActionSource extends RemoteFlowSource {
363365

364366
override string getEventName() { result = this.asExpr().getATriggerEvent().getName() }
365367
}
368+
369+
/**
370+
* A workflow_dispatch input of type string used in an expression.
371+
* Inputs with type string (or no explicit type, which defaults to string)
372+
* are considered untrusted since they can be controlled by any user
373+
* who can trigger the workflow (write access to the repository)
374+
*/
375+
class WorkflowDispatchInputSource extends RemoteFlowSource {
376+
WorkflowDispatchInputSource() {
377+
exists(InputsExpression e, Event event, string inputName |
378+
this.asExpr() = e and
379+
inputName = e.getFieldName() and
380+
event = e.getATriggerEvent() and
381+
event.getName() = "workflow_dispatch" and
382+
exists(YamlMapping eventMapping, YamlMapping inputsMapping |
383+
eventMapping = event.(EventImpl).getValueNode() and
384+
inputsMapping = eventMapping.lookup("inputs") and
385+
exists(YamlMapping inputMapping |
386+
inputMapping = inputsMapping.lookup(inputName) and
387+
(
388+
// explicit type: string
389+
inputMapping.lookup("type").(YamlScalar).getValue().toLowerCase() = "string"
390+
or
391+
// no explicit type (defaults to string in GitHub Actions)
392+
not exists(inputMapping.lookup("type"))
393+
)
394+
)
395+
)
396+
)
397+
}
398+
399+
override string getSourceType() { result = "text" }
400+
401+
override string getEventName() { result = "workflow_dispatch" }
402+
}
403+
404+
/**
405+
* A workflow_call input of type string used in an expression.
406+
* Only string-typed inputs are considered vulnerable to code injection.
407+
* This is lower risk since only workflow authors control the callers,
408+
* but the inputs may still originate from untrusted user input in the
409+
* calling workflow.
410+
*/
411+
class WorkflowCallInputSource extends RemoteFlowSource {
412+
WorkflowCallInputSource() {
413+
exists(InputsExpression e, ReusableWorkflowImpl w, string inputName |
414+
this.asExpr() = e and
415+
inputName = e.getFieldName() and
416+
w = e.getEnclosingWorkflow() and
417+
exists(YamlMapping inputsMapping, YamlMapping inputMapping |
418+
inputsMapping = w.getInputs().getNode() and
419+
inputMapping = inputsMapping.lookup(inputName) and
420+
inputMapping.lookup("type").(YamlScalar).getValue().toLowerCase() = "string"
421+
)
422+
)
423+
}
424+
425+
override string getSourceType() { result = "text" }
426+
427+
override string getEventName() {
428+
result = this.asExpr().getATriggerEvent().getName()
429+
or
430+
not exists(this.asExpr().getATriggerEvent()) and result = "workflow_call"
431+
}
432+
}

‎actions/ql/lib/codeql/actions/security/CodeInjectionQuery.qll‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,16 @@ predicate criticalSeverityCodeInjection(
9999
source.getNode().(RemoteFlowSource).getEventName() = event.getName()
100100
}
101101

102+
/**
103+
* Holds if the source is an input expression inside a reusable workflow (workflow_call).
104+
*/
105+
private predicate isWorkflowCallInput(DataFlow::Node source) {
106+
exists(InputsExpression e |
107+
source.asExpr() = e and
108+
e.getEnclosingWorkflow() instanceof ReusableWorkflow
109+
)
110+
}
111+
102112
/**
103113
* Holds if there is a code injection flow from `source` to `sink` with medium severity.
104114
*/
@@ -107,6 +117,22 @@ predicate mediumSeverityCodeInjection(
107117
) {
108118
CodeInjectionFlow::flowPath(source, sink) and
109119
not criticalSeverityCodeInjection(source, sink, _) and
120+
not isWorkflowCallInput(source.getNode()) and
121+
not isGithubScriptUsingToJson(sink.getNode().asExpr())
122+
}
123+
124+
/**
125+
* Holds if there is a code injection flow from `source` to `sink` with low severity.
126+
* This covers workflow_call inputs, which are lower risk since only users with
127+
* write access control the callers, but the inputs may still originate from
128+
* untrusted user input in the calling workflow.
129+
*/
130+
predicate lowSeverityCodeInjection(
131+
CodeInjectionFlow::PathNode source, CodeInjectionFlow::PathNode sink
132+
) {
133+
CodeInjectionFlow::flowPath(source, sink) and
134+
source.getNode() instanceof WorkflowCallInputSource and
135+
not criticalSeverityCodeInjection(source, sink, _) and
110136
not isGithubScriptUsingToJson(sink.getNode().asExpr())
111137
}
112138

‎actions/ql/src/Security/CWE-094/CodeInjectionLow.md‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
## Overview
2+
3+
Using string-typed `workflow_call` inputs in GitHub Actions may lead to code injection in contexts like _run:_ or _script:_.
4+
5+
Inputs declared as `string` should be treated with caution. Although `workflow_call` can only be triggered by other workflows (not directly by external users), the calling workflow may pass untrusted user input as arguments. Since the reusable workflow author has no control over the callers, these inputs may still originate from untrusted data.
6+
7+
Code injection in GitHub Actions may allow an attacker to exfiltrate any secrets used in the workflow and the temporary GitHub repository authorization token.
8+
9+
## Recommendation
10+
11+
The best practice to avoid code injection vulnerabilities in GitHub workflows is to set the untrusted input value of the expression to an intermediate environment variable and then use the environment variable using the native syntax of the shell/script interpreter (that is, not _${{ env.VAR }}_).
12+
13+
It is also recommended to limit the permissions of any tokens used by a workflow such as the GITHUB_TOKEN.
14+
15+
## Example
16+
17+
### Incorrect Usage
18+
19+
The following example uses a `workflow_call` input directly in a _run:_ step, which allows code injection if the caller passes untrusted data:
20+
21+
```yaml
22+
on:
23+
workflow_call:
24+
inputs:
25+
title:
26+
description: 'Title'
27+
type: string
28+
29+
jobs:
30+
greet:
31+
runs-on: ubuntu-latest
32+
steps:
33+
- run: |
34+
echo '${{ inputs.title }}'
35+
```
36+
37+
### Correct Usage
38+
39+
The following example safely uses a `workflow_call` input by passing it through an environment variable:
40+
41+
```yaml
42+
on:
43+
workflow_call:
44+
inputs:
45+
title:
46+
description: 'Title'
47+
type: string
48+
49+
jobs:
50+
greet:
51+
runs-on: ubuntu-latest
52+
steps:
53+
- env:
54+
TITLE: ${{ inputs.title }}
55+
run: |
56+
echo "$TITLE"
57+
```
58+
59+
## References
60+
61+
- GitHub Security Lab Research: [Keeping your GitHub Actions and workflows secure: Untrusted input](https://securitylab.github.com/research/github-actions-untrusted-input).
62+
- GitHub Docs: [Security hardening for GitHub Actions](https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions).
63+
- GitHub Docs: [Reusing workflows](https://docs.github.com/en/actions/using-workflows/reusing-workflows).
64+
- Common Weakness Enumeration: [CWE-94: Improper Control of Generation of Code ('Code Injection')](https://cwe.mitre.org/data/definitions/94.html).
65+
- Common Weakness Enumeration: [CWE-95: Improper Neutralization of Directives in Dynamically Evaluated Code ('Eval Injection')](https://cwe.mitre.org/data/definitions/95.html).
66+
- Common Weakness Enumeration: [CWE-116: Improper Encoding or Escaping of Output](https://cwe.mitre.org/data/definitions/116.html).

‎actions/ql/src/Security/CWE-094/CodeInjectionLow.ql‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/**
2+
* @name Code injection
3+
* @description Using unsanitized workflow_call inputs as code allows a calling workflow to
4+
* pass untrusted user input, leading to code execution.
5+
* @kind path-problem
6+
* @problem.severity warning
7+
* @security-severity 3.0
8+
* @precision low
9+
* @id actions/code-injection/low
10+
* @tags actions
11+
* security
12+
* external/cwe/cwe-094
13+
* external/cwe/cwe-095
14+
* external/cwe/cwe-116
15+
*/
16+
17+
import actions
18+
import codeql.actions.security.CodeInjectionQuery
19+
import CodeInjectionFlow::PathGraph
20+
21+
from CodeInjectionFlow::PathNode source, CodeInjectionFlow::PathNode sink
22+
where lowSeverityCodeInjection(source, sink)
23+
select sink.getNode(), source, sink,
24+
"Potential code injection in $@, which may be controlled by a calling workflow.", sink,
25+
sink.getNode().asExpr().(Expression).getRawExpression()

‎actions/ql/src/Security/CWE-094/CodeInjectionMedium.md‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
Using user-controlled input in GitHub Actions may lead to code injection in contexts like _run:_ or _script:_.
44

5+
This includes inputs from events such as `issue_comment`, `pull_request`, and `workflow_dispatch`. In particular, `workflow_dispatch` inputs with type `string` (or no type, which defaults to `string`) are user-controlled and should be treated as untrusted. Note that `workflow_dispatch` can only be triggered by users with write permissions to the repository, so the risk is lower than for events like `issue_comment` which can be triggered by any user. Nevertheless, this is still a code injection vector and should be handled safely.
6+
57
Code injection in GitHub Actions may allow an attacker to exfiltrate any secrets used in the workflow and the temporary GitHub repository authorization token. The token may have write access to the repository, allowing an attacker to make changes to the repository.
68

79
## Recommendation
@@ -27,6 +29,24 @@ jobs:
2729
echo '${{ github.event.comment.body }}'
2830
```
2931
32+
The following example uses a `workflow_dispatch` string input directly in a _run:_ step, which allows code injection:
33+
34+
```yaml
35+
on:
36+
workflow_dispatch:
37+
inputs:
38+
title:
39+
description: 'Title'
40+
type: string
41+
42+
jobs:
43+
greet:
44+
runs-on: ubuntu-latest
45+
steps:
46+
- run: |
47+
echo '${{ inputs.title }}'
48+
```
49+
3050
The following example uses an environment variable, but **still allows the injection** because of the use of expression syntax:
3151

3252
```yaml
@@ -57,6 +77,26 @@ jobs:
5777
echo "$BODY"
5878
```
5979

80+
The following example safely uses a `workflow_dispatch` input by passing it through an environment variable:
81+
82+
```yaml
83+
on:
84+
workflow_dispatch:
85+
inputs:
86+
title:
87+
description: 'Title'
88+
type: string
89+
90+
jobs:
91+
greet:
92+
runs-on: ubuntu-latest
93+
steps:
94+
- env:
95+
TITLE: ${{ inputs.title }}
96+
run: |
97+
echo "$TITLE"
98+
```
99+
60100
The following example uses `process.env` to read environment variables within JavaScript code.
61101

62102
```yaml

‎actions/ql/src/codeql-suites/actions-security-extended.qls‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,6 @@
22
- queries: .
33
- apply: security-extended-selectors.yml
44
from: codeql/suite-helpers
5+
# Explicitly include low-precision queries that are excluded by security-extended-selectors.yml.
6+
- include:
7+
query path: Security/CWE-094/CodeInjectionLow.ql

‎actions/ql/test/query-tests/Models/ReusableWorkflowsSources.expected‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,22 @@
11
edges
2+
| .github/workflows/reusable_workflow.yml:22:7:24:4 | Job outputs node [job-output1] | .github/workflows/reusable_workflow.yml:11:17:11:52 | jobs.job1.outputs.job-output1 | provenance | |
23
| .github/workflows/reusable_workflow.yml:22:7:24:4 | Job outputs node [job-output2] | .github/workflows/reusable_workflow.yml:13:17:13:52 | jobs.job1.outputs.job-output2 | provenance | |
4+
| .github/workflows/reusable_workflow.yml:22:21:22:57 | steps.step1.outputs.step-output | .github/workflows/reusable_workflow.yml:22:7:24:4 | Job outputs node [job-output1] | provenance | |
35
| .github/workflows/reusable_workflow.yml:23:21:23:63 | steps.step2.outputs.all_changed_files | .github/workflows/reusable_workflow.yml:22:7:24:4 | Job outputs node [job-output2] | provenance | |
6+
| .github/workflows/reusable_workflow.yml:25:9:31:6 | Run Step: step1 [step-output] | .github/workflows/reusable_workflow.yml:22:21:22:57 | steps.step1.outputs.step-output | provenance | |
7+
| .github/workflows/reusable_workflow.yml:27:25:27:49 | inputs.config-path | .github/workflows/reusable_workflow.yml:25:9:31:6 | Run Step: step1 [step-output] | provenance | |
48
| .github/workflows/reusable_workflow.yml:31:9:33:43 | Uses Step: step2 | .github/workflows/reusable_workflow.yml:23:21:23:63 | steps.step2.outputs.all_changed_files | provenance | |
59
nodes
10+
| .github/workflows/reusable_workflow.yml:11:17:11:52 | jobs.job1.outputs.job-output1 | semmle.label | jobs.job1.outputs.job-output1 |
611
| .github/workflows/reusable_workflow.yml:13:17:13:52 | jobs.job1.outputs.job-output2 | semmle.label | jobs.job1.outputs.job-output2 |
12+
| .github/workflows/reusable_workflow.yml:22:7:24:4 | Job outputs node [job-output1] | semmle.label | Job outputs node [job-output1] |
713
| .github/workflows/reusable_workflow.yml:22:7:24:4 | Job outputs node [job-output2] | semmle.label | Job outputs node [job-output2] |
14+
| .github/workflows/reusable_workflow.yml:22:21:22:57 | steps.step1.outputs.step-output | semmle.label | steps.step1.outputs.step-output |
815
| .github/workflows/reusable_workflow.yml:23:21:23:63 | steps.step2.outputs.all_changed_files | semmle.label | steps.step2.outputs.all_changed_files |
16+
| .github/workflows/reusable_workflow.yml:25:9:31:6 | Run Step: step1 [step-output] | semmle.label | Run Step: step1 [step-output] |
17+
| .github/workflows/reusable_workflow.yml:27:25:27:49 | inputs.config-path | semmle.label | inputs.config-path |
918
| .github/workflows/reusable_workflow.yml:31:9:33:43 | Uses Step: step2 | semmle.label | Uses Step: step2 |
1019
subpaths
1120
#select
21+
| .github/workflows/reusable_workflow.yml:11:17:11:52 | jobs.job1.outputs.job-output1 | .github/workflows/reusable_workflow.yml:27:25:27:49 | inputs.config-path | .github/workflows/reusable_workflow.yml:11:17:11:52 | jobs.job1.outputs.job-output1 | Source |
1222
| .github/workflows/reusable_workflow.yml:13:17:13:52 | jobs.job1.outputs.job-output2 | .github/workflows/reusable_workflow.yml:31:9:33:43 | Uses Step: step2 | .github/workflows/reusable_workflow.yml:13:17:13:52 | jobs.job1.outputs.job-output2 | Source |

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

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
name: Code Injection via workflow_call
2+
3+
on:
4+
workflow_call:
5+
inputs:
6+
title:
7+
description: "Title string input"
8+
required: true
9+
type: string
10+
count:
11+
description: "A number input"
12+
required: false
13+
type: number
14+
flag:
15+
description: "A boolean input"
16+
required: false
17+
type: boolean
18+
19+
permissions:
20+
contents: read
21+
22+
jobs:
23+
build:
24+
runs-on: ubuntu-latest
25+
26+
steps:
27+
# Vulnerable: string input used directly in run script
28+
- name: vulnerable string input
29+
run: |
30+
echo "${{ inputs.title }}"
31+
32+
# Not vulnerable: number input (not a string type)
33+
- name: safe number input
34+
run: |
35+
echo "${{ inputs.count }}"
36+
37+
# Not vulnerable: boolean input (not a string type)
38+
- name: safe boolean input
39+
run: |
40+
echo "${{ inputs.flag }}"
41+
42+
# Not vulnerable: input passed safely through env var
43+
- name: safe string input via env
44+
run: |
45+
echo "$title"
46+
env:
47+
title: ${{ inputs.title }}

0 commit comments

Comments
 (0)