Workflows disabled pending vulnerability investigation

[joeyparrish]

A vulnerable workflow exposed this repo to risk of manipulation. All GitHub Actions have been disabled pending investigation of this vulnerability.

Existing releases, tags, and branches are clean and have not been poisoned. Binaries released via PyPi are also clean.

[joeyparrish]

Default permissions for workflows have been fixed organization-wide. Some workflows now need to have permissions added selectively.

[joeyparrish]

Workflows have been fully audited and re-enabled for this repository.

A full discussion of the workflow vulnerability will follow soon.

[joeyparrish]

TL;DR

Above all else, I want to emphasize this:

A vulnerability was found in one of our workflows, not our source code or binaries. The vulnerability was discovered by a researcher who reported it to Google and did not damage anything. Workflows were suspended within hours, and no evidence of compromise has been found in any repos in the weeks I've been working on this.

If you want more details, read on. Questions are welcome.

The main vulnerability

The vulnerability relied on two main things:

1. The default GITHUB_TOKEN permissions were read-write
2. There was a way to inject code into the workflow in context of that token

Versions of those two things were found in several other repos across shaka-project.

As soon as the exploit PR was noticed on shaka-streamer, workflows were paused on shaka-streamer and static-ffmpeg-binaries (both used the exact same pattern). This gave me time to investigate the PR, what it did, and how.

The exploit PR replaced build-matrix.json with build-matrix.json.js, which allowed arbitrary code to execute in context of the workflow. This is because the nodejs code require("./build-matrix.json") triggers a search that first looks for the JSON (which will be automatically read and parsed), then looks for that as the name of a local module (which will be executed).

Because the default token was read-write, the PR author was able to use that token to make live calls to the GitHub API to create a new release as a proof of concept. They could just as easily have modified an existing release, replaced attached binaries on that release, pushed a new branch, pushed to an existing unprotected branch, or created or modified tags.

The main branch was protected, so they would not have been able to push code directly to main without a merged PR. The PR author was a security researcher who did not intend us any harm, and I have verified that no existing releases, tags, or binaries were tampered with.

Mitigations

Reduce default token permissions

The repositories and the organization itself all had their default token permissions set to read-write, as in this screenshot:

So the first mitigation was to update all repository settings, first at the organization, then per-repo in each one I had a fork of. This was done with the gh command line to do it in a semi-automated way across all of those repos.

Next, I engaged in a systematic review of all repositories and workflows. There are 16 active repos under shaka-project, with many workflows each.

I prioritized the most popular/active repositories, as well as those whose maintainers were waiting to make immediate releases.

The default tokens were all basically safe now, but some workflows used non-default tokens for special actions or to do work as specific actors (like @shaka-bot). I made a list of workflows using non-default tokens, and checked how those workflows could be triggered. All were triggerable by maintainers, schedule, or commits to main, and none were exploitable by PRs or any code that had not been reviewed and merged.

Add back granular permissions

The new default token permissions had crippled some workflows that depended on those implicit permissions. So for each workflow, I triggered it, either on the main repo or on my fork, as appropriate. For the ones that failed, I added explicit, granular, and minimal permissions, isolated to the job.

Prevent JSON module code injection

Our (lazy) use of require("./build-matrix.json") was an opportunity for code injection. The solution was simple: read the file explicitly with fs, then parse it separately with JSON.parse(), eliminating the module search.

This was the only code injection used by the researcher, but this was not the only possible path to code injection discovered during the full audit. In a couple of cases, a job was too big and complex and mixed actions that involved PR-controlled code with actions that required privileges. These were split up to isolate permissions, with minimal state transferred between jobs, e.g. by something like actions/cache. It's important to separate jobs, not just steps, since each job runs in a fresh container, preventing previous jobs from tampering with the base image in a way that impacts the next job.

Avoid persisted credentials in actions/checkout

actions/checkout will use your token to read the remote repo, and by default, it will persist those credentials in the local repository. This is generally unnecessary and can lead to unexpected leakage of tokens in long-running workflows. This did not appear to be a factor in the exploit we experienced, but we would rather be ahead of issues in the future, so we took care of this during the post-exploit audit.

The fix is simple: explicitly set persist-credentials: false when calling actions/checkout. There are very few exceptions, each of which has an explicit true setting and clear comments about why we need it, and how we ensure it's safe.

For detailed write-ups on the problem with the default setting, see actions/checkout#485 and https://johnstawinski.com/2024/01/11/playing-with-fire-how-we-executed-a-critical-supply-chain-attack-on-pytorch/ .

All PRs addressing workflow security, organization-wide

PRs to add granular permissions to workflows:

• fix: Set explicit permissions in shared workflows shaka-github-tools#42
• fix: Remove redundant permissions from update-issues shaka-github-tools#44
• ci: Update workflow permissions shaka-player#7731
• ci: Update release workflow tokens and permissions eme_logger#87
• ci: Use GitHub API directly to post PR coverage comments shaka-player#7758
• ci: Update workflow permissions shaka-packager#1462
• ci: Update release workflow tokens and permissions eme-encryption-scheme-polyfill#88
• ci: Update release workflow tokens and permissions generic-webdriver-server#114
• ci: Update release workflow tokens and permissions karma-local-wd-launcher#82
• ci: Update release workflow #223
• ci: Update release workflow tokens and permissions trace-anything#21
• ci: Update release workflow tokens and permissions webdriver-installer#43
• ci: Fix macOS deployment workflow shaka-lab#73

PRs to prevent someone injecting JS in place of build-matrix.json:

• ci: Read build matrix JSON explicitly shaka-packager#1461
• ci: Read build matrix JSON explicitly #219
• ci: Read build matrix JSON explicitly static-ffmpeg-binaries#59

PRs to refactor workflows to isolate and/or simplify permissions:

• ci: Restructure release workflow static-ffmpeg-binaries#60
• ci: Refactor screenshot workflow shaka-player#7773

PRs to disable persisted checkout credentials:

• ci: Do not persist credentials after checkout shaka-player#7772
• ci: Do not persist credentials after checkout shaka-github-tools#45
• ci: Do not persist credentials after checkout eme-encryption-scheme-polyfill#84
• ci: Do not persist credentials after checkout eme_logger#89
• ci: Do not persist credentials after checkout generic-webdriver-server#112
• ci: Do not persist credentials after checkout karma-local-wd-launcher#79
• ci: Do not persist credentials after checkout shaka-lab#71
• ci: Do not persist credentials after checkout shaka-packager#1464
• ci: Do not persist credentials after checkout #221
• ci: Do not persist credentials after checkout trace-anything#19
• ci: Do not persist credentials after checkout webdriver-installer#40
• ci: Do not persist credentials after checkout static-ffmpeg-binaries#62