Platform: Code4rena
Start Date: 10/05/2024
Pot Size: $300,500 USDC
Total HM: 4
Participants: 27
Period: 17 days
Judge: Picodes
Total Solo HM: 1
Id: 375
League: ETH
Rank: 27/27
Findings: 1
Award: $0.00
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: Sathish9098
Also found by: Audinarey, Dup1337, K42, KupiaSec, LessDupes, Rhaydden, SpicyMeatball, Takarez, ZanyBonzy, bronze_pickaxe, carlitox477, dontonka, forgebyola, fyamf, guhu95, hihen, josephdara, ladboy233, slvDev, twcctop, xuwinnie, zanderbyte
0 USDC - $0.00
The BoLD upgrade action will fail
The BoLD upgrade is processed via BOLDUpgradeAction.sol
which will be called through delegatecall
from the governance.
During the upgrade process, it pauses the old Nitro rollup contract and force-refund all stakes in the contract before the upgrade.
function cleanupOldRollup() private { IOldRollupAdmin(address(OLD_ROLLUP)).pause(); // ... for (uint64 i = 0; i < stakerCount; i++) { // ... IOldRollupAdmin(address(OLD_ROLLUP)).forceRefundStaker(stakersToRefund); // ... } // ... }
The pause called during the upgrade, which means validators can still create an assertion on the old Nitro contract.
If any validator creates an assertion before the upgrade happens, the forceRefundStaker
will revert which will prevent the upgrade as the code snippet below shows(Nitro's RollupAdminLogic.sol:L276).
function forceRefundStaker(address[] calldata staker) external override whenPaused { require(staker.length > 0, "EMPTY_ARRAY"); for (uint256 i = 0; i < staker.length; i++) { require(_stakerMap[staker[i]].currentChallenge == NO_CHAL_INDEX, "STAKER_IN_CHALL"); reduceStakeTo(staker[i], 0); turnIntoZombie(staker[i]); } emit OwnerFunctionCalled(22); }
This reverts because the staker's current challenge exists.
Manual Review
Rather than pausing and force-refunding all stakes in the Nitro contract, it should upgrade the old Nitro rollup contract to a patch contract that allows existing stakers to withdraw their stakes by themselves.
DoS
#0 - c4-judge
2024-06-04T16:44:05Z
Picodes changed the severity to QA (Quality Assurance)
#1 - JeffCX
2024-06-05T20:00:06Z
this report claim
If any validator creates an assertion before the upgrade happens, the forceRefundStaker will revert which will prevent the upgrade as the code snippet below shows(Nitro's RollupAdminLogic.sol:L276).
but in the original submission code snippet:
function cleanupOldRollup() private { IOldRollupAdmin(address(OLD_ROLLUP)).pause(); // ... for (uint64 i = 0; i < stakerCount; i++) { // ... IOldRollupAdmin(address(OLD_ROLLUP)).forceRefundStaker(stakersToRefund); // ... } // ... }
the first ... actually prevent this issue from happens.
only if there is no ongoing challange the staker's fund get released.
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); } }
note the logic there.
OldStaker memory staker = ROLLUP_READER.getStaker(stakerAddr); if (staker.isStaked && staker.currentChallenge == 0) {
#2 - c4-judge
2024-06-10T17:24:19Z
Picodes marked the issue as grade-b