slowfi - Attempting to Restore Suspended Transactions Always Reverts Due to Unset `invalidBridgedAmountsPool`

[sherlock-admin3]

slowfi

Medium

Attempting to Restore Suspended Transactions Always Reverts Due to Unset invalidBridgedAmountsPool

Summary

The current implementation lacks a setter function for invalidBridgedAmountsPool in BridgeStorage.layout, preventing the restoration of suspended transactions, which always reverts due to the unset state variable.

Vulnerability Detail

In the restoreBridgeTransaction function within BridgeFacetImpl.sol, an attempt is made to restore a suspended bridge transaction. However, the code assumes that invalidBridgedAmountsPool is set to a non-zero address, which is not possible because there is no setter function defined to initialize this state variable. Consequently, any attempt to restore a suspended transaction fails and reverts due to this uninitialized state variable.

Impact

The absence of a mechanism to set invalidBridgedAmountsPool prevents the successful restoration of suspended transactions. This limitation hinders the operational functionality of the restoreBridgeTransaction function, rendering it ineffective until invalidBridgedAmountsPool can be properly initialized.

Code Snippet

restoreBridgeTransaction

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;
}

Tool used

Manual Review

Recommendation

Implement a setter function for invalidBridgedAmountsPool to allow initialization during contract deployment or through a function accessible only to authorized roles. This setter function should enable the assignment of a valid address to invalidBridgedAmountsPool, ensuring the proper functionality of the restoreBridgeTransaction function.

Duplicate of #9

[sherlock-admin2]

The protocol team fixed this issue in the following PRs/commits: https://github.com/SYMM-IO/protocol-core/pull/46

[sherlock-admin2]

The Lead Senior Watson signed off on the fix.