Skip to content

Latest commit

 

History

History
73 lines (52 loc) · 3.44 KB

045.md

File metadata and controls

73 lines (52 loc) · 3.44 KB
Raw

Jolly Mauve Parakeet

Medium

staleAfter doesn't behave as it is supposed to.

Summary

According to the dev description and protocols docs, there should be > instead of >= when using staleAfter

Root Cause

In readme it states that invariants in natspec comments should hold

Yes - function behavior is defined in the natspec comments and if they pose integration risk we would like to be aware of that.

According to dev description update may still be called at staleAfter time, but its not

/// @dev The maximum amount of time since the latest oracle version that update may still be called
    uint256 staleAfter;

packages/perennial/contracts/types/RiskParameter.sol#L47

        if (
            !(updateContext.currentPositionLocal.magnitude().isZero() && context.latestPositionLocal.magnitude().isZero()) &&       // sender has no position
            !(newOrder.isEmpty() && newOrder.collateral.gte(Fixed6Lib.ZERO)) &&                                                     // sender is depositing zero or more into account, without position change
            (
                !context.latestOracleVersion.valid ||
                context.currentTimestamp - context.latestOracleVersion.timestamp >= context.riskParameter.staleAfter
            )                                                                                                                       // price is not stale
        ) revert IMarket.MarketStalePriceError();

contracts/libs/InvariantLib.sol#L57

In protocol docs its also >

Since oracle prices are only pushed upon request, account updates (position / collateral) are blocked when the market is stale: oracle.currentTimestamp - oracle.latestTimetamp > staleAfter

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

  1. broken assumptions about function behavior

  2. Currently staleAfter value set to 40, so difference in 1 seems to be medium severity, the protocol would need to submit prices every 41 seconds instead of every 40.

PoC

No response

Mitigation

        if (
            !(updateContext.currentPositionLocal.magnitude().isZero() && context.latestPositionLocal.magnitude().isZero()) &&       // sender has no position
            !(newOrder.isEmpty() && newOrder.collateral.gte(Fixed6Lib.ZERO)) &&                                                     // sender is depositing zero or more into account, without position change
            (
                !context.latestOracleVersion.valid ||
-                context.currentTimestamp - context.latestOracleVersion.timestamp >= context.riskParameter.staleAfter
+                context.currentTimestamp - context.latestOracleVersion.timestamp > context.riskParameter.staleAfter
            )                                                                                                                       // price is not stale
        ) revert IMarket.MarketStalePriceError();