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

0 stars 0 forks source link

0xAadi - `restoreBridgeTransaction` Function in `BridgeFacet` Contract will update accounting states even if the accounting is paused #4

Closed sherlock-admin3 closed 1 week ago

sherlock-admin3 commented 2 weeks ago

0xAadi

Medium

restoreBridgeTransaction Function in BridgeFacet Contract will update accounting states even if the accounting is paused

Summary

The restoreBridgeTransaction function in the BridgeFacet contract is intended to restore a previously suspended bridge transaction and update the valid transaction amount. However, this function does not include the whenNotAccountingPaused modifier, which is used to prevent users from changing balances. This poses a potential inconsistency, as the restoreBridgeTransaction function in the BridgeFacetImpl library updates the balance without this check.

Vulnerability Detail

The restoreBridgeTransaction function in the BridgeFacet contract does not include the whenNotAccountingPaused modifier, which is used to prevent balance changes when the accounting is paused. This can lead to an inconsistency in the contract state if the accounting is paused while a DISPUTE_ROLE user attempts to restore a bridge transaction and update the valid transaction 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;
    }

Impact

The missing whenNotAccountingPaused modifier in the restoreBridgeTransaction function allows DISPUTE_ROLE users to update balances even when the accounting is paused. This can result in incorrect balance updates and potential inconsistencies in the contract state.

Code Snippet

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

Tool used

Manual Review

Recommendation

Include the whenNotAccountingPaused modifier in the restoreBridgeTransaction function in the BridgeFacet contract to ensure that balance updates are only allowed when the accounting is not paused. This will help maintain consistency in the contract state and prevent potential issues related to incorrect balance updates.