Caviar Private Pools - 0x4db5362c's results

Lines of code

https://github.com/code-423n4/2023-04-caviar/blob/main/src/EthRouter.sol#L273-L281

Vulnerability details

Impact

EthRouter.change() is always failed during batch changing unless 'Factory.protocolFeeRate()' is 0 and every changeFee of PrivatePool is 0 except the last one.

Proof of Concept

Suppose Alice wants to change token(s) of two different nfts in two different PrivatePools. The changeFee of first PrivatePool is not 0. The initial balance of EthRouter is 0 ether.

EthRouter will call PrivatePool(changes[0].pool).change{value: msg.value}() then PrivatePool(changes[1].pool).change{value: msg.value}(). Since PrivatePool(changes[1].pool) will keep some ether as changeFee and protocolFeeAmount, the ether balance of EthRouter will be less than msg.value, there is no enough ether for next change: PrivatePool(changes[0].pool).change{value: msg.value}() and the whole transaction will be reverted.

Tools Used

None

Recommended Mitigation Steps

Change msg.value to address(this).balance to ensure the ether value sent to next call PrivatePool.change() is no more than the ether balance of EthRouter.