sherlock-audit / 2023-12-notional-update-5-judging

7 stars 7 forks source link

bin2chen - getOracleData() maxExternalDeposit not accurate #49

Open sherlock-admin2 opened 7 months ago

sherlock-admin2 commented 7 months ago

bin2chen

medium

getOracleData() maxExternalDeposit not accurate

Summary

in getOracleData() The calculation of maxExternalDeposit lacks consideration for reserve.accruedToTreasury. This leads to maxExternalDeposit being too large, causing Treasury.rebalance() to fail.

Vulnerability Detail

in getOracleData()

    function getOracleData() external view override returns (OracleData memory oracleData) {
...
        (/* */, uint256 supplyCap) = IPoolDataProvider(POOL_DATA_PROVIDER).getReserveCaps(underlying);
        // Supply caps are returned as whole token values
        supplyCap = supplyCap * UNDERLYING_PRECISION;
        uint256 aTokenSupply = IPoolDataProvider(POOL_DATA_PROVIDER).getATokenTotalSupply(underlying);

        // If supply cap is zero, that means there is no cap on the pool
        if (supplyCap == 0) {
            oracleData.maxExternalDeposit = type(uint256).max;
        } else if (supplyCap <= aTokenSupply) {
            oracleData.maxExternalDeposit = 0;
        } else {
            // underflow checked as consequence of if / else statement
@>          oracleData.maxExternalDeposit = supplyCap - aTokenSupply;
        }

However, AAVE's restrictions are as follows: ValidationLogic.sol#L81-L88

    require(
      supplyCap == 0 ||
        ((IAToken(reserveCache.aTokenAddress).scaledTotalSupply() +
          uint256(reserve.accruedToTreasury)).rayMul(reserveCache.nextLiquidityIndex) + amount) <=
        supplyCap * (10 ** reserveCache.reserveConfiguration.getDecimals()),
      Errors.SUPPLY_CAP_EXCEEDED
    );
  }

The current implementation lacks subtraction of uint256(reserve.accruedToTreasury)).rayMul(reserveCache.nextLiquidityIndex).

Impact

An overly large maxExternalDeposit may cause rebalance() to be unable to execute.

Code Snippet

https://github.com/sherlock-audit/2023-12-notional-update-5/blob/main/contracts-v3/contracts/external/pCash/AaveV3HoldingsOracle.sol#L160

Tool used

Manual Review

Recommendation

subtract uint256(reserve.accruedToTreasury)).rayMul(reserveCache.nextLiquidityIndex)

sherlock-admin2 commented 7 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

valid because { valid }

sherlock-admin commented 7 months ago

The protocol team fixed this issue in PR/commit https://github.com/notional-finance/contracts-v3/pull/31.

sherlock-admin commented 7 months ago

The Lead Senior Watson signed-off on the fix.