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

Handle weird input paths #19

Merged
merged 2 commits into from
Sep 2, 2022
Merged

Conversation

kylewlacy
Copy link
Contributor

I saw a recent blog post by Joren Vrancken about a command injection vulnerability in the GH Pages build pipeline. A small code snippet from this repo was shared, which showed how the input path provided to this action was not being escaped. This action could be triggered by an HTTP endpoint used when changing Jekyll themes, which is ultimately what led to the vulnerability.

The vulnerability has since been resolved by disabling the Jekyll theme picker in the UI (and the associated HTTP endpoint), but the underlying bug in this action still exists: paths are still interpolated directly in a shell script instead of being quoted or escaped properly. Practically speaking, this means that this action doesn't allow input paths with spaces or quotes.

This PR fixes that bug by passing the input paths as an env var, rather than interpolating the string in the shell script directly. I even set up some test repositories to exercise this, so now even repo layouts with weird path names work, including spaces and double quotes.

@kylewlacy kylewlacy requested a review from a team as a code owner August 27, 2022 20:12
Copy link
Contributor

@JamesMGreene JamesMGreene left a comment

Choose a reason for hiding this comment

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

We appreciate you bringing this to our attention. It's always good to close these gaps!

action.yml Outdated Show resolved Hide resolved
action.yml Outdated Show resolved Hide resolved
action.yml Outdated Show resolved Hide resolved
@JamesMGreene JamesMGreene merged commit a597aec into actions:main Sep 2, 2022
@JamesMGreene
Copy link
Contributor

This fix is now available in the following versions, ordered from most specific/secure to least:

uses: actions/upload-pages-artifact@a597aecd27af1cf14095ccaa29169358e3d91e28
uses: actions/[email protected]
uses: actions/upload-pages-artifact@v1
uses: actions/upload-pages-artifact@main

🚀

@kylewlacy kylewlacy deleted the handle-weird-paths branch September 3, 2022 19:59
@yoannchaudet yoannchaudet mentioned this pull request Sep 12, 2022
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