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

[SECURITY] nethermind is logging the password used for personal_unlockAccount RPC call #4185

Closed
c0deright opened this issue Jun 22, 2022 · 4 comments · Fixed by #4206
Closed
Assignees

Comments

@c0deright
Copy link

Describe the bug
Nethermind logs all RPC requests and their arguments in its logfile. Nethermind does not obfuscate or censor the password argument when personal_unlockAccount is called.

This behaviour
a) is a potential security risk and
b) against best practices to not log secrets at all and
c) should be avoided

To Reproduce
Steps to reproduce the behavior:

  1. run nethermind
  2. create a new account
  3. unlock that account via RPC
  4. search nethermind log for the string personal_unlockAccount and see your password in plain text

Expected behavior
Logging arguments should be filtered to sanitize secrets in arguments for specific RPC calls

Logs

2022-06-22 09:25:56.3230|INFO|92|Executing JSON RPC call eth_blockNumber with params []
2022-06-22 09:26:08.7609|INFO|130|Executing JSON RPC call eth_getBalance with params [0x1234512345123451234512345123451234512345,latest]
2022-06-22 09:26:56.3685|INFO|95|Executing JSON RPC call personal_unlockAccount with params [0x1234512345123451234512345123451234512345,passwordinplaintext]
@LukaszRozmej
Copy link
Member

We are discouraging to use personal module in the first place. Should we delete some methods from it or all of it?

@gituser
Copy link

gituser commented Jun 24, 2022

@LukaszRozmej some ppl still using this methods to create accounts or send transactions, it's handy to be able to do that, even geth allows personal namespace and not deleting these methods.

There was a debate on this topic also back then on OpenEthereum (parity) and they decided not to delete these methods as they are still used: openethereum/parity-ethereum#9997

Of course for the best security you need to use 3rd party signing, but for testing something it's useful (e.g. testing sending on testnet).

@LukaszRozmej
Copy link
Member

LukaszRozmej commented Jun 24, 2022

Geth would like to drop support for it too: https://twitter.com/URozmej/status/1537374156584517632
https://geth.ethereum.org/docs/clef/tutorial

Nodes are not designed for this and shouldn't be.

@gituser
Copy link

gituser commented Jun 24, 2022

It's useful feature and I'm sure many are still using it who didn't want to invest into creating their own signing system.

And many nodes are staying inside internal networks not exposed anywhere.
So breaking this functionality for the end users isn't a good idea.

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