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

Console logs secure env value when parsing fails #1775

Closed
Synesso opened this issue Dec 21, 2013 · 20 comments
Closed

Console logs secure env value when parsing fails #1775

Synesso opened this issue Dec 21, 2013 · 20 comments

Comments

@Synesso
Copy link

Synesso commented Dec 21, 2013

Put an "unexpected value" character in a secure env variable and that variable will be logged at the console.

e.g. the encrypted value PASSWORD=winnie(the pooh) might become

secure: abcd1234..01

At build time this will log:

$ export PASSWORD=[secure]
/home/travis/build.sh: line 107: syntax error near unexpected token `('
/home/travis/build.sh: line 107: `export PASSWORD=winnie(the pooh)'
@sarahhodne
Copy link
Contributor

See also #825, #1145 and #1722.

I'm a bit torn on how to fix this. The obvious way seems to be to add a bit of escaping in the travis CLI, but if you look at #1722 that has issues as well.

If there is a way to do a syntax check on a bash script before running it, that may be the best thing to do, and just fail with an error if there is a syntax check.

The way to fix this, by the way, is to use travis encrypt 'PASSWORD="winnie(the pooh)"' instead.

@Synesso
Copy link
Author

Synesso commented Dec 28, 2013

Yes, given that the problem only occurs when the user has incorrectly
configured a secure env, an appropriate fix might just be to allow the
deletion of a build's log. The user fixes their config, deletes the old log
and afterwards no sensitive info remains on travis-ci for public view.

On 28 December 2013 16:04, Henrik Hodne notifications@github.com wrote:

See also #825 #825, #1145https://github.com/travis-ci/travis-ci/issues/1145and
#1722 #1722.

I'm a bit torn on how to fix this. The obvious way seems to be to add a
bit of escaping in the travis CLI, but if you look at #1722https://github.com/travis-ci/travis-ci/issues/1722that has issues as well.

If there is a way to do a syntax check on a bash script before running it,
that may be the best thing to do, and just fail with an error if there is a
syntax check.

The way to fix this, by the way, is to use travis encrypt
PASSWORD="winnie(the pooh)" instead.


Reply to this email directly or view it on GitHubhttps://github.com//issues/1775#issuecomment-31290579
.

@sarahhodne
Copy link
Contributor

Agreed, that would probably be a good workaround too. Right now, the correct way of getting a log cleared is to email support, but we have a “delete build” feature on the roadmap: #877.

@joshk
Copy link
Contributor

joshk commented Dec 28, 2013

I am a big fan of adding a 'delete log' feature as several requests to remove builds have been because of security issues with the logs. I would actually favour this feature over #877

@sarahhodne
Copy link
Contributor

I agree. Opened #1791 for that.

@BanzaiMan
Copy link
Contributor

Is there anything to do on this ticket?

@sarahhodne
Copy link
Contributor

I think we are still considering adding "filtering" of some sort, removing the values of encrypted environment variables from the logs if they are accidentally printed. There's also the question of automatically escaping the variables in travis.rb when adding them. Both of those might be separate tickets, though.

@joshk
Copy link
Contributor

joshk commented Jul 21, 2014

Personally I'm not in favour of us filtering the logs as the concept of log parts makes filtering hard and complicated.

The 'remove log' feature should help in cases where secure vars are exported in clear text when they shouldn't.

@rkh
Copy link
Contributor

rkh commented Jul 22, 2014

In theory, filtering logs would allow an attack, if the attacker could
inject strings into the logs.

On Tue, Jul 22, 2014 at 12:32 AM, Josh Kalderimis notifications@github.com
wrote:

Personally I'm not in favour of us filtering the logs as the concept of
log parts makes filtering hard and complicated.

The 'remove log' feature should help in cases where secure vars are
exported in clear text when they shouldn't.


Reply to this email directly or view it on GitHub
#1775 (comment)
.

@sarahhodne
Copy link
Contributor

@rkh I thought that at first, too, but then @drogus pointed out that if they have access to inject strings to the log, they also have access to the environment variables and can just print them instead.

@rkh
Copy link
Contributor

rkh commented Jul 22, 2014

Not necessarily, the log could be output from a curl command, for instance.

@bguerin
Copy link

bguerin commented Feb 20, 2015

@henrikhodne

If there is a way to do a syntax check on a bash script before running it

bash -n

@camelspeed
Copy link

camelspeed commented Jun 21, 2016

@henrikhodne @BanzaiMan Any progress on this issue? I just had my hosting environment access token exposed due to a malformed character in the string. I purged my log but it is still concerning that secrets are exposed (and this issue is 18mos old and still unresolved) even though the information entered the system in a protected state. Thank you guys.

@octocat-mona
Copy link

I configured a (apparently malformed) secure variable via the settings in Travis, why can't that be checked before it is saved?

@langri-sha
Copy link

Hello everyone! Would it not be possible to export these values before the build begins and re-export them for verbosity and clerical purposes?

I am happy with any way you manage to fix this, as this seems to be a critical security error, because it's easy to use robots to divulge logs that have partially leaked a secret in public.

@BanzaiMan
Copy link
Contributor

I've opened the PR to at least partially address this issue. I welcome comments there. Thank you!

@BanzaiMan
Copy link
Contributor

I'm closing this now. We now send export's STDOUT to /dev/null, so that parts are not leaked to the logs, and show link for further information on Bash special characters if the command fails.

If you see a similar issue going forward, please open a new issue.

Thank you!

@BanzaiMan
Copy link
Contributor

The PR was reverted.

@BanzaiMan BanzaiMan reopened this Jan 24, 2017
@Swyter
Copy link

Swyter commented Jan 29, 2017

The fix is easy, just escape them as literal strings like everyone else. That's it.

Spent the entire afternoon scratching my head before figuring out why my URLs weren't working.
That I had to base64-encode them to use them in scripts is just ridiculous.

@BanzaiMan
Copy link
Contributor

travis-ci/travis-build#942 is live. Accidental leakage is far less likely now.

I'm closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants