ETH sent directly to `GeVault` would not be wrapped as intended.

[code423n4]

Lines of code

https://github.com/code-423n4/2023-08-goodentry/blob/main/contracts/GeVault.sol#L463

Vulnerability details

Impact

ETH sent to the GeVault would be reverted.

Proof of Concept

GeVault implements recieve payable to wrap any eth sent directly into the contracts but fails to send the msg.value following the call to deposit, this causes the call to attempt to transfer token from msg.sender instead of depositing/wrapping ETH.

receive() external payable {
    if(msg.sender != address(WETH)) deposit(address(WETH), msg.value);
  }

When ETH is sent to the contract is attempt to deposit.

function deposit(address token, uint amount) public payable nonReentrant returns (uint liquidity) 
  {
    require(isEnabled, "GEV: Pool Disabled");
    require(poolMatchesOracle(), "GEV: Oracle Error");
    require(token == address(token0) || token == address(token1), "GEV: Invalid Token");
    require(amount > 0 || msg.value > 0, "GEV: Deposit Zero");

    // Wrap if necessary and deposit here
    if (msg.value > 0){//@audit direct deposit from recieve would fail
      require(token == address(WETH), "GEV: Invalid Weth");
      // wraps ETH by sending to the wrapper that sends back WETH
      WETH.deposit{value: msg.value}();
      amount = msg.value;
    }
    else {//@audit this block would be executed instead 
      ERC20(token).safeTransferFrom(msg.sender, address(this), amount);
    }
   ...SNIP

  }

But since the msg.value is not pass to the call to deposit it would be zero and would attempt to transferFrom rather than wrapping the ETH.

Tools Used

Manual Review

Recommended Mitigation Steps

Pass the msg.value to the call to deposit

receive() external payable {
+    if(msg.sender != address(WETH)) deposit{value: msg.value}(address(WETH), msg.value);
  }

Assessed type

ETH-Transfer

[141345]

user mistake

QA might be more appropriate.

[c4-sponsor]

Keref marked the issue as sponsor disputed

[Keref]

Internal function calls don't need to pass tx parameters around. msg.value is still set in deposit. This isn't an external call

[c4-judge]

gzeon-c4 marked the issue as unsatisfactory: Invalid