Good Entry - shirochan'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

User can lose ETH during swap. The following functions do not check if ETH was safely transferred to the users:

• swapETHForExactTokens
• swapTokensForExactETH
• swapExactTokensForETH

Proof of Concept

The above three functions send Ether via call. Call does not revert if it fails and instead returns a boolean value indicating success or failure. The boolean value is not checked here, so this call can fail silently, and users can potentially lose ETH.

msg.sender.call{value: <AMOUNT>}("");

This call can fail for the following cases:

• msg.sender is a contract without a payable fallback function.
• msg.sender is a contract with a fallback function, but the fallback function reverts or consumes all the available gas.

Tools Used

Manual Review

Recommended Mitigation Steps

Use a helper function that will safely transfer Eth and check that the call completed successfully.

library TransferHelper {
  /// @notice Transfers ETH to the recipient address
  /// @dev Fails with `TransferHelper: ETH_TRANSFER_FAILED`
  /// @param to The destination of the transfer
  /// @param value The value to be transferred
  function safeTransferETH(address to, uint value) internal {
     (bool success,) = to.call{value:value}(new bytes(0));
     require(success, 'TransferHelper: ETH_TRANSFER_FAILED');
  }
}

function swapETHForExactTokens(uint amountOut, address[] calldata path, address to, uint deadline) payable external returns (uint[] memory amounts) {
  ...
  if (msg.value > amounts[0]) TransferHelper.safeTransferETH(msg.sender, msg.value - amounts[0]);

function swapTokensForExactETH(uint amountOut, uint amountInMax, address[] calldata path, address to, uint deadline) payable external returns (uint[] memory amounts) {
  ...
  TransferHelper.safeTransferETH(msg.sender, amountOut);

function swapExactTokensForETH(uint amountIn, uint amountOutMin, address[] calldata path, address to, uint deadline) payable external returns (uint[] memory amounts) {
  ...
  TransferHelper.safeTransferETH(msg.sender, amounts[1]);

Assessed type

Invalid Validation