code-423n4 / 2021-08-gravitybridge-findings

1 stars 0 forks source link

Freeze The Bridge Via Large ERC20 Names/Symbols/Denoms #5

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

nascent

Vulnerability details

Ethereum Oracles watch for events on the Gravity.sol contract on the Ethereum blockchain. This is performed in the check_for_events function, ran in the eth_oracle_main_loop.

In this function, there is the following code snippet:

    let erc20_deployed = web3
        .check_for_events(
            starting_block.clone(),
            Some(latest_block.clone()),
            vec![gravity_contract_address],
            vec![ERC20_DEPLOYED_EVENT_SIG],
        )
        .await;

This snippet leverages the web30 library to check for events from the starting_block to the latest_block. Inside the web30 library this nets out to calling:

    pub async fn eth_get_logs(&self, new_filter: NewFilter) -> Result<Vec<Log>, Web3Error> {
        self.jsonrpc_client
            .request_method(
                "eth_getLogs",
                vec![new_filter],
                self.timeout,
                Some(10_000_000),
            )
            .await
    }

The 10_000_000 specifies the maximum size of the return in bytes and returns an error if the return is larger:

        let res: Response<R> = match res.json().limit(limit).await {
            Ok(val) => val,
            Err(e) => return Err(Web3Error::BadResponse(format!("Web3 Error {}", e))),
        };

This can be triggered at will and keep the loop in a perpetual state of returning the GravityError::EthereumRestError(Web3Error::BadResponse( "Failed to get logs!".to_string())) error. To force the node into this state, you just have to deploy ERC20s generated by the public function in Gravity.sol:

    function deployERC20(
        string memory _cosmosDenom,
        string memory _name,
        string memory _symbol,
        uint8 _decimals
    ) public {
        // Deploy an ERC20 with entire supply granted to Gravity.sol
        CosmosERC20 erc20 = new CosmosERC20(address(this), _name, _symbol, _decimals);

        // Fire an event to let the Cosmos module know
        state_lastEventNonce = state_lastEventNonce.add(1);
        emit ERC20DeployedEvent(
            _cosmosDenom,
            address(erc20),
            _name,
            _symbol,
            _decimals,
            state_lastEventNonce
        );
    }

And specify a large string as the denom, name, or symbol. If an attacker uses the denom as the attack vector, they save significant gas costing just 256 per additional 32 bytes. For other cases, to avoid gas overhead, you can have the string be mostly 0s resulting in just 584 gas per additional 32 bytes. This leaves it feasible to surpass the 10mb response data in the 6 block buffer. This would throw every ethereum oracle into a state of perpetual errors and all would fall out of sync with the ethereum blockchain. This would result in the batches, logic calls, deposits, ERC20 creations, and valset updates to never receive attestations from other validators because their ethereum oracles would be down; the bridge would be frozen and remain frozen until the bug is fixed due to get_last_checked_block.

This will freeze the bridge by disallowing attestations to take place.

This requires a patch to reenable the bridge.

Recommendation

Handle the error more concretely and check if you got a byte limit error. If you did, chunk the search size into 2 and try again. Repeat as necessary, and combine the results.

Additionally, you could require that validators sign ERC20 creation requests.

jkilpatr commented 3 years ago

Excellent bug report.

I just ran into the buffer limit issue this morning with an Ethereum block. I agree handling this error correctly is essential to long term reliability.

albertchon commented 3 years ago

Nice :)

loudoguno commented 3 years ago

reopening as per judges assessment as "primary issue" on findings sheet