Incorrect signer check for shorthand accounts
Description
When executing a cosmos message from the Move VM, the signers of the message are checked to ensure that they match the caller or that the shorthand account's original address matches the caller:
for _, signer := range signers {
if bytes.Equal(caller.Address().Bytes(), signer) {
continue
}
// if signer is different from the caller, check if the signer is a shorthand account.
// and then check shorthand account's original address is the same with the caller.
if len(signer) != common.AddressLength {
signerAccount := e.ak.GetAccount(ctx, signer)
if shorthandCallerAccount, ok := signerAccount.(types.ShorthandAccountI); ok {
addr, err := shorthandCallerAccount.GetOriginalAddress(e.ac)
if err != nil {
return nil, ctx.GasMeter().GasConsumedToLimit(), types.ErrPrecompileFailed.Wrap(err.Error())
}
if bytes.Equal(addr.Bytes(), signer) {
continue
}
}
}
The issue is that a shorthand account's original address addr
is compared against the signer instead of the caller, defeating the purpose of the check.
Luckily this branch cannot be reached, as shorthand accounts will always be stored with an address of length common.AddressLength
, preventing the if block from being entered.
Impact
If a shorthand account was stored with an address length not equal to common.AddressLength
, it would allow anyone to send cosmos messages on behalf of the account.
Recommendations
The shorthand account of the caller should be fetched, and its original address should be compared against the signer of the message.