Good Entry - jesusrod15's results

Lines of code

https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/RangeManager.sol#L95-L102 https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/TokenisableRange.sol#L159-L160

Vulnerability details

Impact

owner can lose part of theirs funds

Proof of Concept

function initRange(address tr, uint amount0, uint amount1) external onlyOwner {
    ASSET_0.safeTransferFrom(msg.sender, address(this), amount0);
    ASSET_0.safeIncreaseAllowance(tr, amount0);
    ASSET_1.safeTransferFrom(msg.sender, address(this), amount1);
    ASSET_1.safeIncreaseAllowance(tr, amount1);
    TokenisableRange(tr).init(amount0, amount1);
    ERC20(tr).safeTransfer(msg.sender, TokenisableRange(tr).balanceOf(address(this)));
  }


function init(uint n0, uint n1) external {
    require(status == ProxyState.INIT_LP, "!InitLP");
    require(msg.sender == creator, "Unallowed call");
    status = ProxyState.READY;
    TOKEN0.token.safeTransferFrom(msg.sender, address(this), n0);
    TOKEN1.token.safeTransferFrom(msg.sender, address(this), n1);
    TOKEN0.token.safeIncreaseAllowance(address(POS_MGR), n0);
    TOKEN1.token.safeIncreaseAllowance(address(POS_MGR), n1);
    (tokenId, liquidity, , ) = POS_MGR.mint( 
      INonfungiblePositionManager.MintParams({
         token0: address(TOKEN0.token),
         token1: address(TOKEN1.token),
         fee: feeTier * 100,
         tickLower: lowerTick,
         tickUpper: upperTick,
         amount0Desired: n0,
         amount1Desired: n1,
         amount0Min: n0 * 95 / 100,
         amount1Min: n1 * 95 / 100,
         recipient: address(this),
         deadline: block.timestamp
      })
    );
    
    // Transfer remaining assets back to user
    TOKEN0.token.safeTransfer( msg.sender,  TOKEN0.token.balanceOf(address(this)));
    TOKEN1.token.safeTransfer(msg.sender, TOKEN1.token.balanceOf(address(this)));
    _mint(msg.sender, 1e18);
    emit Deposit(msg.sender, 1e18);
  }

“When the owner calls initRange, this function transfers funds from their wallet to the contract. Then, this function calls TokenisableRange(tr).init(amount0, amount1), which transfers funds from RangeManager.sol to TokenisableRange.sol. As you can see, the init function mints tokens and then transfers the remaining assets back to the user. The problem is that msg.sender is RangeManager.sol, so the funds are transferred to the contract, but there is no way for the owner to transfer the remaining funds back directly to their wallet.

Tools Used

manual review

Recommended Mitigation Steps

add this in the function

 function initRange(address tr, uint amount0, uint amount1) external onlyOwner {
    ASSET_0.safeTransferFrom(msg.sender, address(this), amount0);
    ASSET_0.safeIncreaseAllowance(tr, amount0);
    ASSET_1.safeTransferFrom(msg.sender, address(this), amount1);
    ASSET_1.safeIncreaseAllowance(tr, amount1);
    TokenisableRange(tr).init(amount0, amount1);
    ERC20(tr).safeTransfer(msg.sender, TokenisableRange(tr).balanceOf(address(this)));
+   ASSET_0.safeTransfer(msg.sender, ASSET_0.balanceOf(address(this));
+   ASSET_1.safeTransfer(msg.sender, ASSET_1.balanceOf(address(this));
  }

Assessed type

ERC20