Assessment reportsPublic findings
Back to Zellic site
↗
Assessment reports>Initia>High findings>Query gas limit not enforced through bank module
GeneralOverview
Findings
Critical (3)
High (12)Missing gas charge on memo in native_nft_transferNext zapping ID potentially duplicatedPublished module names do not necessarily match binary moduleStablepools can be created with one or no assetsStablepool swap can be called, repeating the same assetFrozen module coin store can cause chain haltMalicious proposer can skip fee checkMove coins can be burned twiceQuery gas limit not enforced through bank moduleIncorrect signer check for shorthand accountsValidator set updates can skip current validatorsWithdrawal hash clash using variable-length fields
Medium (4)
Low (9)
Informational (3)
DiscussionGeneral DiscussionImprove AMM testsBridge executor minting powerCharge before using valuesEmpty allowed_publishers array allows every address to publishERC20Keeper with custom contractsSuggestions for additional security checksTable handle overflow leads to abort
Threat ModelWhat are threat models?stdlib
Audit ResultsSummary
AppendixProof of concept: Reference safety verifier bypass
Category: Coding Mistakes

Query gas limit not enforced through bank module

High Severity
High Impact
High Likelihood

Description

When performing calls to the EVM, a ContractQueryGasLimit is used to limit the amount of gas that can be used to prevent users from DOSing a node with an infinite loop in an EVM contract:

// Call implements types.QueryServer.
func (qs *queryServerImpl) Call(ctx context.Context, req *types.QueryCallRequest) (res *types.QueryCallResponse, err error) {
    defer func() {
        if r := recover(); r != nil {
            err = errorsmod.Wrap(types.ErrEVMCallFailed, fmt.Sprintf("vm panic: %v", r))
        }
    }()

    sdkCtx := sdk.UnwrapSDKContext(ctx)
    sdkCtx = sdkCtx.WithGasMeter(storetypes.NewGasMeter(qs.config.ContractQueryGasLimit))

The issue is that this limit is not enforced when EVMStaticCall is called directly, such as when the bank module checks the balance of a user:

func (k ERC20Keeper) balanceOf(ctx context.Context, addr, contractAddr common.Address) (math.Int, error) {
    inputBz, err := k.ERC20ABI.Pack("balanceOf", addr)
    if err != nil {
        return math.ZeroInt(), types.ErrFailedToPackABI.Wrap(err.Error())
    }

    retBz, err := k.EVMStaticCall(ctx, types.NullAddress, contractAddr, inputBz)
    if err != nil {
        return math.ZeroInt(), err
    }

    res, err := k.ERC20ABI.Unpack("balanceOf", retBz)
    if err != nil {
        return math.ZeroInt(), types.ErrFailedToUnpackABI.Wrap(err.Error())
    }

    balance, ok := res[0].(*big.Int)
    if !ok {
        return math.ZeroInt(), types.ErrFailedToDecodeOutput
    }

    return math.NewIntFromBigInt(balance), nil
}

Impact

A malicious user can create an ERC-20 contract with an infinite loop in the balanceOf function, mint some coins to themselves, and then trigger a call to balanceOf with minitiad query bank balances to cause the node to enter into an infinite loop.

Recommendations

The ContractQueryGasLimit should always be enforced, or the default Cosmos SDK setting query-gas-limit should be used to limit the amount of gas that can be used for queries.

Remediation

Zellic © 2024Back to top ↑