diff --git a/ql/lib/codeql/actions/security/CachePoisoningQuery.qll b/ql/lib/codeql/actions/security/CachePoisoningQuery.qll index 318548859b5..e80ea71c958 100644 --- a/ql/lib/codeql/actions/security/CachePoisoningQuery.qll +++ b/ql/lib/codeql/actions/security/CachePoisoningQuery.qll @@ -17,51 +17,40 @@ string defaultBranchNames() { result = default_branch_name ) or - not exists(string default_branch_name | - repositoryDataModel(_, default_branch_name) - ) and + not exists(string default_branch_name | repositoryDataModel(_, default_branch_name)) and result = ["main", "master"] } -predicate runsOnDefaultBranch(Job j) { - exists(Event e | - j.getATriggerEvent() = e and +predicate runsOnDefaultBranch(Event e) { + ( + e.getName() = defaultBranchTriggerEvent() and + not e.getName() = "pull_request_target" + or + e.getName() = "push" and + e.getAPropertyValue("branches") = defaultBranchNames() + or + e.getName() = "pull_request_target" and ( - e.getName() = defaultBranchTriggerEvent() and - not e.getName() = "pull_request_target" + // no filtering + not e.hasProperty("branches") and not e.hasProperty("branches-ignore") or - e.getName() = "push" and + // only branches-ignore filter + e.hasProperty("branches-ignore") and + not e.hasProperty("branches") and + not e.getAPropertyValue("branches-ignore") = defaultBranchNames() + or + // only branches filter + e.hasProperty("branches") and + not e.hasProperty("branches-ignore") and e.getAPropertyValue("branches") = defaultBranchNames() or - e.getName() = "pull_request_target" and - ( - // no filtering - not e.hasProperty("branches") and not e.hasProperty("branches-ignore") - or - // only branches-ignore filter - e.hasProperty("branches-ignore") and - not e.hasProperty("branches") and - not e.getAPropertyValue("branches-ignore") = defaultBranchNames() - or - // only branches filter - e.hasProperty("branches") and - not e.hasProperty("branches-ignore") and - e.getAPropertyValue("branches") = defaultBranchNames() - or - // branches and branches-ignore filters - e.hasProperty("branches") and - e.hasProperty("branches-ignore") and - e.getAPropertyValue("branches") = defaultBranchNames() and - not e.getAPropertyValue("branches-ignore") = defaultBranchNames() - ) + // branches and branches-ignore filters + e.hasProperty("branches") and + e.hasProperty("branches-ignore") and + e.getAPropertyValue("branches") = defaultBranchNames() and + not e.getAPropertyValue("branches-ignore") = defaultBranchNames() ) ) - or - j.getATriggerEvent().getName() = "workflow_call" and - exists(ExternalJob call | - call.getCallee() = j.getLocation().getFile().getRelativePath() and - runsOnDefaultBranch(call) - ) } abstract class CacheWritingStep extends Step { } diff --git a/ql/lib/qlpack.yml b/ql/lib/qlpack.yml index 9acfb3035a4..bf05e80e0a6 100644 --- a/ql/lib/qlpack.yml +++ b/ql/lib/qlpack.yml @@ -2,7 +2,7 @@ library: true warnOnImplicitThis: true name: githubsecuritylab/actions-all -version: 0.0.32 +version: 0.0.33 dependencies: codeql/util: ^0.2.0 codeql/yaml: ^0.1.2 diff --git a/ql/src/Security/CWE-349/CachePoisoning.ql b/ql/src/Security/CWE-349/CachePoisoning.ql index d81c13021c1..a6dc7e14fdd 100644 --- a/ql/src/Security/CWE-349/CachePoisoning.ql +++ b/ql/src/Security/CWE-349/CachePoisoning.ql @@ -16,15 +16,25 @@ import codeql.actions.security.UntrustedCheckoutQuery import codeql.actions.security.CachePoisoningQuery import codeql.actions.security.PoisonableSteps -from LocalJob j, PRHeadCheckoutStep checkout, Step s +from LocalJob j, Event e, PRHeadCheckoutStep checkout, Step s where - // the workflow runs in the context of the default branch - runsOnDefaultBranch(j) and + j.getATriggerEvent() = e and + // job can be triggered by an external user + e.isExternallyTriggerable() and + ( + // the workflow runs in the context of the default branch + runsOnDefaultBranch(e) + or + // the workflow caller runs in the context of the default branch + e.getName() = "workflow_call" and + exists(ExternalJob caller | + caller.getCallee() = j.getLocation().getFile().getRelativePath() and + runsOnDefaultBranch(caller.getATriggerEvent()) + ) + ) and // the job checkouts untrusted code from a pull request // TODO: Consider adding artifact downloads as a potential source of cache poisoning j.getAStep() = checkout and - // job can be triggered by an external user - j.getATriggerEvent().isExternallyTriggerable() and ( // the job writes to the cache // (No need to follow the checkout step as the cache writing is normally done after the job completes) @@ -35,7 +45,7 @@ where // (The cache specific token can be leaked even for non-privileged workflows) checkout.getAFollowingStep() = s and s instanceof PoisonableStep and - // excluding privileged workflows since they can be easily exploited in similar circumstances + // excluding privileged workflows since they can be exploited in easier circumstances not j.isPrivileged() ) select checkout, "Potential cache poisoning in the context of the default branch on step $@.", s, diff --git a/ql/src/Security/CWE-349/CachePoisoningByCodeInjection.ql b/ql/src/Security/CWE-349/CachePoisoningByCodeInjection.ql index 5ed3c966ad3..8fdebdbde18 100644 --- a/ql/src/Security/CWE-349/CachePoisoningByCodeInjection.ql +++ b/ql/src/Security/CWE-349/CachePoisoningByCodeInjection.ql @@ -17,16 +17,26 @@ import codeql.actions.security.CodeInjectionQuery import codeql.actions.security.CachePoisoningQuery import CodeInjectionFlow::PathGraph -from CodeInjectionFlow::PathNode source, CodeInjectionFlow::PathNode sink, LocalJob j +from CodeInjectionFlow::PathNode source, CodeInjectionFlow::PathNode sink, LocalJob j, Event e where - CodeInjectionFlow::flowPath(source, sink) and - j = sink.getNode().asExpr().getEnclosingJob() and + j.getATriggerEvent() = e and // job can be triggered by an external user - j.getATriggerEvent().isExternallyTriggerable() and - // excluding privileged workflows since they can be easily exploited in similar circumstances + e.isExternallyTriggerable() and + ( + // the workflow runs in the context of the default branch + runsOnDefaultBranch(e) + or + // the workflow caller runs in the context of the default branch + e.getName() = "workflow_call" and + exists(ExternalJob caller | + caller.getCallee() = j.getLocation().getFile().getRelativePath() and + runsOnDefaultBranch(caller.getATriggerEvent()) + ) + ) and + // excluding privileged workflows since they can be exploited in easier circumstances not j.isPrivileged() and - // The workflow runs in the context of the default branch - runsOnDefaultBranch(j) + CodeInjectionFlow::flowPath(source, sink) and + j = sink.getNode().asExpr().getEnclosingJob() select sink.getNode(), source, sink, "Unprivileged code injection in $@, which may lead to cache poisoning.", sink, sink.getNode().asExpr().(Expression).getRawExpression() diff --git a/ql/src/qlpack.yml b/ql/src/qlpack.yml index 5637bef68a0..2f79bddd77e 100644 --- a/ql/src/qlpack.yml +++ b/ql/src/qlpack.yml @@ -1,7 +1,7 @@ --- library: false name: githubsecuritylab/actions-queries -version: 0.0.32 +version: 0.0.33 groups: - actions - queries diff --git a/ql/test/query-tests/Security/CWE-349/.github/workflows/test18.yml b/ql/test/query-tests/Security/CWE-349/.github/workflows/test18.yml new file mode 100644 index 00000000000..6bfdc5b7d50 --- /dev/null +++ b/ql/test/query-tests/Security/CWE-349/.github/workflows/test18.yml @@ -0,0 +1,31 @@ +name: Test + +on: + pull_request: + push: + branches: + - main + - 'releases/*' + +jobs: + verify-build: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.sha }} + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version-file: .nvmrc + + - name: Install NPM dependencies + run: npm ci + + - name: Rebuild the dist/ directory + run: npm run build + + - name: Compare the expected and actual dist/ directories + run: bin/check-build-output-in-dist-directory