sherlock-audit / 2024-06-symmetrical-update-2-judging

0 stars 0 forks source link

0xAadi - Lack of Handling for `validAmount` Greater Than `bridgeTransaction.amount` in `restoreBridgeTransaction` Function in `BridgeFacetImpl` Library #2

Closed sherlock-admin2 closed 1 week ago

sherlock-admin2 commented 2 weeks ago

0xAadi

Medium

Lack of Handling for validAmount Greater Than bridgeTransaction.amount in restoreBridgeTransaction Function in BridgeFacetImpl Library

Summary

The suspendBridgeTransaction() function in the BridgeFacet contract is designed to suspend a specific bridge transaction. A user with the DISPUTE_ROLE can restore the previously suspended bridge transaction and update the valid transaction amount through the restoreBridgeTransaction function by calling restoreBridgeTransaction from the BridgeFacetImpl library.

The issue is that the restoreBridgeTransaction function in the BridgeFacetImpl library does not handle situations where the supplied validAmount is greater than the previously set bridgeTransaction.amount.

According to the README of the audit:

Are there any limitations on values set by admins (or other roles) in the codebase, including restrictions on array lengths?

No

There is no limitation on the values that can be used in the restoreBridgeTransaction function. Therefore, this function is expected to accept an updated amount that is either less than or greater than the previously set amount.

Vulnerability Detail

The restoreBridgeTransaction function in the BridgeFacetImpl library does not handle cases where validAmount is greater than bridgeTransaction.amount.

    function restoreBridgeTransaction(uint256 transactionId, uint256 validAmount) internal {
        BridgeStorage.Layout storage bridgeLayout = BridgeStorage.layout();
        BridgeTransaction storage bridgeTransaction = bridgeLayout.bridgeTransactions[transactionId];

        require(bridgeTransaction.status == BridgeTransactionStatus.SUSPENDED, "BridgeFacet: Invalid status");
        require(bridgeLayout.invalidBridgedAmountsPool != address(0), "BridgeFacet: Zero address");

@>>     AccountStorage.layout().balances[bridgeLayout.invalidBridgedAmountsPool] += (bridgeTransaction.amount - validAmount);
        bridgeTransaction.status = BridgeTransactionStatus.RECEIVED;
        bridgeTransaction.amount = validAmount;
    }

This scenario results in a revert due to underflow caused by the operation bridgeTransaction.amount - validAmount. The function should allow DISPUTE_ROLE users to increase bridgeTransaction.amount by using a validAmount larger than bridgeTransaction.amount.

Code Snippet

https://github.com/sherlock-audit/2024-06-symmetrical-update-2/blob/f5b76ca33f5f05b927a9c0f2f57938e919d6420b/protocol-core/contracts/facets/Bridge/BridgeFacetImpl.sol#L90

Impact

The implementation restricts DISPUTE_ROLE users from increasing bridgeTransaction.amount by using a validAmount larger than bridgeTransaction.amount.

Tool used

Manual Review

Recommendation

Update the restoreBridgeTransaction() function to handle validAmounts greater than bridgeTransaction.amount, allowing DISPUTE_ROLE users to increase bridgeTransaction.amount.