Lines of code

https://github.com/code-423n4/2024-05-arbitrum-foundation/blob/main/src/rollup/RollupUserLogic.sol#L358

Vulnerability details

Impact

In the current implementation of the RollupUserLogic, a new assertion can only be created if a user staked the requiredStake amount and therefore became a validator. If there are 2 children of the previous assertion, challenge dispute game can be started and the second child has to send his amount to the loserStakeEscrow. The problem is that it's not somehow reflected in the RollupCore mapping structure and therefore the user can possibly request a withdraw after the finish of the dispute.

Proof of Concept

Let's say there are 2 assertions: A and B. The first assertion stakes the required amount and suggest a new assertion:

https://github.com/code-423n4/2024-05-arbitrum-foundation/blob/main/src/rollup/RollupUserLogic.sol#L175

 require(isStaked(msg.sender), "NOT_STAKED");

https://github.com/code-423n4/2024-05-arbitrum-foundation/blob/main/src/rollup/RollupUserLogic.sol#L182

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

And the mapping in RollupCore is also updated:

https://github.com/code-423n4/2024-05-arbitrum-foundation/blob/main/src/rollup/RollupCore.sol#L277

_stakerMap[stakerAddress] = Staker(depositAmount, _latestConfirmed, stakerIndex, true, withdrawalAddress);

When the second assertion (B) is created, the process is the same and but the stake is transferred to the loserStakeEscrow as it's the second child:

https://github.com/code-423n4/2024-05-arbitrum-foundation/blob/main/src/rollup/RollupUserLogic.sol#L210-216

 if (!getAssertionStorage(newAssertionHash).isFirstChild) {
            // We assume assertion.beforeStateData is valid here as it will be validated in createNewAssertion
            // only 1 of the children can be confirmed and get their stake refunded
            // so we send the other children's stake to the loserStakeEscrow
            // NOTE: if the losing staker have staked more than requiredStake, the excess stake will be stuck
            IERC20(stakeToken).safeTransfer(loserStakeEscrow, assertion.beforeStateData.configData.requiredStake);
        }

The problem is that once assertion is confirmed, the second user can still withdraw his stake even if he submitted incorrect state:

https://github.com/code-423n4/2024-05-arbitrum-foundation/blob/main/src/rollup/RollupUserLogic.sol#L358-364

function withdrawStakerFunds() external override whenNotPaused returns (uint256) {
        uint256 amount = withdrawFunds(msg.sender);
        require(amount > 0, "NO_FUNDS_TO_WITHDRAW");
        // This is safe because it occurs after all checks and effects
        IERC20(stakeToken).safeTransfer(msg.sender, amount);
        return amount;
    }

By doing this, he bypasses the protocol requirement of the malicious party's stake to be taken in the case of submitting malicious assertion.

Tools Used

Manual review.

Recommended Mitigation Steps

Set some flag for the second assertion when the first assertion is confirmed.

Assessed type

Other

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

The staker that lost the challenge dispute game can still withdraw his stake #32