`deposit` would revert if token address is `WETH`

[code423n4]

Lines of code

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

Vulnerability details

Impact

deposits to WETH would revert

Proof of Concept

In GeVault.deposit validates that require(token == address(token0) || token == address(token1), "GEV: Invalid Token"); but fails to include the case that token address could also be WETH This implies that any call to deposit with WETH as the token address would revert. https://github.com/code-423n4/2023-08-goodentry/blob/main/contracts/GeVault.sol#L251

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");//@audit WETH would be invalidated
    require(amount > 0 || msg.value > 0, "GEV: Deposit Zero");

    // Wrap if necessary and deposit here
    if (msg.value > 0){//@audit 
      require(token == address(WETH), "GEV: Invalid Weth");//@audit but WETH is required but won't pass the first check
      // wraps ETH by sending to the wrapper that sends back WETH
      WETH.deposit{value: msg.value}();
      amount = msg.value;
    }
    else { 
      ERC20(token).safeTransferFrom(msg.sender, address(this), amount);
    }

    ...SNIP
  }

You can see that WETH address won't pass the first check but it's require subsequently with the function.

Tools Used

Manual Review

Recommended Mitigation Steps

WETH address should be included within the first check

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) || token == address(WETH)), "GEV: Invalid Token");
    require(amount > 0 || msg.value > 0, "GEV: Deposit Zero");

    // Wrap if necessary and deposit here
    if (msg.value > 0){ 
      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 { 
      ERC20(token).safeTransferFrom(msg.sender, address(this), amount);
    }

    ...SNIP
  }

Assessed type

Invalid Validation

[141345]

WETH as token

QA might be more appropriate.

[c4-pre-sort]

141345 marked the issue as duplicate of #493

[c4-judge]

gzeon-c4 marked the issue as unsatisfactory: Invalid

[emmydev9]

POC shows that depositing ETH in a non ETH pool reverts, which is expected. When depositing ETH in a ETH pool, bc underlying ETH is converted to WETH by uniswap, the token address passed should be WETH.

From sponsors comment on #570 it is expected that the address passed to deposit should be either WETH or token0 or token1. But looking closely at deposit there is no way WETH would pass the first validation at L#251. This would create an unreachable code from L#255-L#260 when the address is WETH because it would revert early.

[gzeon-c4]

I believe you are referring to this sponsor comment

a WETH pool would have either token0 or token1 as WETH