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

3 stars 2 forks source link

Conflicting checks in GeVault.withdraw() makes it impossible to redeem all liquidity by inputting 0 as liquidity #174

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/GeVault.sol#L216 https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/GeVault.sol#L218

Vulnerability details

Impact

if liquidity is 0, one should be able to redeem all GEV tokens when calling GeVault.withdraw() according to intended design, see here

    if (liquidity == 0) liquidity = balanceOf(msg.sender);

but this isn't going to happen due to a conflicting check i.e this

   require(liquidity > 0, "GEV: Withdraw Zero");

It won't be possible to redeem all GEV tokens by passing 0 as liquidity when calling GeVault.withdraw().

Proof of Concept

This check

   require(liquidity > 0, "GEV: Withdraw Zero");

prevents the below line of code

  if (liquidity == 0) liquidity = balanceOf(msg.sender);

Tools Used

LOFI Radio and Manual Review

Recommended Mitigation Steps

remove this check

 require(liquidity > 0, "GEV: Withdraw Zero");

Assessed type

DoS

c4-pre-sort commented 1 year ago

141345 marked the issue as low quality report

141345 commented 1 year ago

invalid

if (liquidity == 0) liquidity = balanceOf(msg.sender); then liquidity > 0

c4-judge commented 1 year ago

gzeon-c4 marked the issue as unsatisfactory: Invalid