Lines of code

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L454

Vulnerability details

Impact

If users attempt to bridge ERC777s from Cosmos to Ethereum, it is highly likely that the transfers will revert. The issue lies in the combination of using safeTransfer() and the idea of batching. safeTransfer() will revert for any failed transfer meaning that the all users within a batch will not receive their bridged tokens. This is especially probable for ERC777 token transfers as they contain hooks that ensure that the receiving contract has successfully implemented a special tokensReceived() function.

Therefore, if even a single malicious OR unaware contract within a batch has not implemented the tokensReceived() function or intentionally throws an exception, the entire userbase within the batch call will not receive any funds. Without knowing Cudos's procedure for handling failed transfers, I cannot deduce that the funds would be lost to the users. Instead, the likely scenario is that the users' funds would be frozed for an indefinite amount of time until the bad actor contract has been identified. Cudos would need to send each transaction individually to ensure that each recipient actually receives the bridged funds.

Extra info

ERC777s are backwards compatible with ERC20s, and aim to solve the issue of lost tokens by supplying send and receive hooks prior to and after transfer. The ERC777.transfer() function eventually calls the internal _callTokensReceived() function:

    function _callTokensReceived(
        address operator,
        address from,
        address to,
        uint256 amount,
        bytes memory userData,
        bytes memory operatorData,
        bool requireReceptionAck
    ) private {
        address implementer = _ERC1820_REGISTRY.getInterfaceImplementer(to, _TOKENS_RECIPIENT_INTERFACE_HASH);
        if (implementer != address(0)) {
            IERC777Recipient(implementer).tokensReceived(operator, from, to, amount, userData, operatorData);
        } else if (requireReceptionAck) {
            require(!to.isContract(), "ERC777: token recipient contract has no implementer for ERC777TokensRecipient");
        }
    }

If the recipient is an EOA, no further action is necessary. If the recipient is a contract, it is required that the receiving contract implements tokensReceived(), else the call will revert. Since this is an external call to the ERC777 contract, the call will return a value of false, causing safeTransfer() to revert the batch call.

Tools Used

Manual review

Recommended Mitigation Steps

The combination of batching and safeTransfer() has a high likelihood of reverting. Instead of requiring that every single transaction was successful, use the transfer() function and capture the returned success bool. On failure, Cudos can emit an event capturing failed transactions. Upon a later time, they can retry these failed transactions if desired.

Since the entire call stack will not be reverted, the successful transfers will be completed.

code-423n4 / 2022-05-cudos-findings