Price feed in MCAGRateFeed#getRate is not sufficiently validated and can return stale price

[code423n4]

Lines of code

https://github.com/code-423n4/2023-02-kuma/blob/3f3d2269fcb3437a9f00ffdd67b5029487435b95/src/kuma-protocol/MCAGRateFeed.sol#L75-L97

Vulnerability details

Impact

MCAGRateFeed#getRate may return stale data

Proof of Concept

    (, int256 answer,,,) = oracle.latestRoundData();

Classic C4 issue. getRate only uses answer but never checks the freshness of the data, which can lead to stale bond pricing data. Stale pricing data can lead to bonds being bought and sold on KUMASwap that otherwise should not be available. This would harm KIBToken holders as KUMASwap may accept bond with too low of a coupon and reduce rewards.

Tools Used

Manual Review

Recommended Mitigation Steps

Validate that updatedAt has been updated recently enough:

-   (, int256 answer,,,) = oracle.latestRoundData();
+   (, int256 answer,,updatedAt,) = oracle.latestRoundData();


+   if (updatedAt < block.timestamp - MAX_DELAY) {
+       revert();
+   }
    
    if (answer < 0) {
        return _MIN_RATE_COUPON;
    }

[GalloDaSballo]

The Warden has shown how, the system will not check for a stale price

In times of high network congestion, liquidations or if the feed has any downtime, the protocol may end-up using a stale price which can leak value.

For these reasons, I agree with Medium Severity

[c4-judge]

GalloDaSballo marked the issue as selected for report

[c4-sponsor]

m19 marked the issue as disagree with severity

[m19]

This issue is tricky: central bank rate updates are not expected more than a handful of times a year, at most. We failed to document this properly and the lack of a fresh updatedAt check was on purpose.

If we were only transmitting rates when an actual rate change happens, it's impossible to determine how "fresh" the data should be, a central bank rate change could happen every 3 months or twice a year or any timeframe really. So should the staleness check be 90 days, 180 days etc?

We agree that the oracle transmitter could just repeat the current rate on a daily or weekly basis instead making a stale data check useful again.

Because central bank rates are not volatile we still disagree with the severity in this case.

[akshaysrivastav]

Did the discussion conclude on this?

Also do the sponsors themselves manage the MCAG feeds? If yes, then this issue won't be as severe as exluding the updatedAt timestamp for an external chainlink price feed. Highly unlikely that the sponsors will stale the feed but keep the rest of the protocol active.

@GalloDaSballo @m19

[GalloDaSballo]

I empathize with the Sponsor's answer and agree that in a way the maximum delay for the check is hard to determine, however, it's important that we acknowledge that the smart contract cannot "defend itself" unless we offer some sort of constraint that avoids it leaking value incorrectly.

Will consult with a colleague although I believe that the finding is consistent with the definition of Medium Severity

[GalloDaSballo]

In contrast to the front-run, which can be viewed as a nuance / technicality;

The lack of a check would allow the bond to be sold for a time that is not intended, this could also have KumaSwap accept bonds for which the yield is no longer competitive

I believe the observation from the sponsor to be valid the changes may be few and far between, however, because the contract relies on the oracle for it's decision making, and no check is there to ensure that the decision "makes sense";

Given the possibility of:

• Unintended behaviour (selling of bond when that should not be possible)
• Loss of yield as a consequence

I believe Medium Severity to be the more appropriate of the options

[GalloDaSballo]

The warden has added a comment in terms of suggested mitigation:

We agree that the oracle transmitter could just repeat the current rate on a daily or weekly basis instead making a stale data check useful again.

To quote the sponsor I believe this would be the best course of action. Even thought the rate does not change frequently, its valuable to have a heartbeat to confirm that all systems are working as intended. Better to experience potential downtime for buys/sells than to have systems unknowingly down during an important rate change.