code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

EthRouter provides or offers a way to execute a buy batch from PrivatePool, A user will try to call buy function in EthRouter to execute a buy operations against private pool, on EthRouter at L129 The buy function in PrivatePool will be triggered and executed, on buy function inside PrivatePool contract L281 a safeTransferETH will be called to transfer a royaltyFee to a recipient.

A recipient contract can call EthRouter buy function with empty parameters' on receive() function. Now buy function inside EthRouter contract will continue to line L142 and call safeTransferETH to transfer the balance of EthRouter to msg.sender. Therefore, the recipient steal the funds from EthRouter balance that's considered as refund for the buyer. eventually after a transfer made to the recipient contract the function buy inside the EthRouter will continue executing and will reach the line L142 to refund any surplus ETH to the caller buyer but the balance already gone and stolen by recipient attacker.

Proof of Concept

A simple scenario for the Reentrancy attack: let's name buy function on EthRouter as Ebuy, and buy function at PrivatePool as Pbuy for clarity.

Bob will call Ebuy function to execute a buy operation with 5 ETH.
Ebuy function eventually will call Pbuy function.
Pbuy will be executed and transfer the surplus ETH to EthRouter (assume it is 1 ETH)
Pbuy will transfer royaltyFee to the recipient Dave's contract.
Dave's contract is calling Ebuy on receive() function.
Ebuy will be called with empty params and a transfer of EthRouter's balance (1 ETH) will be sent to caller Dave's recipient contract
Bob's Ebuy call is continuing the process and execute the rest of the function
Bob doesn't receive his refund 1 ETH since it was stolen by Dave's contract.

Tools Used

Manual review

Recommended Mitigation Steps

Use reentrancy guard.

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as duplicate of #752

c4-judge commented 1 year ago

GalloDaSballo changed the severity to 2 (Med Risk)