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

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

Poor naming of variable isStalePrice #49

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

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

Github username: @chewonithard Submission hash (on-chain): 0x04a260b17297f7f32568f55d62e495b35db8f4fcbbeadc031e0a10acc029762c Severity: low

Description: Description\ In CvgOracle.sol, _postTreatmentAndVerifyEth() uses bool isStalePrice that returns true if lastUpdateDate + oracleParams.maxLastUpdateAggregator > block.timestamp i.e. price is not stale. The variable should be renamed to isNotStalePrice to better reflect its meaning and prevent future bugs/mistakes by the team.

if (isEthPriceRelated) {
    (
    uint256 ethPrice,
    ,
    bool isOracleNotToLow,
    bool isOracleNotToHigh,
    ,
    bool isStalePrice // @audit reversed logic, should rename
    ) = _getOracleAndAggregatorPrices(WETH);
    isEthVerified = isOracleNotToLow && isOracleNotToHigh && isStalePrice;
    price = (price * ethPrice) / 10 ** 18;
}

Attack Scenario\ An serious error could be introduced if a dev checks for false instead of true

Recommendation\ Rename to isNotStalePrice P.S. should also fix isOracleNotToLow and isOracleNotToHigh for typo to isOracleNotTooLow and isOracleNotTooHigh

shalbe-cvg commented 1 year ago

Hello, Thanks a lot for your attention.

This naming error has already been reported in a previous issue.

We have so to consider this issue as Invalid.