diff --git a/ql/src/Security/CWE-367/UntrustedCheckoutTOCTOUCritical.ql b/ql/src/Security/CWE-367/UntrustedCheckoutTOCTOUCritical.ql new file mode 100644 index 00000000000..c5e12c0fccc --- /dev/null +++ b/ql/src/Security/CWE-367/UntrustedCheckoutTOCTOUCritical.ql @@ -0,0 +1,25 @@ +/** + * @name Untrusted Checkout TOCTOU + * @description Untrusted Checkout is protected by a security check but the checked-out branch can be changed after the check. + * @kind problem + * @problem.severity error + * @precision high + * @security-severity 9.3 + * @id actions/untrusted-checkout-toctou/critical + * @tags actions + * security + * external/cwe/cwe-367 + */ + +import actions +import codeql.actions.security.UntrustedCheckoutQuery +import codeql.actions.security.PoisonableSteps + +from ControlCheck check, MutableRefCheckoutStep checkout +where + // the mutable checkout step is protected by an access check + check = [checkout.getIf(), checkout.getEnclosingJob().getIf()] and + // the checked-out code may lead to arbitrary code execution + checkout.getAFollowingStep() instanceof PoisonableStep +select checkout, "The checked-out code can be changed after the authorization check o step $@.", + check, check.toString() diff --git a/ql/src/Security/CWE-367/UntrustedCheckoutTOCTOUHigh.ql b/ql/src/Security/CWE-367/UntrustedCheckoutTOCTOUHigh.ql new file mode 100644 index 00000000000..b74c3389f9d --- /dev/null +++ b/ql/src/Security/CWE-367/UntrustedCheckoutTOCTOUHigh.ql @@ -0,0 +1,25 @@ +/** + * @name Untrusted Checkout TOCTOU + * @description Untrusted Checkout is protected by a security check but the checked-out branch can be changed after the check. + * @kind problem + * @problem.severity warning + * @precision medium + * @security-severity 5.3 + * @id actions/untrusted-checkout-toctou/high + * @tags actions + * security + * external/cwe/cwe-367 + */ + +import actions +import codeql.actions.security.UntrustedCheckoutQuery +import codeql.actions.security.PoisonableSteps + +from ControlCheck check, MutableRefCheckoutStep checkout +where + // the mutable checkout step is protected by an access check + check = [checkout.getIf(), checkout.getEnclosingJob().getIf()] and + // there are no evidences that the checked-out code can lead to arbitrary code execution + not checkout.getAFollowingStep() instanceof PoisonableStep +select checkout, "The checked-out code can be changed after the authorization check o step $@.", + check, check.toString() diff --git a/ql/test/query-tests/Security/CWE-367/.github/workflows/comment.yml b/ql/test/query-tests/Security/CWE-367/.github/workflows/comment.yml new file mode 100644 index 00000000000..498b46090cb --- /dev/null +++ b/ql/test/query-tests/Security/CWE-367/.github/workflows/comment.yml @@ -0,0 +1,41 @@ +# https://github.com/AdnaneKhan/ActionsTOCTOU/blob/main/.github/workflows/comment_victim.yml +name: Comment Triggered Test +on: + issue_comment: + types: [created] +permissions: 'write-all' +jobs: + benchmark: + name: Integration Tests + if: ${{ github.event.issue.pull_request && contains(fromJson('["MEMBER", "OWNER"]'), github.event.comment.author_association) && startsWith(github.event.comment.body, '/run-tests ') }} + runs-on: [ubuntu-latest] + steps: + + # test1 + - uses: actions/github-script@v6 + name: Get PR branch + id: issue + with: + script: | + const pr = context.payload.issue.number + const data = await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: pr + }) + return { + ref: data.data.head.ref, + sha: data.data.head.sha, + } + - uses: actions/checkout@v4 + with: + submodules: recursive + ref: ${{ fromJson(steps.issue.outputs.result).sha }} + - run: bash comment_example/tests.sh + + # test2 + - uses: actions/checkout@v4 + with: + submodules: recursive + ref: "refs/pull/${{ github.event.number }}/merge" + - run: bash comment_example/tests.sh diff --git a/ql/test/query-tests/Security/CWE-367/.github/workflows/deployment.yml b/ql/test/query-tests/Security/CWE-367/.github/workflows/deployment.yml new file mode 100644 index 00000000000..f0a3035777c --- /dev/null +++ b/ql/test/query-tests/Security/CWE-367/.github/workflows/deployment.yml @@ -0,0 +1,31 @@ +# https://github.com/AdnaneKhan/ActionsTOCTOU/blob/main/.github/workflows/deployment_victim.yml +name: Environment PR Check + +on: + pull_request_target: + branches: + - main + paths: + - 'README.md' + workflow_dispatch: +jobs: + test: + environment: Public CI + runs-on: ubuntu-latest + steps: + - name: Checkout from PR branch + uses: actions/checkout@v4 + with: + repository: ${{ github.event.pull_request.head.repo.full_name }} + ref: ${{ github.event.pull_request.head.ref }} + + - name: Set Node.js 20.x for GitHub Action + uses: actions/setup-node@v4 + with: + node-version: 20.x + + - name: installing node_modules + run: cd deployment_example && npm install + + - name: Build GitHub Action + run: cd deployment_example && npm run build diff --git a/ql/test/query-tests/Security/CWE-367/.github/workflows/label.yml b/ql/test/query-tests/Security/CWE-367/.github/workflows/label.yml new file mode 100644 index 00000000000..1f04440d28b --- /dev/null +++ b/ql/test/query-tests/Security/CWE-367/.github/workflows/label.yml @@ -0,0 +1,17 @@ +# https://github.com/AdnaneKhan/ActionsTOCTOU/blob/main/.github/workflows/label_victim.yml +name: Label Trigger Test +on: + pull_request_target: + types: [labeled] + branches: [main] + +jobs: + integration-tests: + runs-on: ubuntu-latest + if: contains(github.event.pull_request.labels.*.name, 'safe-to-test') + steps: + - uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.ref }} + repository: ${{ github.event.pull_request.head.repo.full_name }} + - run: bash label_example/tests.sh diff --git a/ql/test/query-tests/Security/CWE-367/UntrustedCheckoutTOCTOUCritical.expected b/ql/test/query-tests/Security/CWE-367/UntrustedCheckoutTOCTOUCritical.expected new file mode 100644 index 00000000000..e3a42b3265d --- /dev/null +++ b/ql/test/query-tests/Security/CWE-367/UntrustedCheckoutTOCTOUCritical.expected @@ -0,0 +1,2 @@ +| .github/workflows/comment.yml:37:9:41:6 | Uses Step | The checked-out code can be changed after the authorization check o step $@. | .github/workflows/comment.yml:10:9:10:188 | ${{ git ... s ') }} | ${{ git ... s ') }} | +| .github/workflows/label.yml:13:9:17:6 | Uses Step | The checked-out code can be changed after the authorization check o step $@. | .github/workflows/label.yml:11:9:11:73 | contain ... -test') | contain ... -test') | diff --git a/ql/test/query-tests/Security/CWE-367/UntrustedCheckoutTOCTOUCritical.qlref b/ql/test/query-tests/Security/CWE-367/UntrustedCheckoutTOCTOUCritical.qlref new file mode 100644 index 00000000000..f924f8fe750 --- /dev/null +++ b/ql/test/query-tests/Security/CWE-367/UntrustedCheckoutTOCTOUCritical.qlref @@ -0,0 +1 @@ +Security/CWE-367/UntrustedCheckoutTOCTOUCritical.ql diff --git a/ql/test/query-tests/Security/CWE-367/UntrustedCheckoutTOCTOUHigh.expected b/ql/test/query-tests/Security/CWE-367/UntrustedCheckoutTOCTOUHigh.expected new file mode 100644 index 00000000000..e69de29bb2d diff --git a/ql/test/query-tests/Security/CWE-367/UntrustedCheckoutTOCTOUHigh.qlref b/ql/test/query-tests/Security/CWE-367/UntrustedCheckoutTOCTOUHigh.qlref new file mode 100644 index 00000000000..6284c786b3a --- /dev/null +++ b/ql/test/query-tests/Security/CWE-367/UntrustedCheckoutTOCTOUHigh.qlref @@ -0,0 +1 @@ +Security/CWE-367/UntrustedCheckoutTOCTOUHigh.ql