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

0 stars 0 forks source link

Anyone can claim prizes on behalf of someone #25

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

cmichel

Vulnerability details

The PrizeDistributor.claim function allows claiming the prize for other users. While the payouts are sent to the correct address, this can lead to issues with smart contracts that might rely on claiming the tokens themselves.

As one example, suppose the _to address corresponds to a smart contract that has a function of the following form:

function claimAndDoSomething() {
    uint256 claimedAmount = payout.balanceOf(address(this));
    prizeDistributor.claim();
    claimedAmount = payout.balanceOf(address(this)) - claimedAmount;
    // transfer or swap
    payout.transfer(externalWallet, claimedAmount);
}

Impact

If the contract has no other functions to transfer out funds, they may be locked forever in this contract. Or some accounting might rely on all prizes being registered by using the return value of claim (think a yearn-style aggregator claiming prizes and using its return value to immediately re-invest).

Recommended Mitigation Steps

Do not allow users to claim on behalf of other users.

asselstine commented 3 years ago

I think this is an interesting design constraint, but we're going to keep it flexible for the moment.

We can always roll out a new Prize Distributor contract that enforces it!

GalloDaSballo commented 3 years ago

The sponsor acknowledges, however I don't believe there's any finding here. As long as the contracts are immutable, any integrator will be well aware of this functionality and would build around it

In the case of Yearn, they would just have a boolean flag claimBeforeHarvest that would be settable to sidestep the issue

Setting as invalid