Arbitrum BoLD - twcctop's results

Lines of code

https://github.com/code-423n4/2024-05-arbitrum-foundation/blob/265c57800145734362a4bb1b46465ff35b47beac/src/rollup/BOLDUpgradeAction.sol#L341

Vulnerability details

Impact

Some stakers in old roll up will lose their stake when upgrading to new rollup.

Proof of Concept

when bold update , in function perform, we will call cleanupOldRollup to clean up old rollup. The stake count is set to max value 50, it's a magic number, if the staker count is greater than 50, the staker will lose their stake when upgrading to new rollup. In roll up logic, we don't have any check to make sure that staker amount is less than 50, so it's possible to lose stake when upgrading to new rollup.

   function cleanupOldRollup() private {
        IOldRollupAdmin(address(OLD_ROLLUP)).pause();

        uint64 stakerCount = ROLLUP_READER.stakerCount();
        // since we for-loop these stakers we set an arbitrary limit - we dont
        // expect any instances to have close to this number of stakers
 @>       if (stakerCount > 50) {
            stakerCount = 50;
        }
        for (uint64 i = 0; i < stakerCount; i++) {
            address stakerAddr = ROLLUP_READER.getStakerAddress(i);
            OldStaker memory staker = ROLLUP_READER.getStaker(stakerAddr);
            if (staker.isStaked && staker.currentChallenge == 0) {
                address[] memory stakersToRefund = new address[](1);
                stakersToRefund[0] = stakerAddr;

                IOldRollupAdmin(address(OLD_ROLLUP)).forceRefundStaker(stakersToRefund);
            }
        }

        // upgrade the rollup to one that allows validators to withdraw even whilst paused
        DoubleLogicUUPSUpgradeable(address(OLD_ROLLUP)).upgradeSecondaryTo(IMPL_PATCHED_OLD_ROLLUP_USER);
    }

Tools Used

manual

Recommended Mitigation Steps

add check to make sure that staker amount is less than 50 when upgrading to new rollup or remove the magic number 50 .

Assessed type

Invalid Validation