Skip to content

Commit df59e6f

Browse files
author
Alvaro Muñoz
committed
Consider a Reusable Workflow privileged if a caller is
1 parent 1dd7c3d commit df59e6f

File tree

8 files changed

+96
-1576
lines changed

8 files changed

+96
-1576
lines changed

‎ql/lib/codeql/actions/ast/internal/Ast.qll

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ private import codeql.actions.ast.internal.Yaml
22
private import codeql.Locations
33
private import codeql.actions.Helper
44
private import codeql.actions.config.Config
5+
private import codeql.actions.DataFlow
56

67
/**
78
* Gets the length of each line in the StringValue .
@@ -433,7 +434,10 @@ class ReusableWorkflowImpl extends AstNodeImpl, WorkflowImpl {
433434
}
434435

435436
ExternalJobImpl getACaller() {
436-
result.getCallee() = this.getLocation().getFile().getRelativePath()
437+
exists(DataFlow::CallNode call |
438+
call.getCalleeNode() = this and
439+
result = call.getCfgNode().getAstNode()
440+
)
437441
}
438442
}
439443

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ class CallNode extends ExprNode {
7272

7373
CallNode() { this.getCfgNode() instanceof DataFlowCall }
7474

75-
string getCallee() { result = this.getCfgNode().(DataFlowCall).getName() }
75+
DataFlowCallable getCalleeNode() { result = viableCallable(this.getCfgNode()) }
7676
}
7777

7878
/**

‎ql/test/library-tests/test.expected

Lines changed: 1 addition & 1574 deletions
Large diffs are not rendered by default.

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,10 @@ subpaths
414414
| .github/workflows/push.yml:14:19:14:64 | github.event.head_commit.committer.name | .github/workflows/push.yml:14:19:14:64 | github.event.head_commit.committer.name | .github/workflows/push.yml:14:19:14:64 | github.event.head_commit.committer.name | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/push.yml:14:19:14:64 | github.event.head_commit.committer.name | ${{ github.event.head_commit.committer.name }} |
415415
| .github/workflows/push.yml:15:19:15:65 | github.event.commits[11].committer.email | .github/workflows/push.yml:15:19:15:65 | github.event.commits[11].committer.email | .github/workflows/push.yml:15:19:15:65 | github.event.commits[11].committer.email | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/push.yml:15:19:15:65 | github.event.commits[11].committer.email | ${{ github.event.commits[11].committer.email }} |
416416
| .github/workflows/push.yml:16:19:16:64 | github.event.commits[11].committer.name | .github/workflows/push.yml:16:19:16:64 | github.event.commits[11].committer.name | .github/workflows/push.yml:16:19:16:64 | github.event.commits[11].committer.name | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/push.yml:16:19:16:64 | github.event.commits[11].committer.name | ${{ github.event.commits[11].committer.name }} |
417+
| .github/workflows/reusable-workflow-1.yml:36:21:36:39 | inputs.taint | .github/workflows/reusable-workflow-caller-1.yml:11:15:11:46 | github.event.comment.body | .github/workflows/reusable-workflow-1.yml:36:21:36:39 | inputs.taint | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/reusable-workflow-1.yml:36:21:36:39 | inputs.taint | ${{ inputs.taint }} |
418+
| .github/workflows/reusable-workflow-1.yml:53:26:53:39 | env.log | .github/workflows/reusable-workflow-1.yml:44:19:44:56 | github.event.pull_request.title | .github/workflows/reusable-workflow-1.yml:53:26:53:39 | env.log | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/reusable-workflow-1.yml:53:26:53:39 | env.log | ${{ env.log }} |
419+
| .github/workflows/reusable-workflow-2.yml:36:21:36:39 | inputs.taint | .github/workflows/reusable-workflow-caller-2.yml:10:15:10:46 | github.event.comment.body | .github/workflows/reusable-workflow-2.yml:36:21:36:39 | inputs.taint | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/reusable-workflow-2.yml:36:21:36:39 | inputs.taint | ${{ inputs.taint }} |
420+
| .github/workflows/reusable-workflow-2.yml:53:26:53:39 | env.log | .github/workflows/reusable-workflow-2.yml:44:19:44:56 | github.event.pull_request.title | .github/workflows/reusable-workflow-2.yml:53:26:53:39 | env.log | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/reusable-workflow-2.yml:53:26:53:39 | env.log | ${{ env.log }} |
417421
| .github/workflows/simple1.yml:16:18:16:49 | steps.summary.outputs.value | .github/workflows/simple1.yml:11:20:11:58 | github.event.head_commit.message | .github/workflows/simple1.yml:16:18:16:49 | steps.summary.outputs.value | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/simple1.yml:16:18:16:49 | steps.summary.outputs.value | ${{steps.summary.outputs.value}} |
418422
| .github/workflows/test10.yml:57:34:57:77 | github.event.workflow_run.head_branch | .github/workflows/test10.yml:57:34:57:77 | github.event.workflow_run.head_branch | .github/workflows/test10.yml:57:34:57:77 | github.event.workflow_run.head_branch | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/test10.yml:57:34:57:77 | github.event.workflow_run.head_branch | ${{ github.event.workflow_run.head_branch }} |
419423
| .github/workflows/test10.yml:147:34:147:77 | github.event.workflow_run.head_branch | .github/workflows/test10.yml:147:34:147:77 | github.event.workflow_run.head_branch | .github/workflows/test10.yml:147:34:147:77 | github.event.workflow_run.head_branch | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/test10.yml:147:34:147:77 | github.event.workflow_run.head_branch | ${{ github.event.workflow_run.head_branch }} |

‎ql/test/query-tests/Security/CWE-829/.github/reusable_workflows/TestOrg/TestRepo/.github/workflows/formal.yml

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
name: Test Formalities
2+
3+
on:
4+
workflow_call:
5+
6+
jobs:
7+
build:
8+
name: Test Formalities
9+
runs-on: ubuntu-latest
10+
strategy:
11+
fail-fast: false
12+
13+
steps:
14+
- uses: actions/checkout@v4
15+
with:
16+
ref: ${{ github.event.pull_request.head.sha }}
17+
fetch-depth: 0
18+
19+
- name: Determine branch name
20+
run: |
21+
BRANCH="${GITHUB_BASE_REF#refs/heads/}"
22+
echo "Building for $BRANCH"
23+
echo "BRANCH=$BRANCH" >> $GITHUB_ENV
24+
25+
- name: Test formalities
26+
run: |
27+
source .github/workflows/scripts/ci_helpers.sh
28+
29+
RET=0
30+
for commit in $(git rev-list HEAD ^origin/$BRANCH); do
31+
info "=== Checking commit '$commit'"
32+
if git show --format='%P' -s $commit | grep -qF ' '; then
33+
err "Pull request should not include merge commits"
34+
RET=1
35+
fi
36+
37+
author="$(git show -s --format=%aN $commit)"
38+
if echo $author | grep -q '\S\+\s\+\S\+'; then
39+
success "Author name ($author) seems ok"
40+
else
41+
err "Author name ($author) need to be your real name 'firstname lastname'"
42+
RET=1
43+
fi
44+
45+
subject="$(git show -s --format=%s $commit)"
46+
if echo "$subject" | grep -q -e '^[0-9A-Za-z,+/_\.-]\+: ' -e '^Revert '; then
47+
success "Commit subject line seems ok ($subject)"
48+
else
49+
err "Commit subject line MUST start with '<area>: ' ($subject)"
50+
RET=1
51+
fi
52+
53+
body="$(git show -s --format=%b $commit)"
54+
sob="$(git show -s --format='Signed-off-by: %aN <%aE>' $commit)"
55+
if echo "$body" | grep -qF "$sob"; then
56+
success "Signed-off-by match author"
57+
else
58+
err "Signed-off-by is missing or doesn't match author (should be '$sob')"
59+
RET=1
60+
fi
61+
62+
if echo "$body" | grep -v "Signed-off-by:"; then
63+
success "A commit message exists"
64+
else
65+
err "Missing commit message. Please describe your changes"
66+
RET=1
67+
fi
68+
done
69+
70+
exit $RET

‎ql/test/query-tests/Security/CWE-829/.github/workflows/formal.yml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
name: Test Formalities
2+
3+
on:
4+
pull_request:
5+
6+
permissions:
7+
contents: read
8+
9+
jobs:
10+
build:
11+
name: Test Formalities
12+
uses: TestOrg/TestRepo/.github/workflows/formal.yml@main

‎ql/test/query-tests/Security/CWE-829/UntrustedCheckoutCritical.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
edges
22
| .github/actions/dangerous-git-checkout/action.yml:6:7:11:4 | Uses Step | .github/actions/dangerous-git-checkout/action.yml:11:7:12:18 | Run Step |
33
| .github/actions/dangerous-git-checkout/action.yml:11:7:12:18 | Run Step | .github/workflows/untrusted_checkout3.yml:13:9:13:23 | Run Step |
4+
| .github/reusable_workflows/TestOrg/TestRepo/.github/workflows/formal.yml:14:9:19:6 | Uses Step | .github/reusable_workflows/TestOrg/TestRepo/.github/workflows/formal.yml:19:9:25:6 | Run Step |
5+
| .github/reusable_workflows/TestOrg/TestRepo/.github/workflows/formal.yml:19:9:25:6 | Run Step | .github/reusable_workflows/TestOrg/TestRepo/.github/workflows/formal.yml:25:9:70:20 | Run Step |
46
| .github/reusable_workflows/TestOrg/TestRepo/.github/workflows/reusable.yml:23:9:26:6 | Uses Step | .github/reusable_workflows/TestOrg/TestRepo/.github/workflows/reusable.yml:26:9:29:7 | Run Step |
57
| .github/workflows/actor_trusted_checkout.yml:9:7:14:4 | Uses Step | .github/workflows/actor_trusted_checkout.yml:14:7:15:4 | Uses Step |
68
| .github/workflows/actor_trusted_checkout.yml:14:7:15:4 | Uses Step | .github/workflows/actor_trusted_checkout.yml:15:7:19:4 | Run Step |

‎ql/test/query-tests/Security/CWE-829/UntrustedCheckoutMedium.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
| .github/reusable_workflows/TestOrg/TestRepo/.github/workflows/formal.yml:14:9:19:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
12
| .github/workflows/artifactpoisoning81.yml:11:9:14:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
23
| .github/workflows/artifactpoisoning82.yml:11:9:14:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
34
| .github/workflows/dependabot1.yml:15:9:19:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |

0 commit comments

Comments
 (0)