Impact: Critical
Likelihood: Very low
Overall: Medium
The problem comes from using uint32 for settlement/deposit nonces, which is being incremented on every cross-chain transaction
Impact
The probability is low, because there should be more than 2^32 settlements created and so this may happen after more than 10 years, but if this condition is reached, the whole system will DoS, because communication from RootBridgeAgent to other chains will always revert with "Overflow exception". The same condition is valid for Branch deposit nonces, but as the root is the same for all branches, we consider that this condition would hit there first. In the mapping where nonce points to a settlement/deposit struct the key is set to be a uint256 mapping(uint256 chainId => mapping(uint256 nonce => uint256 state)) public executionState;. The same type should be used for the nonce declaration - uint32 public settlementNonce
Proof of Concept
As the impact and trivial and to reproduce we should use a for loop executing 4294967295 transactions I haven't provided written PoC test.
Tools Used
Manual review
Recommended Mitigation Steps
Change nonces type from uint32 to uint256, or at least uint128
Lines of code
https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/RootBridgeAgent.sol#L75 https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/RootBridgeAgent.sol#L168
Vulnerability details
Summery
Impact: Critical Likelihood: Very low Overall: Medium The problem comes from using uint32 for settlement/deposit nonces, which is being incremented on every cross-chain transaction
Impact
The probability is low, because there should be more than 2^32 settlements created and so this may happen after more than 10 years, but if this condition is reached, the whole system will DoS, because communication from RootBridgeAgent to other chains will always revert with "Overflow exception". The same condition is valid for Branch deposit nonces, but as the root is the same for all branches, we consider that this condition would hit there first. In the mapping where nonce points to a settlement/deposit struct the key is set to be a uint256
mapping(uint256 chainId => mapping(uint256 nonce => uint256 state)) public executionState;
. The same type should be used for the nonce declaration -uint32 public settlementNonce
Proof of Concept
As the impact and trivial and to reproduce we should use a for loop executing 4294967295 transactions I haven't provided written PoC test.
Tools Used
Manual review
Recommended Mitigation Steps
Change nonces type from uint32 to uint256, or at least uint128
Assessed type
DoS