Skip to content

fix Expression injection in Actions#23358

Merged
diox merged 1 commit intomozilla:masterfrom
odaysec:patch-1
Apr 24, 2025
Merged

fix Expression injection in Actions#23358
diox merged 1 commit intomozilla:masterfrom
odaysec:patch-1

Conversation

@odaysec
Copy link
Contributor

@odaysec odaysec commented Apr 22, 2025

run: |
event_name="${{ github.event_name }}"
event_action="${{ github.event.action }}"
# Stable check for if the workflow is running on the default branch
# https://stackoverflow.com/questions/64781462/github-actions-default-branch-variable
is_default_branch="${{ format('refs/heads/{0}', env.default_branch) == github.ref }}"
# In most events, the epository refers to the head which would be the fork
is_fork="${{ github.event.repository.fork }}"
# Default version is the branch name
docker_version="${{ github.ref_name }}"
# This is different in a pull_request where we need to check the head explicitly
if [[ "${{ github.event_name }}" == 'pull_request' ]]; then
# repository on a pull request refers to the base which is always mozilla/addons-server
is_head_fork="${{ github.event.pull_request.head.repo.fork }}"
# https://docs.github.com/en/code-security/dependabot/working-with-dependabot/automating-dependabot-with-github-actions
is_dependabot="${{ github.actor == 'dependabot[bot]' }}"
# For PRs we need to reference the head branch
docker_version="${{ github.head_ref }}"
# If the head repository is a fork or if the PR is opened by dependabot
# we consider the run to be a fork. Dependabot and proper forks are treated
# the same in terms of limited read only github token scope
if [[ "$is_head_fork" == 'true' || "$is_dependabot" == 'true' ]]; then
is_fork="true"
fi
fi
is_release_master="false"
is_release_tag="false"
# Releases can only happen if we are NOT on a fork
if [[ "$is_fork" == 'false' ]]; then
# A master release occurs on a push to the default branch of the origin repository
if [[ "$event_name" == 'push' && "$is_default_branch" == 'true' ]]; then
is_release_master="true"
# If we are releasing master, we tag latest
docker_version="latest"
fi
# A tag release occurs when a release is published
if [[ "$event_name" == 'release' && "$event_action" == 'published' ]]; then
is_release_tag="true"
# If we are releasing a tag, we tag the docker version as the git tag
docker_version="${{ github.event.release.tag_name }}"
fi
fi
echo "is_default_branch=$is_default_branch" >> $GITHUB_OUTPUT
echo "is_fork=$is_fork" >> $GITHUB_OUTPUT
echo "is_release_master=$is_release_master" >> $GITHUB_OUTPUT
echo "is_release_tag=$is_release_tag" >> $GITHUB_OUTPUT
echo "docker_version=$docker_version" >> $GITHUB_OUTPUT
echo "event_name: $event_name"
cat $GITHUB_OUTPUT

fix the issue will follow the recommended best practice of assigning the untrusted input (${{ github.head_ref }}) to an environment variable and referencing it using shell syntax ($VAR). This approach ensures that the input is treated as a literal string by the shell, mitigating the risk of command injection.

Specifically:

  1. Replace the direct use of ${{ github.head_ref }} in the script with an environment variable (e.g., HEAD_REF).
  2. Reference the environment variable using $HEAD_REF in the script.

Using user-controlled input in GitHub Actions may lead to code injection in contexts like run: or script:. Code injection in GitHub Actions may allow an attacker to exfiltrate any secrets used in the workflow and the temporary GitHub repository authorization token. The token might have write access to the repository, allowing an attacker to use the token to make changes to the repository.

The following lets a user inject an arbitrary shell command:

on: issue_comment

jobs:
  echo-body:
    runs-on: ubuntu-latest
    steps:
    - run: |
        echo '${{ github.event.comment.body }}'

The following uses an environment variable, but still allows the injection because of the use of expression syntax:

on: issue_comment

jobs:
  echo-body:
    runs-on: ubuntu-latest
    steps:
    -  env:
        BODY: ${{ github.event.issue.body }}
      run: |
        echo '${{ env.BODY }}'

The following uses shell syntax to read the environment variable and will prevent the attack:

on: issue_comment

jobs:
  echo-body:
    runs-on: ubuntu-latest
    steps:
    - env:
        BODY: ${{ github.event.issue.body }}
      run: |
        echo "$BODY"

References

Keeping your GitHub Actions and workflows secure: Untrusted input
Security hardening for GitHub Actions
Permissions for the GITHUB_TOKEN
CWE-94

  • Add #ISSUENUM at the top of your PR to an existing open issue in the mozilla/addons repository.
  • Successfully verified the change locally.
  • The change is covered by automated tests, or otherwise indicated why doing so is unnecessary/impossible.
  • Add before and after screenshots (Only for changes that impact the UI).
  • Add or update relevant docs reflecting the changes made.

@diox
Copy link
Member

diox commented Apr 24, 2025

Thanks for the pull request, that looks good. We have more planned in mozilla/addons#15312 but it's a good first step.

@diox diox merged commit 61e5979 into mozilla:master Apr 24, 2025
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments