Caviar Private Pools - fs0c's results

Lines of code

https://github.com/code-423n4/2023-04-caviar/blob/main/src/Factory.sol#L92

Vulnerability details

Impact

In the worst case a user might loose the funds they deposit to the PrivatePool. This includes both the NFT and the ERC20 token they deposit as the deposit might go to the PrivatePool that an attacker owns.

Vulnerability Details

Private Pools are created using the following code which internally calls create2 with the salt specified by the user:

privatePool = PrivatePool(
            payable(privatePoolImplementation.cloneDeterministic(_salt))
        );

Calling this with the same _salt will result in same privatePool address, but not in a regular scenario as the transaction would fail the second time.

Imagine a scenario where a legit user Alice calls the function create to deploy the privatePool and later wants to call deposit to send funds to it. Bob sees that network block re-org happens and calls the function create with the same salt that Alice used, this would make Bob the actual owner of that pool and Alice’s transaction would be invalidated. Now Alice thinks that they own this privatePool as they recieved the address of the privatePool, so they call deposit , and as the deposit doesn’t check the owner the transaction would pass through and they would transfer tokens to Bob’s privatepool where bob can now withdraw the funds.

Recommendation

Salt can also include msg.sender so the addresses would be different for different users.