search

trezor / trezor-crypto

:lock: Don't use this repo, use the new monorepo instead:
https://github.com/trezor/trezor-firmware
MIT License
501 stars 201 forks source link

secret sanitization #195

Closed keepkeyjon closed 5 years ago

keepkeyjon commented 5 years ago

mnemonic_from_data()s interface prevents cleaning up secrets in RAM after it has done the conversion, since it returns a const char * into a static buffer.

https://github.com/trezor/trezor-crypto/blob/c34e8ab3bd50243b78331e2aecf1aa169003ca28/bip39.c#L73

https://github.com/trezor/trezor-crypto/blob/c34e8ab3bd50243b78331e2aecf1aa169003ca28/bip39.c#L88

While it does mark the memory CONFIDENTIAL, it would be better if the caller could zero it at its earliest convenience. A couple of options I see:

  1. Make it take in working buffer, so the caller can clear it itself.
  2. Make it return a char * so the contents can get memzero(foo, strlen(foo))-ed as necessary.
  3. If data argument is null, memzero the buffer.
  4. Cast away the const (gross).

Which is preferable to you? I'm happy to submit PR(s).

prusnak commented 5 years ago

Addressed in https://github.com/trezor/trezor-crypto/commit/d1c52401e4c76c74a10455682ace0655b7aa644c

Thanks