Assessment reports>Initia>High findings>Incorrect signer check for shorthand accounts
Category: Coding Mistakes

Incorrect signer check for shorthand accounts

High Severity
Low Impact
Low Likelihood

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.

Remediation

Zellic © 2024Back to top ↑