Skip to content

Commit

Permalink
Merge pull request #38 from GitHubSecurityLab/improve_untrusted_co
Browse files Browse the repository at this point in the history
  • Loading branch information
Alvaro Muñoz authored Mar 18, 2024
2 parents 0a2be55 + 9683ae3 commit b6a097c
Show file tree
Hide file tree
Showing 5 changed files with 136 additions and 5 deletions.
24 changes: 19 additions & 5 deletions ql/src/Security/CWE-829/UntrustedCheckout.ql
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
* @name Checkout of untrusted code in trusted context
* @description Workflows triggered on `pull_request_target` have read/write access to the base repository and access to secrets.
* @description Priveleged workflows have read/write access to the base repository and access to secrets.
* By explicitly checking out and running the build script from a fork the untrusted code is running in an environment
* that is able to push to the base repository and to access secrets.
* @kind problem
Expand Down Expand Up @@ -121,12 +121,26 @@ class GitCheckout extends PRHeadCheckoutStep instanceof Run {
}
}

predicate isSingleTriggerWorkflow(Workflow w, string trigger) {
w.getATriggerEvent() = trigger and
count(string t | w.getATriggerEvent() = t | t) = 1
}

from Workflow w, PRHeadCheckoutStep checkout
where
w.hasTriggerEvent([
"pull_request_target", "issue_comment", "pull_request_review_comment", "pull_request_review",
"workflow_run", "check_run", "check_suite", "workflow_call"
]) and
(
// The Workflow is triggered by an event other than `pull_request`
not isSingleTriggerWorkflow(w, "pull_request")
or
// The Workflow is only triggered by `workflow_call` and there is
// a caller workflow triggered by an event other than `pull_request`
isSingleTriggerWorkflow(w, "workflow_call") and
exists(ExternalJob call, Workflow caller |
call.getCallee() = w.getLocation().getFile().getRelativePath() and
caller = call.getWorkflow() and
not isSingleTriggerWorkflow(caller, "pull_request")
)
) and
w.getAJob().(LocalJob).getAStep() = checkout and
not exists(ControlCheck check |
checkout.getIf() = check or checkout.getEnclosingJob().getIf() = check
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
name: changelog

on:
workflow_call:
inputs:
create:
description: Add a log to the changelog
type: boolean
required: false
default: false
update:
description: Update the existing changelog
type: boolean
required: false
default: false

jobs:
changelog:
runs-on: ubuntu-latest
env:
file: CHANGELOG.md
steps:
- uses: actions/checkout@v3
with:
fetch-depth: 0
- name: Check ${{ env.file }}
run: |
if [[ $(git diff --name-only origin/master HEAD -- ${{ env.file }} | grep '^${{ env.file }}$' -c) -eq 0 ]]; then
echo "Expected '${{ env.file }}' to be modified"
exit 1
fi
update:
runs-on: ubuntu-latest
needs: changelog
if: (inputs.create && failure()) || (inputs.update && success())
continue-on-error: true
env:
file: CHANGELOG.md
next_version: next
link: '[#${{ github.event.number }}](https://github.com/fabricjs/fabric.js/pull/${{ github.event.number }})'
steps:
- uses: actions/checkout@v3
with:
ref: ${{ github.event.pull_request.head.ref }}
- name: Update ${{ env.file }} from PR title
id: update
uses: actions/github-script@v6
env:
log: '- ${{ github.event.pull_request.title }} ${{ env.link }}\n'
prev_log: '- ${{ github.event.changes.title.from }} ${{ env.link }}\n'
with:
result-encoding: string
script: |
const fs = require('fs');
const file = './${{ env.file }}';
let content = fs.readFileSync(file).toString();
const title = '[${{ env.next_version }}]';
const log = '${{ env.log }}';
let exists = ${{ needs.changelog.result == 'success' }};
if (!content.includes(title)) {
const insertAt = content.indexOf('\n') + 1;
content =
content.slice(0, insertAt) +
`\n## ${title}\n\n\n` +
content.slice(insertAt);
}
const insertAt = content.indexOf('\n', content.indexOf(title) + title.length + 1) + 1;
if (exists && ${{ github.event.action == 'edited' }}) {
const prevLog = '${{ env.prev_log }}';
const index = content.indexOf(prevLog, insertAt);
if (index > -1) {
content = content.slice(0, index) + content.slice(index + prevLog.length);
exists = false;
}
}
if (!exists) {
content = content.slice(0, insertAt) + log + content.slice(insertAt);
fs.writeFileSync(file, content);
return true;
}
return false;
- name: Setup node
if: fromJson(steps.update.outputs.result)
uses: actions/setup-node@v3
with:
node-version: 18.x
- name: Commit & Push
if: fromJson(steps.update.outputs.result)
run: |
npm ci
npx prettier --write ${{ env.file }}
git config user.name github-actions[bot]
git config user.email github-actions[bot]@users.noreply.github.com
git add ${{ env.file }}
git commit -m "update ${{ env.file }}"
git push
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
name: '📋'

on:
pull_request_target:
branches: [master]

jobs:
changelog:
uses: ./.github/workflows/changelog_from_prt.yml
4 changes: 4 additions & 0 deletions ql/test/query-tests/Security/CWE-094/CodeInjection.expected
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ edges
| .github/workflows/argus_case_study.yml:22:20:22:39 | env.ISSUE_TITLE | .github/workflows/argus_case_study.yml:15:9:24:6 | Uses Step: remove_quotations [replaced] |
| .github/workflows/changed-files.yml:16:9:20:6 | Uses Step: changed-files | .github/workflows/changed-files.yml:22:24:22:75 | steps.changed-files.outputs.all_changed_files |
| .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog.yml:58:26:58:39 | env.log |
| .github/workflows/changelog_from_prt.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log |
| .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] | .github/workflows/cross3.yml:39:31:39:75 | steps.remove_quotations.outputs.replaced |
| .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] | .github/workflows/cross3.yml:57:29:57:73 | steps.remove_quotations.outputs.replaced |
| .github/workflows/cross3.yml:32:18:32:53 | github.event.commits[0].message | .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] |
Expand Down Expand Up @@ -66,6 +67,8 @@ nodes
| .github/workflows/changed-files.yml:22:24:22:75 | steps.changed-files.outputs.all_changed_files | semmle.label | steps.changed-files.outputs.all_changed_files |
| .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | semmle.label | github.event.pull_request.title |
| .github/workflows/changelog.yml:58:26:58:39 | env.log | semmle.label | env.log |
| .github/workflows/changelog_from_prt.yml:49:19:49:56 | github.event.pull_request.title | semmle.label | github.event.pull_request.title |
| .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | semmle.label | env.log |
| .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | semmle.label | github.event.comment.body |
| .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | semmle.label | github.event.comment.body |
| .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | semmle.label | github.event.issue.body |
Expand Down Expand Up @@ -205,6 +208,7 @@ subpaths
| .github/workflows/argus_case_study.yml:27:33:27:77 | steps.remove_quotations.outputs.replaced | .github/workflows/argus_case_study.yml:17:25:17:53 | github.event.issue.title | .github/workflows/argus_case_study.yml:27:33:27:77 | steps.remove_quotations.outputs.replaced | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/argus_case_study.yml:27:33:27:77 | steps.remove_quotations.outputs.replaced | ${{steps.remove_quotations.outputs.replaced}} |
| .github/workflows/changed-files.yml:22:24:22:75 | steps.changed-files.outputs.all_changed_files | .github/workflows/changed-files.yml:16:9:20:6 | Uses Step: changed-files | .github/workflows/changed-files.yml:22:24:22:75 | steps.changed-files.outputs.all_changed_files | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/changed-files.yml:22:24:22:75 | steps.changed-files.outputs.all_changed_files | ${{ steps.changed-files.outputs.all_changed_files }} |
| .github/workflows/changelog.yml:58:26:58:39 | env.log | .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog.yml:58:26:58:39 | env.log | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/changelog.yml:58:26:58:39 | env.log | ${{ env.log }} |
| .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | .github/workflows/changelog_from_prt.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | ${{ env.log }} |
| .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | ${{ github.event.comment.body }} |
| .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | ${{ github.event.comment.body }} |
| .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | ${{ github.event.issue.body }} |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ edges
| .github/workflows/argus_case_study.yml:22:20:22:39 | env.ISSUE_TITLE | .github/workflows/argus_case_study.yml:15:9:24:6 | Uses Step: remove_quotations [replaced] |
| .github/workflows/changed-files.yml:16:9:20:6 | Uses Step: changed-files | .github/workflows/changed-files.yml:22:24:22:75 | steps.changed-files.outputs.all_changed_files |
| .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog.yml:58:26:58:39 | env.log |
| .github/workflows/changelog_from_prt.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log |
| .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] | .github/workflows/cross3.yml:39:31:39:75 | steps.remove_quotations.outputs.replaced |
| .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] | .github/workflows/cross3.yml:57:29:57:73 | steps.remove_quotations.outputs.replaced |
| .github/workflows/cross3.yml:32:18:32:53 | github.event.commits[0].message | .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] |
Expand Down Expand Up @@ -66,6 +67,8 @@ nodes
| .github/workflows/changed-files.yml:22:24:22:75 | steps.changed-files.outputs.all_changed_files | semmle.label | steps.changed-files.outputs.all_changed_files |
| .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | semmle.label | github.event.pull_request.title |
| .github/workflows/changelog.yml:58:26:58:39 | env.log | semmle.label | env.log |
| .github/workflows/changelog_from_prt.yml:49:19:49:56 | github.event.pull_request.title | semmle.label | github.event.pull_request.title |
| .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | semmle.label | env.log |
| .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | semmle.label | github.event.comment.body |
| .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | semmle.label | github.event.comment.body |
| .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | semmle.label | github.event.issue.body |
Expand Down Expand Up @@ -204,6 +207,7 @@ subpaths
#select
| .github/workflows/argus_case_study.yml:27:33:27:77 | steps.remove_quotations.outputs.replaced | .github/workflows/argus_case_study.yml:17:25:17:53 | github.event.issue.title | .github/workflows/argus_case_study.yml:27:33:27:77 | steps.remove_quotations.outputs.replaced | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/argus_case_study.yml:27:33:27:77 | steps.remove_quotations.outputs.replaced | ${{steps.remove_quotations.outputs.replaced}} |
| .github/workflows/changelog.yml:58:26:58:39 | env.log | .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog.yml:58:26:58:39 | env.log | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/changelog.yml:58:26:58:39 | env.log | ${{ env.log }} |
| .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | .github/workflows/changelog_from_prt.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | ${{ env.log }} |
| .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | ${{ github.event.comment.body }} |
| .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | ${{ github.event.comment.body }} |
| .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | ${{ github.event.issue.body }} |
Expand Down

0 comments on commit b6a097c

Please sign in to comment.