Good Entry - Udsen's results

Lines of code

https://github.com/code-423n4/2023-08-goodentry/blob/main/contracts/helper/V3Proxy.sol#L156 https://github.com/code-423n4/2023-08-goodentry/blob/main/contracts/helper/V3Proxy.sol#L174 https://github.com/code-423n4/2023-08-goodentry/blob/main/contracts/helper/V3Proxy.sol#L192

Vulnerability details

Impact

The V3Proxy.swapTokensForExactETH and V3Proxy.swapExactTokensForETH functions use the low level call function to transfer Eth to the msg.sender as shown below:

    payable(msg.sender).call{value: amountOut}("");

But there is no verification of the return value of the above low level call execution. The call function will not revert during failure but return boolean false value. Hence no checking the return value of the low level call execution will deem the transaction as successful even if the Eth transfer failed thus putting the msg.sender to a disadvantage and breaking the state of the contract as well.

Proof of Concept

        msg.sender.call{value: msg.value - amounts[0]}("");

https://github.com/code-423n4/2023-08-goodentry/blob/main/contracts/helper/V3Proxy.sol#L156

        payable(msg.sender).call{value: amountOut}("");

https://github.com/code-423n4/2023-08-goodentry/blob/main/contracts/helper/V3Proxy.sol#L174

        payable(msg.sender).call{value: amounts[1]}("");

https://github.com/code-423n4/2023-08-goodentry/blob/main/contracts/helper/V3Proxy.sol#L192

Tools Used

Manual Review and VSCode

Recommended Mitigation Steps

Hence it is recommended to check the return value of the low level call function as shown below:

(bool success, ) = payable(msg.sender).call{value: amountOut}("");
require(success, "Transfer failed");

Assessed type

ETH-Transfer