sherlock-audit / 2023-09-Gitcoin-judging

11 stars 7 forks source link

Arz - Fee on transfer tokens can cause problems when distributing funds #903

Closed sherlock-admin2 closed 11 months ago

sherlock-admin2 commented 12 months ago

Arz

medium

Fee on transfer tokens can cause problems when distributing funds

When using fee on transfer tokens the poolAmount in the pool will differ from the actual balance of the pool which can cause problems when distributing funds.

Vulnerability Detail

As stated in the docs, the pools can use fee on transfer tokens however the problem is that when funding a pool or allocating, the amount we sent to the pool will differ from the amount that we used to increment the poolAmount or the claims[].

The tokens will be sent however this will cause problems when distributing funds because the actual balance will be smaller than the amount we should receive so the tx can revert.

So for example we have 3 recipients and i allocate 100 USDT to each recipient. The fee is 5% so i have sent 95 USDT but their claims[recipientId][token] was increased by 100.

Each recipient will then claim his tokens but because their claims is bigger than the actual balance of the contract, only 2 of the recipients will be able to claim their funds. The 3rd one wont be able to claim his funds because the amount to claim is bigger than the actual balance left.

Impact

This will cause problems when distributing the funds because the amount to receive is bigger than the actual balance and some recipients will fail to get their funds.

Code Snippet

https://github.com/sherlock-audit/2023-09-Gitcoin/blob/6430c8004017e96ae2f5aac365bdefd0b6eeea72/allo-v2/contracts/core/Allo.sol#L516-L517

516:     _transferAmountFrom(_token, TransferData({from: msg.sender, to: address(_strategy), amount: amountAfterFee}));
517:     _strategy.increasePoolAmount(amountAfterFee);

As you can see here for example the poolAmount is increased by the amount sent but not the actual amount that the contract received. This also applies to other functions like _afterAllocate() in the DonationVotingMerkleDistributionVaultStrategy

Tool used

Manual Review

Recommendation

The transfer functions can be updated to return the actually received amount by checking the balance before and after the transfer.

Duplicate of #547