code-423n4 / 2023-04-caviar-findings

9 stars 4 forks source link

PrivatePool ERC-20 allowance stealing via reconfiguration frontrunning #916

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-04-caviar/blob/cd8a92667bcb6657f70657183769c244d04c015c/src/PrivatePool.sol#L538-L542

Vulnerability details

Impact

The PrivatePool owner can front-run other traders' transactions in their own pool by changing the pool configuration just before the transaction, not in the trader's favor.

In PrivatePool, there are functions that assume the trader has approved the contract to spend their funds. For example, these functions are buy(), change(), and deposit():

However, these functions do not reset the allowance based on the results of the trades. The allowance can be greater than the commission requires, up to infinity. For example, some DeFi projects on Ethereum Mainnet set a request on the frontend to sign a transaction for infinite allowance so that the user does not pay unnecessary fees next time they interact with the project.

A malicious owner can change the reserve configuration by calling the setVirtualReserves() function with arbitrary values, as the function does not require the parameters to match the pool's actual balances. As a result, the trader who trades with PrivatePool using an ERC-20 token as a base will experience huge slippage. The owner will then restore the correct reserve configuration by calling setVirtualReserves() again and then take the profit obtained through withdraw().

It should be noted that the router for private pools with ERC-20 base tokens has not yet been implemented. Therefore, interaction with them is currently possible only directly, which makes the importance of the discovery HIGH.

Recommended Mitigation Steps

It is recommended to implement a router to control slippage for pools with ERC-20 tokens.

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as duplicate of #829