diff --git a/ql/lib/codeql/actions/config/Config.qll b/ql/lib/codeql/actions/config/Config.qll index a439f999623..a21f3e358d1 100644 --- a/ql/lib/codeql/actions/config/Config.qll +++ b/ql/lib/codeql/actions/config/Config.qll @@ -120,7 +120,7 @@ predicate vulnerableActionsDataModel( } /** - * MaD models for vulnerable actions + * MaD models for immutable actions * Fields: * - action: action name */ diff --git a/ql/lib/codeql/actions/security/UseOfUnversionedImmutableAction.qll b/ql/lib/codeql/actions/security/UseOfUnversionedImmutableAction.qll new file mode 100644 index 00000000000..2be71612f26 --- /dev/null +++ b/ql/lib/codeql/actions/security/UseOfUnversionedImmutableAction.qll @@ -0,0 +1,11 @@ +import actions + +class UnversionedImmutableAction extends UsesStep { + string immutable_action; + + UnversionedImmutableAction() { + immutableActionsDataModel(immutable_action) and + this.getCallee() = immutable_action and + not this.getVersion().regexpMatch("^(v)?[0-9]+(\\.[0-9]+)*(\\.[xX])?$") + } +} diff --git a/ql/src/Security/CWE-829/UnversionedImmutableAction.ql b/ql/src/Security/CWE-829/UnversionedImmutableAction.ql index d9a1394641f..0c6443bc3e6 100644 --- a/ql/src/Security/CWE-829/UnversionedImmutableAction.ql +++ b/ql/src/Security/CWE-829/UnversionedImmutableAction.ql @@ -2,7 +2,6 @@ * @name Unversioned Immutable Action * @description Using an Immutable Action without a semantic version tag opts out of the protections of Immutable Action * @kind problem - * @security-severity 5.0 * @problem.severity recommendation * @precision high * @id actions/unversioned-immutable-action @@ -12,27 +11,9 @@ */ import actions +import codeql.actions.security.UseOfUnversionedImmutableAction -bindingset[version] -private predicate isSemanticVersioned(string version) { version.regexpMatch("^v[0-9]+(\\.[0-9]+)*(\\.[xX])?$") } - -bindingset[repo] -private predicate isTrustedOrg(string repo) { - exists(string org | org in ["actions", "github", "advanced-security", "octokit"] | repo.matches(org + "/%")) -} - -from UsesStep uses, string repo, string version, Workflow workflow, string name -where - uses.getCallee() = repo and - uses.getEnclosingWorkflow() = workflow and - ( - workflow.getName() = name - or - not exists(workflow.getName()) and workflow.getLocation().getFile().getBaseName() = name - ) and - uses.getVersion() = version and - not isTrustedOrg(repo) and - not isPinnedCommit(version) -select uses.getCalleeNode(), - "Unpinned 3rd party Action '" + name + "' step $@ uses '" + repo + "' with ref '" + version + - "', not a pinned commit hash", uses, uses.toString() +from UnversionedImmutableAction step +select step, + "The workflow is using an immutable action ($@) without versinoning so it doesn't work", step, + step.getCallee() \ No newline at end of file diff --git a/ql/test/query-tests/Security/CWE-829/.github/actions/dangerous-git-checkout/action.yml b/ql/test/query-tests/Security/CWE-829/.github/actions/dangerous-git-checkout/action.yml index 57058e7a076..cd4f0fe660a 100644 --- a/ql/test/query-tests/Security/CWE-829/.github/actions/dangerous-git-checkout/action.yml +++ b/ql/test/query-tests/Security/CWE-829/.github/actions/dangerous-git-checkout/action.yml @@ -4,7 +4,7 @@ runs: using: "composite" steps: - name: Checkout repo - uses: actions/checkout@v4 + uses: actions/checkout@4 with: ref: ${{ github.event.pull_request.head.sha }} fetch-depth: 2 diff --git a/ql/test/query-tests/Security/CWE-829/UnversionedImmutableAction.expected b/ql/test/query-tests/Security/CWE-829/UnversionedImmutableAction.expected new file mode 100644 index 00000000000..5ae46862fb4 --- /dev/null +++ b/ql/test/query-tests/Security/CWE-829/UnversionedImmutableAction.expected @@ -0,0 +1,19 @@ +| .github/actions/download-artifact-2/action.yaml:6:7:25:4 | Uses Step | The workflow is using an immutable action ($@) without versinoning so it doesn't work | .github/actions/download-artifact-2/action.yaml:6:7:25:4 | Uses Step | actions/github-script | +| .github/actions/download-artifact/action.yaml:6:7:25:4 | Uses Step | The workflow is using an immutable action ($@) without versinoning so it doesn't work | .github/actions/download-artifact/action.yaml:6:7:25:4 | Uses Step | actions/github-script | +| .github/workflows/artifactpoisoning91.yml:17:9:18:6 | Uses Step | The workflow is using an immutable action ($@) without versinoning so it doesn't work | .github/workflows/artifactpoisoning91.yml:17:9:18:6 | Uses Step | actions/checkout | +| .github/workflows/artifactpoisoning91.yml:25:9:28:6 | Uses Step | The workflow is using an immutable action ($@) without versinoning so it doesn't work | .github/workflows/artifactpoisoning91.yml:25:9:28:6 | Uses Step | actions/checkout | +| .github/workflows/artifactpoisoning92.yml:17:9:18:6 | Uses Step | The workflow is using an immutable action ($@) without versinoning so it doesn't work | .github/workflows/artifactpoisoning92.yml:17:9:18:6 | Uses Step | actions/checkout | +| .github/workflows/artifactpoisoning92.yml:25:9:28:6 | Uses Step | The workflow is using an immutable action ($@) without versinoning so it doesn't work | .github/workflows/artifactpoisoning92.yml:25:9:28:6 | Uses Step | actions/checkout | +| .github/workflows/poc.yml:30:9:36:6 | Uses Step | The workflow is using an immutable action ($@) without versinoning so it doesn't work | .github/workflows/poc.yml:30:9:36:6 | Uses Step | actions/checkout | +| .github/workflows/poc.yml:36:9:38:6 | Uses Step | The workflow is using an immutable action ($@) without versinoning so it doesn't work | .github/workflows/poc.yml:36:9:38:6 | Uses Step | actions/configure-pages | +| .github/workflows/poc.yml:43:9:47:2 | Uses Step | The workflow is using an immutable action ($@) without versinoning so it doesn't work | .github/workflows/poc.yml:43:9:47:2 | Uses Step | actions/upload-pages-artifact | +| .github/workflows/poc.yml:59:9:63:26 | Uses Step: deployment | The workflow is using an immutable action ($@) without versinoning so it doesn't work | .github/workflows/poc.yml:59:9:63:26 | Uses Step: deployment | actions/deploy-pages | +| .github/workflows/priv_pull_request_checkout.yml:14:9:20:6 | Uses Step | The workflow is using an immutable action ($@) without versinoning so it doesn't work | .github/workflows/priv_pull_request_checkout.yml:14:9:20:6 | Uses Step | actions/checkout | +| .github/workflows/test8.yml:20:9:26:6 | Uses Step | The workflow is using an immutable action ($@) without versinoning so it doesn't work | .github/workflows/test8.yml:20:9:26:6 | Uses Step | actions/checkout | +| .github/workflows/test9.yml:11:9:16:6 | Uses Step | The workflow is using an immutable action ($@) without versinoning so it doesn't work | .github/workflows/test9.yml:11:9:16:6 | Uses Step | actions/checkout | +| .github/workflows/test11.yml:84:7:90:4 | Uses Step | The workflow is using an immutable action ($@) without versinoning so it doesn't work | .github/workflows/test11.yml:84:7:90:4 | Uses Step | actions/checkout | +| .github/workflows/test12.yml:86:7:92:4 | Uses Step | The workflow is using an immutable action ($@) without versinoning so it doesn't work | .github/workflows/test12.yml:86:7:92:4 | Uses Step | actions/checkout | +| .github/workflows/test14.yml:101:7:105:4 | Uses Step | The workflow is using an immutable action ($@) without versinoning so it doesn't work | .github/workflows/test14.yml:101:7:105:4 | Uses Step | actions/checkout | +| .github/workflows/test14.yml:105:7:111:4 | Uses Step | The workflow is using an immutable action ($@) without versinoning so it doesn't work | .github/workflows/test14.yml:105:7:111:4 | Uses Step | actions/checkout | +| .github/workflows/test15.yml:60:7:65:4 | Uses Step | The workflow is using an immutable action ($@) without versinoning so it doesn't work | .github/workflows/test15.yml:60:7:65:4 | Uses Step | actions/checkout | +| .github/workflows/test15.yml:110:7:115:4 | Uses Step | The workflow is using an immutable action ($@) without versinoning so it doesn't work | .github/workflows/test15.yml:110:7:115:4 | Uses Step | actions/checkout | diff --git a/ql/test/query-tests/Security/CWE-829/UnversionedImmutableAction.qlref b/ql/test/query-tests/Security/CWE-829/UnversionedImmutableAction.qlref new file mode 100644 index 00000000000..6ce4123fa5e --- /dev/null +++ b/ql/test/query-tests/Security/CWE-829/UnversionedImmutableAction.qlref @@ -0,0 +1 @@ +Security/CWE-829/UnversionedImmutableAction.ql \ No newline at end of file