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

Potential credentials leak when setting environmentVariables #19

Closed
britter opened this issue Feb 15, 2021 · 6 comments · Fixed by #20
Closed

Potential credentials leak when setting environmentVariables #19

britter opened this issue Feb 15, 2021 · 6 comments · Fixed by #20

Comments

@britter
Copy link
Contributor

britter commented Feb 15, 2021

Problem

Currently if environmentVariables is set, this will cause the whole environment from the Gradle process to be inherited to the vagrant command. This is then printed on info level to the console log.

Context

A common pattern for injecting credentials into the build process on build servers is to set them as environment variables on the build tool process. This way e.g. the credentials for deploying to maven central can be passed the Gradle tasks that need them.
So when my build executes a Vagrant task that has some environment variables set and also has reads some credentials from the environment, this may leak the credentials to build scans and the build server log.

Options

  • Remove envp from console output.
  • log on debug level - this will prevent the output to show up in build scans. It will not prevent the output to end up in build server logs, if Gradle is configured to log on debug level.
  • Add a verbose configuration flag to control whether or not the command and environment should be printed to the log. It should default to false.
@britter
Copy link
Contributor Author

britter commented Feb 15, 2021

Another thing about the environmentVariables setting that I find confusing is that it only adds System.getenv() if any environment variable was set on the task. If the user has not supplied any environment variables, and empty map will be used:

getEnvironmentVariables().size() > 0 ? OsUtils.prepareEnvVars(getEnvironmentVariables()) : null

@bmuschko
Copy link
Owner

I agree with your analysis. We should probably remove envp from the console output.

Another thing about the environmentVariables setting that I find confusing is that it only adds System.getenv() if any environment variable was set on the task.

Let's fix that as well.

britter added a commit to britter/gradle-vagrant-plugin that referenced this issue Feb 16, 2021
@JLLeitschuh
Copy link

Hey,

This should get a CVE number assigned to it. Can you open a Github security advisory against the repo. Happy to assist with disclosure.

@bmuschko
Copy link
Owner

@JLLeitschuh Can you provide a link that explains the process?

@JLLeitschuh
Copy link

Hey @bmuschko,

Here's the link to the place you can create an advisory against your repository.

Feel free to add me if you'd like assistance here. I have a lot of experience in this area.

https://github.com/bmuschko/gradle-vagrant-plugin/security/advisories

Here's the documentation about GitHub Security Advisories.
https://docs.github.com/en/github/managing-security-vulnerabilities/about-github-security-advisories

@JLLeitschuh
Copy link

JLLeitschuh commented Mar 8, 2021

This has been assigned CVE-2021-21361
GHSA-jpcm-4485-69p7

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 a pull request may close this issue.

3 participants