Caviar Private Pools - zion's results

Lines of code

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

Vulnerability details

Impact

Because of the CREATE2 opcode clones an existing contract at a deterministic address by the salt, this could be front-run by an MEV bot passing in the same parameters and changing the salt.

Proof of Concept

// @params (_baseToken, _nft, _virtualBaseTokenReserves, _virtualNftReserves, _changeFee, _feeRate, _merkleRoot, _useStolenNftOracle, _payRoyalties, tokenIds, baseTokenAmount) could all be copied // while @param _salt can be changed and the transaction could be front-run to essentially take up the address that the salt would have clone would now revert.

function create( address _baseToken, address _nft, uint128 _virtualBaseTokenReserves, uint128 _virtualNftReserves, uint56 _changeFee, uint16 _feeRate, bytes32 _merkleRoot, bool _useStolenNftOracle, bool _payRoyalties, bytes32 _salt, uint256[] memory tokenIds, // put in memory to avoid stack too deep error uint256 baseTokenAmount ) public payable returns (PrivatePool privatePool) {

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

Tools Used

https://docs.openzeppelin.com/cli/2.8/deploying-with-create2

Recommended Mitigation Steps

make the salt unique based off of hashing the existing parameters and by adding msg.sender to ensure each salt is soley unique to one sender.

salt = sha3({params}, msg.sender)