code423n4 commented 1 year ago

Lines of code

https://github.com/prepo-io/prepo-monorepo/blob/3541bc704ab185a969f300e96e2f744a572a3640/apps/smart-contracts/core/contracts/TokenSender.sol#L41-L42

Vulnerability details

Impact

When a user performs a transaction with fees, TokenSender.sol sends a proportionate amount of PPO tokens to them to reimburse them.

The assumption for a user should be that the sum of multiple smaller transactions should provide the same refund as one larger transaction. There shouldn't be an incentive to break up transactions in order to maximize refund.

Once the amount of PPO is calculated, the function checks the balance of the contract's PPO. If it is less than the outputAmount, no funds are send.

The result is that a user who should have received some refund (and would receive a refund if they broke up their transaction) can receive zero, even if there are PPO funds in the contract.

Proof of Concept

Let's imagine a user is entitled to a refund of $10. Exchange rate is 1:1. There are 5 PPO in the contract.
The function checks if (outputAmount > _outputToken.balanceOf(address(this))) return;
The results in a return and skips the transfer of PPO.
If the user performed two transactions with refunds of $5 each, the first would send the refund and the second wouldn't.

Tools Used

Manual Review

Recommended Mitigation Steps

Instead of returning if outputAmount > balance, simply set outputAmount = _outputToken.balanceOf(address(this)) before transferring.

This will result in sending the remaining funds to the user if they deserve a refund, rather than skipping it and sending it to a future user with a smaller transaction.

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #313