Dliteofficial - `_maxAge()` might cause transactions to revert due to staleness check in `OracleModule`

[sherlock-admin]

Dliteofficial

medium

_maxAge() might cause transactions to revert due to staleness check in OracleModule

Summary

A low maxAge value might cause transactions in StableModule and LeverageModule to revert for the failure to meet the staleness check in OracleModule::_getPrice()

Vulnerability Detail

During the execution of an announced deposit order in StableModule::executeDeposit(), the function checks the time passed since executionAtTime. The result of this call, maxAge, is used in the staleness check in OracleModule::_getPrice(). The issue here is that when maxAge is less than onchainOracle.maxAge and offchainOracle.maxAge, the staleness check in _getOnchainPrice() and _getOffchainPrice() will pass but the one in OracleModule::_getPrice() (see code snippet) will fail. The same applies to the others in StableModule and LeverageModule

Impact

executeOrder transactions will fail on execution due to the failure of maxAge to meet the staleness check

Code Snippet

StableModule::executeDeposit()

    function executeDeposit(
        address _account,
        uint64 _executableAtTime,
        FlatcoinStructs.AnnouncedStableDeposit calldata _announcedDeposit
    ) external whenNotPaused onlyAuthorizedModule returns (uint256 _liquidityMinted) {
        uint256 depositAmount = _announcedDeposit.depositAmount;

        uint32 maxAge = _getMaxAge(_executableAtTime);

        _liquidityMinted = (depositAmount * (10 ** decimals())) / stableCollateralPerShare(maxAge);

        if (_liquidityMinted < _announcedDeposit.minAmountOut)
            revert FlatcoinErrors.HighSlippage(_liquidityMinted, _announcedDeposit.minAmountOut);

        _mint(_account, _liquidityMinted);

        vault.updateStableCollateralTotal(int256(depositAmount));

        if (totalSupply() < MIN_LIQUIDITY)
            revert FlatcoinErrors.AmountTooSmall({amount: totalSupply(), minAmount: MIN_LIQUIDITY});

        IPointsModule pointsModule = IPointsModule(vault.moduleAddress(FlatcoinModuleKeys._POINTS_MODULE_KEY));
        pointsModule.mintDeposit(_account, _announcedDeposit.depositAmount);

        emit FlatcoinEvents.Deposit(_account, depositAmount, _liquidityMinted);
    }

OracleModule::_getPrice()

    function _getPrice(uint32 maxAge) internal view returns (uint256 price, uint256 timestamp) {

        ...

        if (maxAge < type(uint32).max && timestamp + maxAge < block.timestamp) {
            revert FlatcoinErrors.PriceStale(
                offchain ? FlatcoinErrors.PriceSource.OffChain : FlatcoinErrors.PriceSource.OnChain
            );
        }
    }

Tool used

Manual Review

Recommendation

Consider removing _getMaxAge()

StableModule::executeDeposit()

    function executeDeposit(
        address _account,
        uint64 _executableAtTime,
        FlatcoinStructs.AnnouncedStableDeposit calldata _announcedDeposit
    ) external whenNotPaused onlyAuthorizedModule returns (uint256 _liquidityMinted) {
        uint256 depositAmount = _announcedDeposit.depositAmount;

-        uint32 maxAge = _getMaxAge(_executableAtTime);
        _liquidityMinted = (depositAmount * (10 ** decimals())) / stableCollateralPerShare(maxAge);

        if (_liquidityMinted < _announcedDeposit.minAmountOut)
            revert FlatcoinErrors.HighSlippage(_liquidityMinted, _announcedDeposit.minAmountOut);

        _mint(_account, _liquidityMinted);

        vault.updateStableCollateralTotal(int256(depositAmount));

        if (totalSupply() < MIN_LIQUIDITY)
            revert FlatcoinErrors.AmountTooSmall({amount: totalSupply(), minAmount: MIN_LIQUIDITY});

        IPointsModule pointsModule = IPointsModule(vault.moduleAddress(FlatcoinModuleKeys._POINTS_MODULE_KEY));
        pointsModule.mintDeposit(_account, _announcedDeposit.depositAmount);

        emit FlatcoinEvents.Deposit(_account, depositAmount, _liquidityMinted);
    }

Duplicate of #170

[sherlock-admin]

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

takarez commented:

valid: medium(10)

[securitygrid]

Escalate This isssue is not dup of #170 due to insufficient proof.

[sherlock-admin2]

Escalate This isssue is not dup of #170 due to insufficient proof.

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.

[Evert0x]

Planning to reject and keep issue state as is. I believe it's clear that the report is describing the same core issue as described in #170

[nevillehuang]

Agree with @Evert0x

[Czar102]

Result: Medium Duplicate of #170

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• securitygrid: rejected