code-423n4 / 2023-09-venus-findings

4 stars 4 forks source link

Unchecked Return Value in `estimateAPR`. #366

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-venus/blob/9c1016326a0e97376749860266c4541a313a86c2/contracts/Tokens/Prime/Prime.sol#L527-L548

Vulnerability details

Impact

If _capitalForScore encounters an error or an exceptional condition, it might return unexpected or invalid values for capital, cappedSupply, and cappedBorrow. If these values are not properly validated, it could lead to incorrect APR estimations, potentially affecting user rewards.

Proof of Concept

In the estimateAPR function, the return value of _capitalForScore is not checked. Depending on the implementation, this may introduce vulnerabilities.

Let's pinpoint the relevant line of code in the estimateAPR function.

The purpose of this line below is to calculate various values used in the estimation of the APR for a user. It involves the calculation of capital, capped supply, and capped borrow based on input parameters like xvsBalanceForScore, borrow, supply, and market.

#L537-543

(uint256 capital, uint256 cappedSupply, uint256 cappedBorrow) = _capitalForScore(
    xvsBalanceForScore,
    borrow,
    supply,
    market
);

Within the estimateAPR function. It calls the _capitalForScore function to calculate the capital, cappedSupply, and cappedBorrow values. However, it does not check the return value of _capitalForScore. It assumes that the function call will succeed and provide valid results.

The issue is, If the _capitalForScore function were to encounter an error or an exceptional condition, it might return unexpected or invalid values for capital, cappedSupply, and cappedBorrow. If these values are not properly validated, it could lead to incorrect APR estimations, potentially affecting user rewards.

https://github.com/code-423n4/2023-09-venus/blob/9c1016326a0e97376749860266c4541a313a86c2/contracts/Tokens/Prime/Prime.sol#L527-L548

_capitalForScore

Destructuring Assignment:

The impact is that, If the return values are not properly validated, incorrect APR estimations may be performed. This could lead to inaccurate rewards being distributed to users, potentially affecting their earnings.

To further prove the issue in the estimateAPR function where the return value of _capitalForScore is not checked, potentially leading to vulnerabilities.

(uint256 capital, uint256 cappedSupply, uint256 cappedBorrow) = _capitalForScore(
    xvsBalanceForScore,
    borrow,
    supply,
    market
);

In this code, the _capitalForScore function is called to calculate capital, cappedSupply, and cappedBorrow, but there's no subsequent validation or checks performed on these values. The function assumes that _capitalForScore will always provide valid results.

Proof of Concept:

Let's consider a scenario where _capitalForScore encounters an exceptional condition or error and returns unexpected values:

function _capitalForScore(
    uint256 xvsBalanceForScore,
    uint256 borrow,
    uint256 supply,
    address market
) internal pure returns (uint256 capital, uint256 cappedSupply, uint256 cappedBorrow) {
    // Simulate an exceptional condition where xvsBalanceForScore is zero
    if (xvsBalanceForScore == 0) {
        // Return zero values to simulate an error condition
        return (0, 0, 0);
    }

    // Perform calculations normally
    capital = borrow + supply;
    cappedSupply = supply;
    cappedBorrow = borrow;

    return (capital, cappedSupply, cappedBorrow);
}

In all this _capitalForScore, i introduce an exceptional condition where it returns zero values if xvsBalanceForScore is zero. This is just a simple example to illustrate how unexpected or erroneous results can be returned.

Now, let's see how this can affect the estimateAPR function:

function estimateAPR(
    address market,
    address user,
    uint256 borrow,
    uint256 supply,
    uint256 xvsStaked
) external view returns (uint256 supplyAPR, uint256 borrowAPR) {
    uint256 totalScore = markets[market].sumOfMembersScore - interests[market][user].score;

    uint256 xvsBalanceForScore = _xvsBalanceForScore(xvsStaked);

    // Call _capitalForScore, but it returns zero values due to an exceptional condition
    (uint256 capital, uint256 cappedSupply, uint256 cappedBorrow) = _capitalForScore(
        xvsBalanceForScore,
        borrow,
        supply,
        market
    );

    uint256 userScore = Scores.calculateScore(xvsBalanceForScore, capital, alphaNumerator, alphaDenominator);

    totalScore = totalScore + userScore;

    return _calculateUserAPR(market, supply, borrow, cappedSupply, cappedBorrow, userScore, totalScore);
}

In this scenario:

As a result, when _calculateUserAPR is called with these unexpected zero values, it may lead to incorrect APR estimations and potentially affect user rewards, as the calculations are based on invalid data.

This demonstrates why it's essential to implement proper error handling and validation checks after calling _capitalForScore to ensure that the returned values are valid and meet the necessary conditions before proceeding with further calculations in the estimateAPR function.

Tools Used

Vs

Recommended Mitigation Steps

It is crucial to implement proper error handling and validation checks after calling _capitalForScore. This involves checking the returned capital, cappedSupply, and cappedBorrow values to ensure they fall within expected ranges or meet necessary conditions before further calculations or actions are performed in the estimateAPR function.

Assessed type

Invalid Validation

c4-pre-sort commented 1 year ago

0xRobocop marked the issue as low quality report

c4-judge commented 1 year ago

fatherGoose1 marked the issue as unsatisfactory: Invalid