hats-finance / Convergence-Finance---IBO-0x0e410e7af8e70fc5bffcdbfbdf1673ee7b3d0777

IBO, Vesting & Bond mecanism repo prepared for Hat finance audit competition
0 stars 0 forks source link

Incorrect Stablecoin Price Calculation #14

Open hats-bug-reporter[bot] opened 1 year ago

hats-bug-reporter[bot] commented 1 year ago

Github username: @aviggiano Submission hash (on-chain): 0x592934313787dc431b0083ec538078d43faf7165399d7a2b582fa49c41152b37 Severity: medium

Description:

Description

In CvgOracle.sol:259, the CvgOracle is not calculating the price of the token when it is a stablecoin. This can be problematic if the stablecoin depegs. For example, UST has dropped from $1 to $0, USDT from $1 to $0.95, and USDC from $0.96.

Attack scenario

An attacker can take advantage of this vulnerability when a stablecoin depegs. As the Oracle is not recalculating the stablecoin's price, it will continue to assume it as 1 USD which is no longer its actual price. This can lead to incorrect financial calculations and potential losses to users.

Proof of Concept

The code snippet where the issue is present:

/**
 *  @notice double security, compare the price computed through pool with Chainlink oracle
 *  @param erc20Address address of the token we want to fetch the price
 */
function _getOracleAndAggregatorPrices(
    IERC20Metadata erc20Address
) internal view returns (uint256, uint256, bool, bool, bool, bool) {
    /// @dev Fetch price through CvgOracle
    IOracleStruct.OracleParams memory oracleParams = oracleParametersPerERC20[erc20Address];
    // @audit-issue CvgOracle is not calculating the price of the token when it is a stablecoin. 
    (uint256 poolOraclePrice, bool isEthVerified) = oracleParams.isStable
        ? (10 ** 18, true)
        : _getPriceOracle(erc20Address);
    (uint256 aggregatorOraclePrice, uint256 lastUpdateDate) = _getPriceAggregator(oracleParams.aggregatorOracle);
    uint256 delta = (oracleParams.deltaAggregatorCvgOracle * poolOraclePrice) / 10_000;

    return (
        poolOraclePrice,
        aggregatorOraclePrice,
        poolOraclePrice + delta > aggregatorOraclePrice,
        poolOraclePrice - delta < aggregatorOraclePrice,

The above code is always setting the token's price to 1 for stablecoins no matter the actual market price.

Recommendation

Use a price feed aggregator for all tokens, including stablecoins, to avoid inaccuracy in price which can potentially lead to financial misrepresentations and losses for users.

aviggiano commented 1 year ago

Typo

For example, UST has dropped from $1 to $0, USDT from $1 to $0.95, and USDC from $1 to $0.96.

walk-on-me commented 1 year ago

Hello, Thanks a lot for your attention.

Your assummption is right if only we were'nt using a check on the price feed returned by Chainlink. We mitigate this risk thanks to the deltaAggregatorCvgOracle. In fact, a low % on the deltaAggregator at 0.4% implies that the deposit on the bond will fail if the Chainlink Aggregator returns a price > 1.004$ or price < 0.996$. We have so to consider this issue as Invalid