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

3 stars 2 forks source link

Lack of Maximum Cap Check when depositing into `GeValut` #400

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#L269-L279

Vulnerability details

There is a check to ensure that thevalueX8of the deposited tokens does not exceed the tvlCap. However, it would be beneficial to add an additional check to compare the current TVL (Total Value Locked) with a maximum allowed TVL. If the current TVL reaches the maximum limit, the deposit function should reject any further deposits.

Impact

The current implementation of the TVL (Total Value Locked) in the contract does not have a check against the maximum allowed value. This could potentially result in the TVL exceeding the intended maximum value that should be locked in the contract. The lack of a check against the maximum value for the Total Value Locked (TVL), can have several potential impacts. Here are a few that come to mind:

  1. Financial Risk: Without a check in place, the TVL could unintentionally exceed the maximum limit set by the contract. This has the potential to put the funds at risk, as the system may not be designed to handle such large amounts. This could lead to unforeseen consequences, such as failed transactions or even potential loss of funds.

  2. Contract Stability: Exceeding the maximum TVL may strain the stability and efficiency of the contract itself. Without proper checks and balances, it's possible that the contract's performance and reliability could be compromised. This could lead to delayed transactions, system malfunctions, or even a complete breakdown of functionality.

Proof of Concept

  require(tvlCap > valueX8 + getTVL(), "GEV: Max Cap Reached");
     uint vaultValueX8 = getTVL();
     uint tSupply = totalSupply();
     // initial liquidity at 1e18 token ~ $1
     if (tSupply == 0 || vaultValueX8 == 0)
     liquidity = valueX8 * 1e10;
  else {
    liquidity = tSupply * valueX8 / vaultValueX8;
  }

In the code sinpet above there is a check to ensure that thevalueX8of the deposited tokens does not exceed the tvlCap. However, it would be beneficial to add an additional check to compare the current TVL (Total Value Locked) with a maximum allowed TVL.

Tools Used

Manual Review

Recommended Mitigation Steps

define a variable maximumTVL and set it to the desired maximum allowed TVL. Then, before accepting a deposit, you can compare the current TVL with the maximumTVL using an if statement:

require(currentTVL + valueX8 <= maximumTVL, "Exceeded maximum TVL");

This code ensures that the current Total Value Locked (currentTVL) plus the value times 8 (valueX8`) of the deposited tokens does not exceed the maximum allowed TVL. If the maximum TVL is reached, the deposit function will reject any further deposits and revert with an error message.

Assessed type

Invalid Validation

c4-pre-sort commented 1 year ago

141345 marked the issue as low quality report

141345 commented 1 year ago

invalid

already has tvlCap

c4-judge commented 1 year ago

gzeon-c4 marked the issue as unsatisfactory: Invalid