fix(sec-bug): handle no-login mode extensively #2615
Conversation
Signed-off-by: Onur Özkan <[email protected]>
Signed-off-by: Onur Özkan <[email protected]>
Signed-off-by: Onur Özkan <[email protected]>
Signed-off-by: Onur Özkan <[email protected]>
onur-ozkan
added
improvement: security
Signed-off-by: Onur Özkan <[email protected]>
Signed-off-by: Onur Özkan <[email protected]>
onur-ozkan
force-pushed
the
handle-no-login-mode
branch
from
1d15b89 to
bf596dc
Compare
Signed-off-by: Onur Özkan <[email protected]>
|
p.s. some tests in |
Signed-off-by: Onur Özkan <[email protected]>
shamardy
left a comment
shamardy
left a comment
There was a problem hiding this comment.
My review according to this #2615 (comment)
| pub async fn lp_coininit(ctx: &MmArc, ticker: &str, req: &Json) -> Result<MmCoinEnum, String> { | ||
| if ctx.is_no_login_mode() { | ||
| return ERR!("Cannot enable coins in no-login mode."); | ||
| } |
There was a problem hiding this comment.
do we need something similar in v2 activation methods?
| if ctx.is_no_login_mode() { | ||
| return MmError::err(EnablePlatformCoinWithTokensError::CannotEnableInNoLoginMode); | ||
| } |
There was a problem hiding this comment.
I see that you did add the check for some of the v2 activation methods, but I think that we need it in the utxo v2 activation methods as well.
There was a problem hiding this comment.
Isn't this a main gateway for all the v2 activations?
There was a problem hiding this comment.
Isn't this a main gateway for all the v2 activations?
Apparently it's not, what the hell..? They live under the same module but not implement the activation traits...
There was a problem hiding this comment.
Any activation or private-key related logic that isn't explicitly handled will eventually be covered by this check so we don't need to add ifs all over the codebase.
Signed-off-by: Onur Özkan <[email protected]>
There is a potential risk that if users make a typo in the
passphrasefield of the KDF config and then run theget_private_keysRPC, the returned private key will be incorrect because an empty string will be used as the passphrase. A typo inpassphrasemeans the field is not defined, which causes the KDF to assume the user wants to operate in no-login mode (this is exactly what happens in #2458).This PR disables coin activation and
get_private_keysin no-login mode to prevent any risk on no-login mode, so users do not risk their funds and private keys due to a simple typo.Resolves: #2458