Users can create assertion with stale `baseStake` amount

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-05-arbitrum-foundation/blob/6f861c85b281a29f04daacfe17a2099d7dad5f8f/src/assertionStakingPool/AssertionStakingPool.sol#L40-L46 https://github.com/code-423n4/2024-05-arbitrum-foundation/blob/6f861c85b281a29f04daacfe17a2099d7dad5f8f/src/rollup/RollupUserLogic.sol#L177-L182 https://github.com/code-423n4/2024-05-arbitrum-foundation/blob/6f861c85b281a29f04daacfe17a2099d7dad5f8f/src/rollup/RollupCore.sol#L492 https://github.com/code-423n4/2024-05-arbitrum-foundation/blob/6f861c85b281a29f04daacfe17a2099d7dad5f8f/src/rollup/RollupCore.sol#L69-L70 https://github.com/code-423n4/2024-05-arbitrum-foundation/blob/6f861c85b281a29f04daacfe17a2099d7dad5f8f/src/rollup/RollupAdminLogic.sol#L223-L226

Vulnerability details

Impact

This breaks core protocol functionality because it lead to a situation where assertions are created by a user whose stake is below the current baseStake amount.

Proof of Concept

Users can create assertion once required stake amount is reached by calling AssertionStakingPool::createAssertion(...) . The call trace is as shown below

  AssertionStakingPool::createAssertion(...)
   ->RollupUserLogic::newStakeOnNewAssertion(...)
@> -->RollupUserLogic::stakeOnNewAssertion(...)
   ---> RollupCore::createNewAssertion(...)

New assertions build on the configData of their parent assertion and as such the RollupUser(rollup)::stakeOnNewAssertion(...) implements a check to ensure that the user creating a new assertion is sufficiently staked with respect to the baseStake value at the time the parent assertion was created as seen on L182 below

File: RollupUserLogic.sol
163:     function stakeOnNewAssertion(AssertionInputs calldata assertion, bytes32 expectedAssertionHash)
164:         public
165:         onlyValidator
166:         whenNotPaused
167:     {
...
175:         require(isStaked(msg.sender), "NOT_STAKED");
176: 
177:         // requiredStake is user supplied, will be verified against configHash later
178:         // the prev's requiredStake is used to make sure all children have the same stake
...
182:@>       require(amountStaked(msg.sender) >= assertion.beforeStateData.configData.requiredStake, "INSUFFICIENT_STAKE");
183: 

However the problem lies in the validation on L182 in the RollupUser(rollup)::stakeOnNewAssertion(...) function, because the value of baseStake can be updated by the rollup owner, if the baseStake value is increased before this new assertion is created and the current amountStaked(msg.sender) is less than this new baseStake amount, a user can still successfully create an assertion. This breaks the core protocol functionality because despite raising the bar for honesty in the system users are still able to create assertions with less than the current base stake required by the system.

Also, when the new assertion is finally created the config is set to the new baseStake as shown below (on L492) but this does not the assertion user from creating the assertion with less than required amount successfully

File: RollupCore.sol
487:         // state updates
488:         AssertionNode memory newAssertion = AssertionNodeLib.createAssertion(
489:             prevAssertion.firstChildBlock == 0, // assumes block 0 is impossible
490:             RollupLib.configHash({
491:                 wasmModuleRoot: wasmModuleRoot,
492: @>              requiredStake: baseStake,
493:                 challengeManager: address(challengeManager),
494:                 confirmPeriodBlocks: confirmPeriodBlocks,
495:                 nextInboxPosition: uint64(nextInboxPosition)
496:             })
497:         );

POC Summary

• Assume current baseStake is 100ETH
• Alice creates assertion: batch 5, blockhash 0xabc with configData.requiredStake = 100ETH
• Admin calls RollupAdminLogic::setBaseStake(...) to increase baseStake to 120ETH
• current baseStake = 120ETh
• Bob whose amountStaked(msg.sender) = 101ETH creates assertion: batch 10, blockhash 0x123 but his stake is checked to be greater than 100ETH instead of 120ETH

  182:@>       require(amountStaked(msg.sender) >= assertion.beforeStateData.configData.requiredStake, "INSUFFICIENT_STAKE");

• Bobs assertion is successfully created

As you can see this, although asset are not at direct risk, but the functionality of the protocol is impacted allowing users to create assertions with less than the minimum amount staked required by the system (and this could be a hypothetical attack path because assertions are created using stale baseStake value)

Tools Used

Manual review

Recommended Mitigation Steps

Modify the RollupUserLogic::stakeOnNewAssertion(...) as shown below

File: RollupUserLogic.sol
163:     function stakeOnNewAssertion(AssertionInputs calldata assertion, bytes32 expectedAssertionHash)
164:         public
165:         onlyValidator
166:         whenNotPaused
167:     {
...

...
182:   -     require(amountStaked(msg.sender) >= assertion.beforeStateData.configData.requiredStake, "INSUFFICIENT_STAKE");
182:   +     require(amountStaked(msg.sender) >= baseStake, "INSUFFICIENT_STAKE");
183: 

Assessed type

Invalid Validation

[c4-sponsor]

gzeoneth (sponsor) disputed

[gzeoneth]

Expected behavior to use the stake amount stored in the prev.

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid