code-423n4 / 2023-05-maia-findings

24 stars 13 forks source link

uint32 is too small for `settlementNonce` #678

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/main/src/ulysses-omnichain/RootBridgeAgent.sol#L164

Vulnerability details

Impact

When a total of 4294967295 settlements(including non-bridging) have been made on a RootBridgeAgent, the contract becomes unusable, and users will not be able to even bridge their tokens back to a branch chain for withdrawal because it also requires settlement.

Proof of Concept

As we can see in LN 164 of BranchBridgeAgent.sol, settlementNonce has a type uint32. Once settlementNonce reaches max(uint32), subsequent calls that requires settlement such as bridging of tokens will fail due to integer overflow. It is no exaggeration that uint32 max value of 4294967295 will definitely be reached even though it may take a long period. What worsens this is that the contract is not upgradeable, and would require a redeployment to allow users to use the contract again. Given the difficulty in manually incrementing settlementNonce, the impact on the protocol when it happens(Settlement of users' funds won't be possible), and the difficulty of fixing the issue when it happens(contract is not upgradeable), I believe this qualifies as MEDIUM impact

NOTE that all BranchBridgeAgents from all chains can increase settlementNonce, so malicious users can perform many actions on chains where it would cost them nothing but gas(arbitrum), or chains with cheap transactions that would cause fallback gas to be low(polygon)

Tools Used

Manual Review

Recommended Mitigation Steps

settlementNonce should be of type uint256.

Assessed type

DoS

c4-judge commented 1 year ago

trust1995 marked the issue as unsatisfactory: Out of scope