Cosmos-sdk: Update IBC messages to validate Signer string

[colin-axner]

Summary

While auditing IBC we noticed that the signer in a message is validated using:

 if msg.Signer == "" {
        return sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "signer address cannot be empty")
    }   

Should this be?

 if strings.TrimSpace(msg.Signer) == "" {
        return sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "signer address cannot be empty")
    }   

I assume this would need to be changed for all SDK messages, but I have not checked.

---

For Admin Use

• [ ] Not duplicate issue
• [ ] Appropriate labels applied
• [ ] Appropriate contributors tagged
• [ ] Contributor assigned/self-assigned

[colin-axner]

Feel free to close if this is a not an issue

cc/ @anilCSE @amaurymartiny @blushi @atheeshp

[anilCSE]

Basic validation is now using AccAddressFromBech32 directly to validate account address (https://github.com/cosmos/cosmos-sdk/blob/master/types/address.go#L126-L128, https://github.com/cosmos/cosmos-sdk/blob/master/types/address.go#L137). So may be we should update all IBC signer validations with something like:

_, err := sdk.AccAddressFromBech32(msg.Signer)
if err != nil {
    return err
}

[colin-axner]

@anilCSE sounds good, thanks for the reply. Will update the issue

[fedekunze]

not sure if the proposed solution applies to ibc transfers though? as the sender could be from a chain with a different prefix. This is why we changed the address to string in that package in the first place. cc: @cwgoes

[cwgoes]

We should not validate the address in a way which will fail if the account prefix is different, that is intentional. I think checking that the address isn't an empty string is fine, but we probably shouldn't validate any more since connected chains may use totally different formats (e.g. they may not even be using the Cosmos SDK).

[fedekunze]

I think adding the address verification for the IBC core messages is fine

[cwgoes]

Ah yes, that's true, sorry, I was answering in reference to specifically ICS 20.