KupiaSec - Not decreasing oracle timestamp validation leads to DoS for protocol users

[sherlock-admin2]

KupiaSec

Medium

Not decreasing oracle timestamp validation leads to DoS for protocol users

Summary

The protocol only allows equal or increased timestamp of oracle prices whenever an action happens in the protocol.
This validation is wrong since it will lead to DoS for users.

Vulnerability Detail

The protocol uses RedStone oracle, where token prices are added as a part of calldata of transactions.
In RedStone oracle, it allows prices from 3 minutes old upto 1 minute in the future, as implemented in RedstoneDefaultsLib.sol.

@internal
def extract_price(
    quote_decimals: uint256,
    payload       : Bytes[224]
) -> uint256:
  price: uint256 = 0
  ts   : uint256 = 0
  (price, ts) = self.EXTRACTOR.extractPrice(self.FEED_ID, payload)

  # Redstone allows prices ~10 seconds old, discourage replay attacks
  assert ts >= self.TIMESTAMP, "ERR_ORACLE"
  self.TIMESTAMP = ts

In oracle.vy, it extracts the token price from the RedStone payload, which also includes the timestamp of which the prices were generated.
As shown in the code snippet, the protocol reverts when the timestamp extracted from the calldata is smaller than the stored timestamp, thus forcing timestamps only increase or equal to previous one.
This means that the users who execute transaction with price 1 minute old gets reverted when there is another transaction executed with price 30 seconds old.

NOTE: The network speed of all around the world is not same, so there can be considerable delays based on the location, api availability, etc.

By abusing this vulnerability, an attacker can regularly make transactions with newest prices which will revert all other transactions with slightly older price data like 10-20 seconds older, can be all reverted.

Impact

The vulnerability causes DoS for users who execute transactions with slightly older RedStone oracle data.

Code Snippet

https://github.com/sherlock-audit/2024-08-velar-artha/blob/18ef2d8dc0162aca79bd71710f08a3c18c94a36e/gl-sherlock/contracts/oracle.vy#L91-L93

Tool used

Manual Review

Recommendation

It's recommended to remove that non-decreasing timestamp validation.
If the protocol wants more strict oracle price validation than the RedStone does, it can just use the difference between oracle timestamp and current timestamp.

[KupiaSecAdmin]

Escalate

Information required for this issue to be rejected.

[sherlock-admin3]

Escalate

Information required for this issue to be rejected.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[rickkk137]

the protocol for every action fetch a redstone's payload and attach that to the transaction let's assume:
User A:fetch payload a in T1
user B:fetch payload b in T2
user C:fetch payload c in T3
and T3 > T2 > T1 and block.timestamp-[T1,T2,T3] < 3 minutes(I mean all of them are valid)
if all of them send their Txs to the network sequence is very important its mean just with this sequence [t1,t2,t3] none of TXs wouldn't be reverted,
if t2 will be executed first then t1 will be reverted and if t3 will be executed first t1 and t2 will be reverted

 contract RedstoneExtractor is PrimaryProdDataServiceConsumerBase {
+  uint lastTimeStamp;
+  uint price;
   function extractPrice(bytes32 feedId, bytes calldata)
       public view returns(uint256, uint256)
   {
+    if(block.timestamp - lastTimeStamp > 3 minutes){
     bytes32[] memory dataFeedIds = new bytes32[](1);
     dataFeedIds[0] = feedId;
     (uint256[] memory values, uint256 timestamp) =
         getOracleNumericValuesAndTimestampFromTxMsg(dataFeedIds);
     validateTimestamp(timestamp); //!!!
-    return (values[0], timestamp);
+    lastTimeStamp = block.timestamp;
+    price = values[0];
+    }
+    return (price, lastTimeStamp);
   }
 }

But there isn't loss of funds ,users can repeat their TXs

[KupiaSecAdmin]

@rickkk137 - Thanks for providing the PoC. Of course there's no loss of funds, and users can repeat their transactions but those can be reverted again. Overall, there's pretty high chance that users' transactions will bereverted.

[rickkk137]

I agree with u and this can be problematic and Here's an example of this approach but the issue's final result depend on sherlock rules

[WangSecurity]

Not sure I understand how this can happen.

For example, we fetched the price from RedStone at timestamp = 10.
Then someone fetches the price again, and for the revert to happen, the timestamp of that price has to be 9, correct?

How could that happen? Is it the user who chooses the price?

[KupiaSecAdmin]

@WangSecurity - As a real-world example, there will be multiple users who get price from RedStone and call Verla protocol with that price data. NOTE: price data(w/ timestamp) is added as calldata for every call.
So among those users, there will be users calling protocol with price timestamp 8, some with 9, some with 10.

If one call with price timestamp 10 is called, other remaining calls with price timestamp 8 and 9 will revert.

[WangSecurity]

Thank you for this clarification. Indeed, this is possible, and it can happen even intentionally. On the other hand, this is only one-block DOS and the users could proceed with the transactions in the next block (assuming they use the newer price). However, this vulnerability affects the opening and closing of positions, which are time-sensitive functions. Hence, this should be a valid medium based on this rule:

Griefing for gas (frontrunning a transaction to fail, even if can be done perpetually) is considered a DoS of a single block, hence only if the function is clearly time-sensitive, it can be a Medium severity issue.

Planning to accept the escalation and validate with medium severity.

[WangSecurity]

Result:
Medium
Unique

[sherlock-admin4]

Escalations have been resolved successfully!

Escalation status:

• KupiaSecAdmin: accepted