crypto: LedgerShowAddress's public key comparison equality == instead of .Equals will soon result in unexpected results if we upgrade modules of tendermint/crypto

[odeke-em]

During an audit, I noticed that the code for LedgerShowAddress compares PubKeySecp256k1 values by equality i.e.

cosmos-sdk/crypto/ledger_secp256k1.go

Lines 118 to 120 in 6ceedc9

	if pubKey != expectedPubKey {
	return fmt.Errorf("the key's pubkey does not match with the one retrieved from Ledger. Check that the HD path and device are the correct ones")
	}

and given that we are at go module github.com/tendermint/[email protected], the type of PubKeySecp256k1 is a BYTE ARRAY aka [SIZE]byte.

getPubKeyUnsafe returns an interface so that'll mask the problem until runtime where it'll panic.

Byte arrays have equality defined on them, but as soon as we upgrade to a new module, that type turns into a byte slice that'll panic, but will fail e.g.
https://play.golang.org/p/sZdl964nL8b

Recommendation

Both versions have a defined .Equals method, we should use that and it'll guard against that sensitivity for any interfaces

• [email protected]
• [email protected]

[JulianToledano]

Hello 👋 @odeke-em
do you think we can close this? as right now PubKey.Equals() is being used?

cosmos-sdk/crypto/ledger/ledger_secp256k1.go

Lines 158 to 170 in 1bd9cbe

	if !pubKey.Equals(expectedPubKey) {
	return fmt.Errorf("the key's pubkey does not match with the one retrieved from Ledger. Check that the HD path and device are the correct ones")
	}
	pubKey2, _, err := getPubKeyAddrSafe(device, path, accountAddressPrefix)
	if err != nil {
	return err
	}
	if !pubKey2.Equals(expectedPubKey) {
	return fmt.Errorf("the key's pubkey does not match with the one retrieved from Ledger. Check that the HD path and device are the correct ones")
	}