When Secondary Offchain Oracle is Invalid, Primary Onchain Will Be Broken Too

Summary

When the secondary offchain oracle is inaccessible or the price is not valid, the primary oracle will also not be available.

Vulnerability Detail

The source code intends to use the price from the primary onchain oracle when the offchain price is invalid. This is evident from the logic within the _getPrice function.

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/OracleModule.sol#L115-L128

if (offchainInvalid == false) {
    // return the freshest price
    if (offchainTime >= onchainTime) {
        price = offchainPrice;
        timestamp = offchainTime;
        offchain = true;
    } else {
        price = onchainPrice;
        timestamp = onchainTime;
    }
} else {
    price = onchainPrice;
    timestamp = onchainTime;
}

However, when offchainInvalid == false, the offchainPrice will be zero, which can cause a revert in the following code block due to a price mismatch:

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/OracleModule.sol#L111-L113

uint256 priceDiff = (int256(onchainPrice) - int256(offchainPrice)).abs();
uint256 diffPercent = (priceDiff * 1e18) / onchainPrice;
if (diffPercent > maxDiffPercent) revert FlatcoinErrors.PriceMismatch(diffPercent);

Btw the issue was not covered by testing due to disable the price difference check for easier testing: https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/test/helpers/Setup.sol#L180

Impact

An invalid price in the secondary oracle can disrupt the price fetch process. This is a medium severity issue because it affects core functionality.

Code Snippet

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/OracleModule.sol#L113

Tool used

Manual Review

Recommendation

Modify the code to compare the onchain and offchain price differences only when the offchain price is valid:

diff --git a/flatcoin-v1/src/OracleModule.sol b/flatcoin-v1/src/OracleModule.sol
index 128071a..04a8315 100644
--- a/flatcoin-v1/src/OracleModule.sol
+++ b/flatcoin-v1/src/OracleModule.sol
@@ -108,11 +108,12 @@ contract OracleModule is IOracleModule, ModuleUpgradeable, ReentrancyGuardUpgrad
         (uint256 offchainPrice, uint256 offchainTime, bool offchainInvalid) = _getOffchainPrice();
         bool offchain;

-        uint256 priceDiff = (int256(onchainPrice) - int256(offchainPrice)).abs();
-        uint256 diffPercent = (priceDiff * 1e18) / onchainPrice;
-        if (diffPercent > maxDiffPercent) revert FlatcoinErrors.PriceMismatch(diffPercent);
-
         if (offchainInvalid == false) {
+
+            uint256 priceDiff = (int256(onchainPrice) - int256(offchainPrice)).abs();
+            uint256 diffPercent = (priceDiff * 1e18) / onchainPrice;
+            if (diffPercent > maxDiffPercent) revert FlatcoinErrors.PriceMismatch(diffPercent);
+
             // return the freshest price
             if (offchainTime >= onchainTime) {
                 price = offchainPrice;

Duplicate of #177

sherlock-admin commented 5 months ago

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

takarez commented:

valid: medium(2)