Coordinated Disclosure Timeline
- 2020-11-26: Report sent to maintainer
- 2020-11-26: Maintainer acknowledges
- 2020-12-05: All issues resolved
Summary
The ‘acceptance.yml’ GitHub workflow is vulnerable to arbitrary code execution and shell command injection.
Product
getsentry/sentry GitHub repository
Tested Version
The latest changeset f325d54 to the date.
Details
Issue 1: Untrusted code is explicitly checked out and run on a Pull Request from a fork
pull_request_target was introduced to allow triggered workflows to comment on PRs, label them, assign people, etc.. In order to make it possible the triggered action runner has read/write token for the base repository and the access to secrets. In order to prevent untrusted code from execution it runs in a context of the base repository.
By explicitly checking out and running build script from a fork the untrusted code is running in an environment that is able to push to the base repository and to access secrets.
on:
push:
branches:
- master
- releases/**
# XXX: We are using `pull_request_target` instead of `pull_request` because we want
# Visual Snapshots to run on forks. It allows forks to access secrets safely by
# only running workflows from the main branch. Prefer to use `pull_request` when possible.
#
# See https://github.com/getsentry/sentry/pull/21600 for more details
pull_request_target:
...
steps:
- uses: actions/checkout@v2
name: Checkout sentry (pull_request_target)
if: github.event.pull_request.head.ref != ''
with:
# Note this is required because of `pull_request_target`, which allows
# forks to access secrets safely by only running workflows from the main branch.
ref: ${{ github.event.pull_request.head.ref }}
repository: ${{ github.event.pull_request.head.repo.full_name }}
...
- name: Install dependencies
run: yarn install --frozen-lockfile
- name: jest
run: |
NODE_ENV=production yarn build-css
yarn test-ci --forceExit
Impact
The vulnerability allows for unauthorized modification of the base repository and secrets exfiltration.
Issue 2: The forked branch name is used to format a shell command
The forked branch name is used to format a bash script.
- name: Get merge base
if: github.event.pull_request.head.ref != ''
id: merge-base
run: |
echo "::set-output name=sha::$(git merge-base origin/master $)"
This vulnerability allows for arbitrary command injection into the bash script. For a Proof of Concept create a PR from branch named a";echo${IFS}"hello"#.
Impact
The injection allows for exfiltration of secrets and the temporary GitHub repository authorization token to the attacker controlled server. Although the token is not valid after the workflow finishes, since the attacker controls the execution of the workflow he or she can delay it to give the malicious server time to modify the repository.
Credit
This issue was discovered and reported by GHSL team member @JarLob (Jaroslav Lobačevski).
Contact
You can contact the GHSL team at securitylab@github.com, please include a reference to GHSL-2020-241 in any communication regarding this issue.