code-423n4 / 2022-06-canto-v2-findings

0 stars 0 forks source link

Underlying asset price oracle for CToken in BaseV1-periphery is inaccuarte #134

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/Plex-Engineer/lending-market-v2/blob/443a8c0fed3c5018e95f3881a31b81a555c42b2d/contracts/Stableswap/BaseV1-periphery.sol#L489

Vulnerability details

Impact

Detailed description of the impact of this finding.

Underlying asset price oracle for CToken in BaseV1-periphery is inaccuarte

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

    function getUnderlyingPrice(CToken ctoken) external override view returns(uint price) {
        IBaseV1Pair pair;
        uint8 stable;
        bool stablePair;
        address underlying;

        if (compareStrings(ctoken.symbol(), "cCANTO")) {
            stable = 0;
            underlying = address(wcanto);
        } 
        //set price statically to 1 when the Comptroller is retrieving Price
        else if (compareStrings(ctoken.symbol(), "cNOTE") && msg.sender == Comptroller) {
            return 1; // Note price is fixed to 1
        }

we should not be return 1. 1 is 1 wei. we should be 10 ** 18

Tools Used

VIM

Recommended Mitigation Steps

we can return 10 ** 18

GalloDaSballo commented 2 years ago

The warden has shown what probably is a developer mistake, which will scale down the price of the cNOTE token to 1.

The sponsor confirms.

It should be noted that if cNOTE always returns 1e18 then the math for diff will always be zero, meaning the interest will exclusively be dictated by baseRatePerYear

Because the sponsor confirms, and because the comments point to values "scaled by 1e18" I believe the finding to be valid, and since the "math is wrong", I do agree with High Severity