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

Please consider adding input safelisting #10

Closed
franky47 opened this issue Mar 18, 2020 · 4 comments
Closed

Please consider adding input safelisting #10

franky47 opened this issue Mar 18, 2020 · 4 comments

Comments

@franky47
Copy link

GitHub Actions can be vulnerable to environment injection for optional inputs, see my article here:
https://francoisbest.com/posts/2020/the-security-of-github-actions

One form of defence against that for now is to add another input that lists input names that are safe to load (because explicitly defined by the user in their workflow).

Example:

uses: docker/build-push-action@v1
with:
  username: ${{ secrets.DOCKER_USERNAME }}
  password: ${{ secrets.DOCKER_PASSWORD }}
  repository: myorg/myrepository
  tags: latest
  inputsSafeList: username,password,repository,tags

In this example, if a malicious action defines and exports INPUT_REGISTRY, it would be ignored as registry is not part of the safelist. Without it, the image could be pushed to a registry controlled by the attacker.

@crazy-max
Copy link
Member

@franky47 That's wrong each step in a job workflow is isolated in his own node process. Do you have a POC?

@franky47
Copy link
Author

franky47 commented Sep 2, 2020

Steps may be isolated, but environment variables can be shared between steps within a job. My article (linked above) has a detailed explanation of the "issue" (which is considered a feature by GitHub). Unless something changed since I wrote the article, it is possible to inject arguments from a previous action to control another.

@crazy-max
Copy link
Member

crazy-max commented Sep 2, 2020

@franky47 In your article you said process.env.INPUT_FOO === 'bar' will be expanded to the whole job. But it's not. Let me know when you've got time to create a POC about this. See also https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#env

@franky47
Copy link
Author

franky47 commented Sep 2, 2020

Indeed, it seems they fixed the issue, but partially. When invoking Node directly from a run clause, the environment is injectable, but not when running an action from a repository. See POC run for reference.

So this action is no longer vulnerable to injection, good to know. I will update my article accordingly.

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

No branches or pull requests

2 participants