Skip to content

Conversation

@pwntester
Copy link
Contributor

@pwntester pwntester commented Sep 6, 2024

This query models the issue coined as ArtiPACKED where the actions/checkout action stores the GITHUB_TOKEN into .git/config and then this directory gets included into an artifact.

The latest version of actions/upload-artifact has been patched to prevent hidden files to be included into the artifact. This query reports those cases where users are both using an unpatched version of actions/upload-artifact and also using actions/checkout in a way that persists credentials into .git directory.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 6, 2024

QHelp previews:

javascript/ql/src/Security/CWE-312/ActionsArtifactLeak.qhelp

Storage of sensitive information in GitHub Actions artifact

Sensitive information included in a GitHub Actions artifact can allow an attacker to access the sensitive information if the artifact is published.

Recommendation

Only store information that is meant to be publicly available in a GitHub Actions artifact.

Example

The following example uses actions/checkout to checkout code which stores the GITHUB_TOKEN in the `.git/config` file and then stores the contents of the `.git` repository into the artifact:

name: secrets-in-artifacts
on:
  pull_request:
jobs:
  a-job: # VULNERABLE
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - name: "Upload artifact"
        uses: actions/upload-artifact@1746f4ab65b179e0ea60a494b83293b640dd5bba # v4.3.2
        with:
          name: file
          path: .

The issue has been fixed below, where the actions/upload-artifact uses a version (v4+) which does not include hidden files or directories into the artifact.

name: secrets-in-artifacts
on:
  pull_request:
jobs:
  a-job: # NOT VULNERABLE
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - name: "Upload artifact"
        uses: actions/upload-artifact@v4
        with:
          name: file
          path: .

Copy link
Contributor

@intrigus-lgtm intrigus-lgtm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work 👍
You could maybe add a test for persist-credentials: false for completeness.

@pwntester
Copy link
Contributor Author

pwntester commented Sep 6, 2024

Thanks @intrigus-lgtm, will do
UPDATE: Actually, that uncovered a small bug, thanks!

@pwntester pwntester changed the title [javascript] Query to detect GITHUB_TOKEN leaked in artifacts Javascript: Query to detect GITHUB_TOKEN leaked in artifacts Sep 6, 2024
Copy link
Contributor

@asgerf asgerf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit in the change note otherwise LGTM

…-leak-query.md

Co-authored-by: Asger F <asgerf@github.com>
@asgerf
Copy link
Contributor

asgerf commented Sep 11, 2024

I've started a DCA run, let's merge if that looks good.

@asgerf
Copy link
Contributor

asgerf commented Sep 11, 2024

DCA run looks good, merging

@asgerf asgerf merged commit 07bd854 into github:main Sep 11, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants