Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate codecov uploader before executing. #125

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nuclearsandwich
Copy link

@nuclearsandwich nuclearsandwich commented May 3, 2021

After the recent Codecov security incident1 I've been reviewing
codecov usage across ROS repositories.

This script is fetching the codecov bash uploader and env scripts
without performing the recommended validation step.

The validation step does not appear to have been widely explained or
publicised and even the official codecov GitHub action was not
validating the script until the recent security incident.

I have made an attempt to validate the bash uploader here.
The environment script is also used but early enough in the process that
it wasn't convenient to validate with my lack of familiarity in the
travis scripting style.

If there's interest I can probably refactor this to fetch and validate
both scripts during the setup phase instead of trying to do the bash
uploader inline. However I wanted to start with a minimal change.

Author's note: I initially pushed this branch directly to the moveit_ci parent repository in my capacity as an org owner. I have deleted that branch and re-opened the PR from my own fork. Maintainer edits are enabled.

@nuclearsandwich nuclearsandwich self-assigned this May 3, 2021
# Download and validate codecov bash uploader script
travis_run --title "Download codecov uploader" "curl -s 'https://codecov.io/bash' > codecov"
local codecov_version="$(grep -o 'VERSION=\"[0-9\.]*\"' codecov | cut -d'"' -f2);"
travis_run --title "Validate codecov uploader" shasum -a 512 -c <(curl -s "https://raw.githubusercontent.com/codecov/codecov-bash/${codecov_version}/SHA512SUM" | grep -w codecov)
Copy link
Author

Choose a reason for hiding this comment

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

The shasum utility is provided by perl on most systems which is a dependency of git on most systems. If there is a way to explicitly note that dependency in this script I think doing so is worth it.

After the recent Codecov security incident[1] I've been reviewing
codecov usage across ROS repositories.

This script is fetching the codecov bash uploader and env scripts
without performing the recommended validation step.

The validation step does not appear to have been widely explained or
publicised and even the official codecov GitHub action was not
validating the script until the recent security incident.

I have made an attempt to validate the bash uploader here.
The environment script is also used but early enough in the process that
it wasn't convenient to validate with my lack of familiarity in the
travis scripting style.

If there's interest I can probably refactor this to fetch and validate
both scripts during the setup phase instead of trying to do the bash
uploader inline. However I wanted to start with a minimal change.

[1]: https://about.codecov.io/security-update/
Copy link
Contributor

@rhaschke rhaschke left a comment

Choose a reason for hiding this comment

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

Thanks for pointing us at this vulnerability. @tylerjw, please also have a look at the corresponding github actions.

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