Skip to content

Commit 822e9bc

Browse files
author
Alvaro Muñoz
committed
env var injection query
1 parent ff3759e commit 822e9bc

File tree

9 files changed

+251
-2
lines changed

9 files changed

+251
-2
lines changed

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
private import actions
2+
private import codeql.actions.TaintTracking
3+
private import codeql.actions.dataflow.ExternalFlow
4+
import codeql.actions.dataflow.FlowSources
5+
import codeql.actions.DataFlow
6+
7+
predicate writeToGithubEnvSink(DataFlow::Node sink) {
8+
exists(Expression expr, Run run, string script, string line, string value |
9+
script = run.getScript() and
10+
line = script.splitAt("\n") and
11+
value = line.regexpCapture("echo\\s+.*\\s*=(.*)>>\\s*\\$GITHUB_ENV", 1) and
12+
expr = sink.asExpr() and
13+
run.getAnScriptExpr() = expr and
14+
value.indexOf(expr.getRawExpression()) > 0
15+
)
16+
}
17+
18+
private class EnvVarInjectionSink extends DataFlow::Node {
19+
EnvVarInjectionSink() {
20+
writeToGithubEnvSink(this) or
21+
externallyDefinedSink(this, "envvar-injection")
22+
}
23+
}
24+
25+
/**
26+
* A taint-tracking configuration for unsafe user input
27+
* that is used to construct and evaluate an environment variable.
28+
*/
29+
private module EnvVarInjectionConfig implements DataFlow::ConfigSig {
30+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
31+
32+
predicate isSink(DataFlow::Node sink) { sink instanceof EnvVarInjectionSink }
33+
}
34+
35+
/** Tracks flow of unsafe user input that is used to construct and evaluate an environment variable. */
36+
module EnvVarInjectionFlow = TaintTracking::Global<EnvVarInjectionConfig>;

‎ql/src/Security/CWE-077/EnvVarInjection.ql

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/**
2+
* @name Enviroment Variable built from user-controlled sources
3+
* @description Building an environment variable from user-controlled sources may alter the execution of following system commands
4+
* @kind path-problem
5+
* @problem.severity warning
6+
* @security-severity 5.0
7+
* @precision high
8+
* @id actions/envvar-injection
9+
* @tags actions
10+
* security
11+
* external/cwe/cwe-077
12+
* external/cwe/cwe-020
13+
*/
14+
15+
import actions
16+
import codeql.actions.security.EnvVarInjectionQuery
17+
import EnvVarInjectionFlow::PathGraph
18+
19+
from EnvVarInjectionFlow::PathNode source, EnvVarInjectionFlow::PathNode sink
20+
where EnvVarInjectionFlow::flowPath(source, sink)
21+
select sink.getNode(), source, sink,
22+
"Potential environment variable injection in $@, which may be controlled by an external user.",
23+
sink, sink.getNode().asExpr().(Expression).getRawExpression()

‎ql/src/Security/CWE-077/PrivilegedEnvVarInjection.ql

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/**
2+
* @name Enviroment Variable built from user-controlled sources
3+
* @description Building an environment variable from user-controlled sources may alter the execution of following system commands
4+
* @kind path-problem
5+
* @problem.severity error
6+
* @security-severity 9
7+
* @precision high
8+
* @id actions/privileged-envvar-injection
9+
* @tags actions
10+
* security
11+
* external/cwe/cwe-077
12+
* external/cwe/cwe-020
13+
*/
14+
15+
import actions
16+
import codeql.actions.security.EnvVarInjectionQuery
17+
import EnvVarInjectionFlow::PathGraph
18+
19+
from EnvVarInjectionFlow::PathNode source, EnvVarInjectionFlow::PathNode sink, Workflow w
20+
where
21+
EnvVarInjectionFlow::flowPath(source, sink) and
22+
w = source.getNode().asExpr().getEnclosingWorkflow() and
23+
w.hasTriggerEvent(source.getNode().(RemoteFlowSource).getATriggerEvent())
24+
select sink.getNode(), source, sink,
25+
"Potential privileged environment variable injection in $@, which may be controlled by an external user.",
26+
sink, sink.getNode().asExpr().(Expression).getRawExpression()

‎ql/test/query-tests/Security/CWE-077/.github/workflows/test1.yml

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
name: Pull Request Open
2+
3+
on:
4+
pull_request_target:
5+
branches:
6+
- main
7+
- 14.0.x
8+
9+
types:
10+
- opened
11+
- reopened
12+
13+
jobs:
14+
updateJira:
15+
if: github.actor != 'dependabot[bot]'
16+
runs-on: ubuntu-latest
17+
steps:
18+
- name: Checkout repository
19+
uses: actions/checkout@v4
20+
21+
- name: Extract Jira Key
22+
run: echo ISSUE_KEY=$(echo "${{ github.event.pull_request.title }}") >> $GITHUB_ENV
23+
24+
25+

‎ql/test/query-tests/Security/CWE-077/.github/workflows/test2.yml

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
# https://www.legitsecurity.com/blog/github-privilege-escalation-vulnerability-0
2+
# https://github.com/firebase/friendlyeats-web/commit/df65aefd24cf6f092a27a5576067ff9f29aa2ef1
3+
name: Deploy Preview
4+
on:
5+
workflow_run:
6+
workflows: ["Generate Preview"]
7+
types:
8+
- completed
9+
10+
jobs:
11+
deploy:
12+
runs-on: ubuntu-latest
13+
if: >
14+
${{ github.event.workflow_run.event == 'pull_request' &&
15+
github.event.workflow_run.conclusion == 'success' }}
16+
steps:
17+
- name: 'Download artifact'
18+
uses: actions/[email protected]
19+
with:
20+
script: |
21+
var artifacts = await github.actions.listWorkflowRunArtifacts({
22+
owner: context.repo.owner,
23+
repo: context.repo.repo,
24+
run_id: ${{ github.event.workflow_run.id }},
25+
});
26+
var matchPrArtifact = artifacts.data.artifacts.filter((artifact) => {
27+
return artifact.name == "pr"
28+
})[0];
29+
var matchPreviewArtifact = artifacts.data.artifacts.filter((artifact) => {
30+
return artifact.name == "preview"
31+
})[0];
32+
var downloadPr = await github.actions.downloadArtifact({
33+
owner: context.repo.owner,
34+
repo: context.repo.repo,
35+
artifact_id: matchPrArtifact.id,
36+
archive_format: 'zip',
37+
});
38+
var downloadPreview = await github.actions.downloadArtifact({
39+
owner: context.repo.owner,
40+
repo: context.repo.repo,
41+
artifact_id: matchPreviewArtifact.id,
42+
archive_format: 'zip',
43+
});
44+
var fs = require('fs');
45+
fs.writeFileSync('${{github.workspace}}/pr.zip', Buffer.from(downloadPr.data));
46+
fs.writeFileSync('${{github.workspace}}/firestore-web.zip', Buffer.from(downloadPreview.data));
47+
- run: |
48+
unzip pr.zip
49+
echo "pr_number=$(cat NR)" >> $GITHUB_ENV
50+
mkdir firestore-web
51+
unzip firestore-web.zip -d firestore-web
52+
- name: Deploy preview
53+
id: deploy_preview
54+
uses: FirebaseExtended/action-hosting-deploy@v0
55+
with:
56+
repoToken: '${{ secrets.GITHUB_TOKEN }}'
57+
firebaseServiceAccount: '${{ secrets.FIREBASE_SERVICE_ACCOUNT_FIR_CODELABS_89252 }}'
58+
projectId: fir-codelabs-89252
59+
entryPoint: firestore-web
60+
channelId: firestore-web-${{ env.pr_number }}
61+
env:
62+
FIREBASE_CLI_PREVIEWS: hostingchannels
63+
- name: Write Comment
64+
uses: actions/github-script@v3
65+
with:
66+
github-token: ${{ secrets.GITHUB_TOKEN }}
67+
script: |
68+
await github.issues.createComment({
69+
owner: context.repo.owner,
70+
repo: context.repo.repo,
71+
issue_number: ${{ env.pr_number }},
72+
body: 'View preview ${{ steps.deploy_preview.outputs.details_url }}'
73+
});

‎ql/test/query-tests/Security/CWE-077/.github/workflows/test3.yml

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
# https://www.legitsecurity.com/blog/-how-we-found-another-github-action-environment-injection-vulnerability-in-a-google-project
2+
# https://github.com/google/orbit/commit/6cd71a3f1eec098d0de61bf9bb742737cb3aa5fa
3+
name: report-checks
4+
on:
5+
workflow_run:
6+
workflows: ['checks']
7+
types:
8+
- completed
9+
permissions: read-all
10+
jobs:
11+
12+
report-clang-tidy-diff:
13+
permissions:
14+
pull-requests: write
15+
runs-on: ubuntu-latest
16+
steps:
17+
- name: Download PR metadata
18+
uses: dawidd6/action-download-artifact@e6e25ac3a2b93187502a8be1ef9e9603afc34925 # v2.24.2
19+
with:
20+
workflow: ${{ github.event.workflow_run.workflow_id }}
21+
workflow_conclusion: ''
22+
name: pr_metadata
23+
if_no_artifact_found: 'ignore'
24+
- name: Download clang_tidy_fixes
25+
uses: dawidd6/action-download-artifact@e6e25ac3a2b93187502a8be1ef9e9603afc34925 # v2.24.2
26+
with:
27+
workflow: ${{ github.event.workflow_run.workflow_id }}
28+
workflow_conclusion: ''
29+
name: clang_tidy_fixes
30+
if_no_artifact_found: 'ignore'
31+
- name: Set found_files
32+
id: set_found_files
33+
run: |
34+
if [ -f clang-tidy-fixes.yml ] && [ -f pr_number.txt ] && [ -f pr_head_repo.txt ] && [ -f pr_head_ref.txt ]; then
35+
echo "found_files=true" >> $GITHUB_OUTPUT
36+
else
37+
echo "found_files=false" >> $GITHUB_OUTPUT
38+
fi
39+
- run: |
40+
echo "PR_NUMBER=$(cat pr_number.txt | jq -r .)" >> $GITHUB_ENV
41+
echo "PR_HEAD_REPO=$(cat pr_head_repo.txt | jq -Rr .)" >> $GITHUB_ENV
42+
echo "PR_HEAD_REF=$(cat pr_head_ref.txt | jq -Rr .)" >> $GITHUB_ENV
43+
if: steps.set_found_files.outputs.found_files == 'true'
44+
- uses: actions/checkout@v3
45+
if: steps.set_found_files.outputs.found_files == 'true'
46+
with:
47+
repository: ${{ env.PR_HEAD_REPO }}
48+
ref: ${{ env.PR_HEAD_REF }}
49+
persist-credentials: false
50+
- name: Redownload clang_tidy_fixes
51+
if: steps.set_found_files.outputs.found_files == 'true'
52+
uses: dawidd6/action-download-artifact@e6e25ac3a2b93187502a8be1ef9e9603afc34925 # v2.24.2
53+
with:
54+
workflow: ${{ github.event.workflow_run.workflow_id }}
55+
workflow_conclusion: ''
56+
name: clang_tidy_fixes
57+
if_no_artifact_found: 'ignore'
58+
- uses: platisd/clang-tidy-pr-comments@89ea1b828cdac1a6ec993d225972adea3b8841b6
59+
if: steps.set_found_files.outputs.found_files == 'true'
60+
with:
61+
github_token: ${{ secrets.ORBITPROFILER_BOT_PAT }}
62+
clang_tidy_fixes: clang-tidy-fixes.yml
63+
pull_request_id: ${{ env.PR_NUMBER }}
64+

‎ql/test/query-tests/Security/CWE-077/EnvVarInjection.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Security/CWE-077/EnvVarInjection.ql

‎ql/test/query-tests/Security/CWE-077/PrivilegedEnvVarInjection.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Security/CWE-077/PrivilegedEnvVarInjection.ql

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
jn: push
1+
on: push
22

33
jobs:
44
job0:
@@ -36,7 +36,7 @@ jobs:
3636

3737
if: ${{ always() }}
3838

39-
needs: job1
39+
needs: job
4040

4141
steps:
4242
- id: sink

0 commit comments

Comments
 (0)