code-423n4 / 2023-08-goodentry-findings

3 stars 2 forks source link

Function deposit is vulnerable to reentrancy. #9

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-08-goodentry/blob/4b785d455fff04629d8675f21ef1d1632749b252/contracts/GeVault.sol#L247

Vulnerability details

Impact

Function deposit is vulnerable to reentrancy. It doesn't update the contract's state before calling WETH.deposit() on the external contract.

Proof of Concept

Attacker can repeatedly call deposit with a malicious contract that re-enters and manipulates the state, causing a reentrancy attack.

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){
  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);
}

https://github.com/code-423n4/2023-08-goodentry/blob/4b785d455fff04629d8675f21ef1d1632749b252/contracts/GeVault.sol#L247C3-L263C6

Tools Used

Manual Audit

Recommended Mitigation Steps

Update the state before making external calls to WETH.deposit()

Assessed type

Reentrancy

c4-pre-sort commented 1 year ago

141345 marked the issue as duplicate of #2

c4-judge commented 1 year ago

gzeon-c4 marked the issue as unsatisfactory: Insufficient quality