code-423n4 / 2024-05-arbitrum-foundation-validation

0 stars 0 forks source link

The requiredStake on stakeOnNewAssertion is directed by the previous assertion and not a base value #345

Open c4-bot-2 opened 4 months ago

c4-bot-2 commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-05-arbitrum-foundation/blob/6f861c85b281a29f04daacfe17a2099d7dad5f8f/src/rollup/RollupCore.sol#L371-L373

Vulnerability details

Impact

Stake price when creating an assertion is not uniformed, and instead follow the previous staked value.

Proof of Concept

When creating a new stake through RollupUserLogic.stakeOnNewAssertion(), the user sets the AssertionInputs and the expectedAssertionHash.

function stakeOnNewAssertion(AssertionInputs calldata assertion, bytes32 expectedAssertionHash)

The function then checks that the staker has amountStaked > requiredStake

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

If the newAssertionHash has a first child, then transfer the tokens from the contract to the loserStakeEscrow.

assertion.beforeStateData.configData.requiredStake is a user input, which means the assertion creator can set it at some arbitrary value.

When cross checking whether the prevAssertion exists (therefore using the beforeStateData), the requiredStake is not checked.

Line 184:
 bytes32 prevAssertion = RollupLib.assertionHash(
            assertion.beforeStateData.prevPrevAssertionHash,
            assertion.beforeState,
            assertion.beforeStateData.sequencerBatchAcc
        );
        getAssertionStorage(prevAssertion).requireExists();

In RollupCore.createNewAssertion, the beforeStateData is checked and compared with the previous assertion.

Line 370
  // Validate the config hash
        RollupLib.validateConfigHash(
            assertion.beforeStateData.configData, getAssertionStorage(prevAssertionHash).configHash
        );

This means that the required stake will always be the same as the previous stake. If, for some reason, the stake is set at an extremely low price for a previous assertion, the next few assertion will follow this amount. If the previous stake is set at zero, then the new assertions require zero bonds since requiredStake is a user input.

The correct idea should be that every assertion should require a certain amount staked, and not follow the previous assertion.

Tools Used

Recommended Mitigation Steps

At the highest level, compare the user input stake with a value that the protocol dictates which is needed for creating an assertion, instead of following the previous assertion staked value.

Assessed type

Context