Skip to content

jsre: leave out lines from history possibly containing passwords#1562

Merged
obscuren merged 1 commit intoethereum:developfrom
ethersphere:blankpasswd
Aug 4, 2015
Merged

jsre: leave out lines from history possibly containing passwords#1562
obscuren merged 1 commit intoethereum:developfrom
ethersphere:blankpasswd

Conversation

@zelig
Copy link
Copy Markdown
Contributor

@zelig zelig commented Jul 31, 2015

Currently the js console keeps a history. If you use methods of the personal module, and give your password as an argument, it ends up in the history file in cleartext.

The situation is worse, since the js console currently does not even allow interactive password input, so supplying it as argument is the only option.

Ideally blanking out password arguments should result in history entries like: personal.newAccount(XXXXX), however finding the right argument is very hard, a line could look like this:

personal.unlockAccount(complex(method(arg0, arg1, arg2)), "password"))

potentially several calls within complex expression.

Due to lack of a better solution, I blank the entire line if it contains a personal command just to be on the safe side.

@robotally
Copy link
Copy Markdown

Vote Count Reviewers
👍 2 @fjl @obscuren
👎 1 @bas-vk

Updated: Mon Aug 3 14:33:09 UTC 2015

@obscuren
Copy link
Copy Markdown
Contributor

This PR is adequate 👍

@bas-vk
Copy link
Copy Markdown
Member

bas-vk commented Jul 31, 2015

👎

something like: personal. unlockAccount(...., ....) will still end up in the history. Ik would recommend trimming any spaces before the regex check.

@obscuren
Copy link
Copy Markdown
Contributor

@bas-vk this is a hot fix, a quick and dirty solution for a silly problem. It's not supposed to be a all catching, ever lasting solution :-)

@bas-vk
Copy link
Copy Markdown
Member

bas-vk commented Jul 31, 2015

I absolutely agree on that, the real solution would be not to pass any sensitive information through arguments. But a little bit of normalization of the input can catch basic typo's.

@zelig
Copy link
Copy Markdown
Contributor Author

zelig commented Jul 31, 2015

@bas-vk is right, i will add optional whitespace
also we should purge all these lines from the existing history and rewrite on startup ok?

typos like peronal.newAccount("password") and things like

p = "password"
personal.unlockAccount(primary, p)

will still give it away, but these are really the user's fault, cannot to anything

@fjl
Copy link
Copy Markdown
Contributor

fjl commented Aug 3, 2015

👍

@fjl fjl added this to the 1.0.1 milestone Aug 4, 2015
obscuren added a commit that referenced this pull request Aug 4, 2015
jsre: leave out lines from history possibly containing passwords
@obscuren obscuren merged commit ff66e8f into ethereum:develop Aug 4, 2015
@zelig zelig self-assigned this Aug 11, 2015
@zelig zelig deleted the blankpasswd branch December 8, 2015 20:49
Rampex1 pushed a commit to streamingfast/go-ethereum that referenced this pull request Jun 6, 2025
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.

5 participants