THE EXECUTION OF THE GOVERNANCE ACTIONS (CONTINOUS TRANSACTIONS PACKED TOGETHER) ON GNOSIS CHAIN COULD `DoS`, IF A SINGLE MALICIOUS `target` CONTRACT REVERTS THE TRANSACTION

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2023-12-autonolas/blob/main/governance/contracts/bridges/HomeMediator.sol#L160-L163 https://github.com/code-423n4/2023-12-autonolas/blob/main/governance/contracts/bridges/HomeMediator.sol#L125-L168 https://github.com/code-423n4/2023-12-autonolas/blob/main/governance/contracts/bridges/FxGovernorTunnel.sol#L160-L163

Vulnerability details

Impact

In the HomeMediator.processMessageFromForeign function the data variable is passed into the function. The issue here is that set of continuous transactions can be packed into a single buffer and executed in the function.

The data variable is parsed inside the for loop and relavent parameters are extracted and each transaction is executed using the low-level call function as shown below:

        (bool success, ) = target.call{value: value}(payload); //@audit-info - execute hte transaction
        if (!success) { //@audit-issue - a single malicious target can revert the whole transaction
            revert TargetExecFailed(target, value, payload);
        } 

The problem here is that a transaction can be executed without the payload as well. Hence a single malicious target contract can revert the transaction by consuming all the gas inside its receive function.

If the payload is included then the transaction can be reverted inside the respective called function of the target contract. As a result the entire HomeMediator.processMessageFromForeign transaction will revert thus the subsequent transactions in the buffer will not be executed.

Similar issue is found in the FxGovernorTunnel.processMessageFromRoot function as well. It also executes low-level call function on the derived target address. If a single low-level call transaction fails due to a malicious target contract as explained above, all the transactions in the data buffer will revert.

Proof of Concept

            (bool success, ) = target.call{value: value}(payload);
            if (!success) {
                revert TargetExecFailed(target, value, payload);
            }

https://github.com/code-423n4/2023-12-autonolas/blob/main/governance/contracts/bridges/HomeMediator.sol#L160-L163

        for (uint256 i = 0; i < dataLength;) {
            address target;
            uint96 value;
            uint32 payloadLength;
            // solhint-disable-next-line no-inline-assembly
            assembly {
                // First 20 bytes is the address (160 bits)
                i := add(i, 20)
                target := mload(add(data, i))
                // Offset the data by 12 bytes of value (96 bits)
                i := add(i, 12)
                value := mload(add(data, i))
                // Offset the data by 4 bytes of payload length (32 bits)
                i := add(i, 4)
                payloadLength := mload(add(data, i))
            }

            // Check for the zero address
            if (target == address(0)) {
                revert ZeroAddress();
            }
            // Check for the value compared to the contract's balance
            if (value > address(this).balance) {
                revert InsufficientBalance(value, address(this).balance);
            }

            // Get the payload
            bytes memory payload = new bytes(payloadLength);
            for (uint256 j = 0; j < payloadLength; ++j) {
                payload[j] = data[i + j];
            }
            // Offset the data by the payload number of bytes
            i += payloadLength;

            // Call the target with the provided payload
            (bool success, ) = target.call{value: value}(payload);
            if (!success) {
                revert TargetExecFailed(target, value, payload);
            }
        }

        // Emit received message
        emit MessageReceived(governor, data);
    }

https://github.com/code-423n4/2023-12-autonolas/blob/main/governance/contracts/bridges/HomeMediator.sol#L125-L168

            (bool success, ) = target.call{value: value}(payload);
            if (!success) {
                revert TargetExecFailed(target, value, payload);
            }

https://github.com/code-423n4/2023-12-autonolas/blob/main/governance/contracts/bridges/FxGovernorTunnel.sol#L160-L163

Tools Used

Manual Review and VSCode

Recommended Mitigation Steps

Hence it is recommended to use a try-catch while calling the low-level call function on external contracts and use a fallback mechanism to control the transaction failure on a specific target contract. This way subsequent transactions can be executed even if a single transaction on a particular target contract fails.

Assessed type

DoS

[c4-pre-sort]

alex-ppg marked the issue as sufficient quality report

[c4-sponsor]

kupermind (sponsor) disputed

[c4-judge]

dmvt marked the issue as unsatisfactory: Invalid