Skip to content

Conversation

chappjc
Copy link
Member

@chappjc chappjc commented Nov 17, 2021

The string address can have different formattings, so parse the input string into a common.Address and compare that to the account address. For example, it can be formatted with mixed case and with or without "0x", among other things.

Also use direct array comparison for address equality instead of bytes.Equal.

Also address some missing error checks.

The string address can have different formattings, so parse the input
string into a common.Address and compare that to the account address.

Also use direct array comparison for address equality instead of
bytes.Equal.
@chappjc chappjc force-pushed the ownsaddress-robustness branch from 4826b96 to fb45006 Compare November 17, 2021 20:59
@chappjc chappjc marked this pull request as ready for review November 17, 2021 21:00
Comment on lines +340 to +343
if !common.IsHexAddress(address) {
return false, errors.New("invalid address")
}
addr := common.HexToAddress(address)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

common.NewMixedCaseAddress would work fine for this too, but it has a little extra overhead we don't need

@chappjc chappjc modified the milestone: 0.4 Nov 22, 2021
@chappjc chappjc added the ETH label Nov 22, 2021
@chappjc
Copy link
Member Author

chappjc commented Nov 26, 2021

Thank you. I saw the same SignMessage receiver change in #1301 , which I have been reviewing today

@chappjc chappjc merged commit 3991a0a into decred:master Nov 26, 2021
@chappjc chappjc deleted the ownsaddress-robustness branch November 26, 2021 17:36
@chappjc chappjc added this to the 0.5 milestone Nov 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants