3 ms·
> If my repo always runs all tests on a PR, could someone just add a PR with a new test that is then run? Thus running their arbitrary code. Running arbitrary
by oefrha 5y ago
> If my repo always runs all tests on a PR, could someone just add a PR with a new test that is then run? Thus running their arbitrary code.
Running arbitrary code is inevitable if an action is configured to run on all PRs. People have abused this to run crypto miners and stuff in the past, but this for the most part is merely an annoyance to maintainers, not a security problem. It does become a security problem when arbitrary code execution is allowed with your secrets, including your configured secrets and the read/write GITHUB_TOKEN.
Expanding on the topic of secrets, if you trigger your test from the usual pull_request event, the workflow won't have access to GITHUB_TOKEN or configured secrets, so it's the safe default you should almost always choose. That becomes a problem when you need write access to the repo, e.g. to assign labels or add comments to the PR from the workflow, in which case you have to use the privileged pull_request_target event to expose GITHUB_TOKEN and secrets. pull_request_target by default runs in the context of the base of the PR, so there's still no arbitrary code, but you can explicitly check out the PR in that context, and when you do, your secrets are potentially exposed to arbitrary code. If you execute that arbitrary code in any job, or like in this case, post the content of effectively any file on disk as directed by an attacker, boom, owned.
Therefore, you should
- Avoid pull_request_target unless white access to the repo and/or access to configured secrets is absolutely necessary;
- When using pull_request_target, avoid checking out untrusted code;
- If it's absolutely necessary to check out untrusted code, make absolutely sure that the untrusted code isn't executed in any way, and that your trusted handling code can't be tricked by untrusted content in any way, like an arbitrary symlink. This is of course difficult to verify.
In this specific case, the fix seems to be checking that the absolute path of the untrusted advice.txt is within GITHUB_WORKSPACE (https://github.com/check-spelling/check-spelling/commit/436362fc6b588d9d561cbdb575260ca593c8dc56 https://github.com/check-spelling/check-spelling/commit/4363...). IMO that's a wrong fix only covering up the symptom. The real cause is using untrusted configuration files at all; why not make a copy of the trusted version of configuration files and use those instead???
GitHub has an article about security considerations here: https://securitylab.github.com/research/github-actions-preventing-pwn-requests/ https://securitylab.github.com/research/github-actions-preve...