Good Entry - ravikiranweb3's results

Lines of code

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

Vulnerability details

Impact

Swapping tokens for ETH will fail when the msg.sender is a smart contract that does not implement receive and fallback functions.

Proof of Concept

Smart contract cannot receive ETH if they dont implement receive and fallback functions. If such contracts participate in swapping, the ETH value will be stuck as there is no logic implemented to reverse the swapping process.

function swapTokensForExactETH(uint amountOut, uint amountInMax, address[] calldata path, address to, uint deadline) payable external returns (uint[] memory amounts) {
        require(path.length == 2, "Direct swap only");
        require(path[1] == ROUTER.WETH9(), "Invalid path");
        ERC20 ogInAsset = ERC20(path[0]);
        ogInAsset.safeTransferFrom(msg.sender, address(this), amountInMax);
        ogInAsset.safeApprove(address(ROUTER), amountInMax);
        amounts = new uint[](2);
        amounts[0] = ROUTER.exactOutputSingle(ISwapRouter.ExactOutputSingleParams(path[0], path[1], feeTier, address(this), deadline, amountOut, amountInMax, 0));         
        amounts[1] = amountOut; 
        ogInAsset.safeApprove(address(ROUTER), 0);
        IWETH9 weth = IWETH9(ROUTER.WETH9());
        acceptPayable = true;
        weth.withdraw(amountOut);
        acceptPayable = false;
        payable(msg.sender).call{value: amountOut}("");
        emit Swap(msg.sender, path[0], path[1], amounts[0], amounts[1]); 
    }

    function swapExactTokensForETH(uint amountIn, uint amountOutMin, address[] calldata path, address to, uint deadline) payable external returns (uint[] memory amounts) {
        require(path.length == 2, "Direct swap only");
        require(path[1] == ROUTER.WETH9(), "Invalid path");
        ERC20 ogInAsset = ERC20(path[0]);
        ogInAsset.safeTransferFrom(msg.sender, address(this), amountIn);
        ogInAsset.safeApprove(address(ROUTER), amountIn);
        amounts = new uint[](2);
        amounts[0] = amountIn;         
        amounts[1] = ROUTER.exactInputSingle(ISwapRouter.ExactInputSingleParams(path[0], path[1], feeTier, address(this), deadline, amountIn, amountOutMin, 0));
        ogInAsset.safeApprove(address(ROUTER), 0); 
        IWETH9 weth = IWETH9(ROUTER.WETH9());
        acceptPayable = true;
        weth.withdraw(amounts[1]);
        acceptPayable = false;
        payable(msg.sender).call{value: amounts[1]}("");
        emit Swap(msg.sender, path[0], path[1], amounts[0], amounts[1]);                 
    }

If the msg.sender is a smart contract that cannot accept ether, then there should be a reversal process for swapping or else the external transfers related to swapping will not auto revert and could incur losses.

Tools Used

Manual

Recommended Mitigation Steps

1. Documentation for smart contract interaction
2. Reversal of swapping incase the msg.sender being a smart contract cannot accept ether.

Assessed type

ETH-Transfer

1. PositionManager has two state variables that are never initialized or used in the contract.

    ILendingPool public LENDING_POOL; 
    RoeRouter public ROEROUTER; 
   

2. Aave lendingPool::repay function returns the final amount that was repaid. The remaining balance should be computed based on return value from the pool and now internal to the contract is a better approach,

 if ( debt > 0 ){
        if (amt <= debt ) {
          LP.repay( asset, amt, 2, user);
          return;
        }
        else {
          LP.repay( asset, debt, 2, user);
          amt = amt - debt;
        }
      }
      // deposit remaining tokens
      LP.deposit( asset, amt, user, 0 );

3. RangeManager contract is using older complier and also has floating pragma.

4. V2Proxy::swapExactTokensForTokens() has to parameter, but never used in the function. The same issue lies with other swap functions where to was not used.

5. FixedOracle contract is using floating pragma and different version of compiler.