sherlock-audit / 2023-10-mzero-judging

3 stars 2 forks source link

Drynooo - Users could face hefty fines #39

Closed sherlock-admin3 closed 5 months ago

sherlock-admin3 commented 5 months ago

Drynooo

medium

Users could face hefty fines

Summary

If updateInterval starts as 0 and then changes to something other than 0, the user may face a huge fine.

Vulnerability Detail

Since updateInterval is 0 at the beginning, users have no incentive to updateCollateral. When updateInterval becomes non-0, users who do not update assets for a long time will face huge penalties.

    function _getMissedCollateralUpdateParameters(
        uint40 lastUpdateTimestamp_,
        uint40 lastPenalizedUntil_,
        uint32 updateInterval_
    ) internal view returns (uint40 missedIntervals_, uint40 missedUntil_) {
        uint40 penalizeFrom_ = UIntMath.max40(lastUpdateTimestamp_, lastPenalizedUntil_);

        // If brand new minter or `updateInterval_` is 0, then there is no missed interval charge at all.
        if (lastUpdateTimestamp_ == 0 || updateInterval_ == 0) return (0, penalizeFrom_);

        uint40 timeElapsed_ = uint40(block.timestamp) - penalizeFrom_;

        if (timeElapsed_ < updateInterval_) return (0, penalizeFrom_);

        missedIntervals_ = timeElapsed_ / updateInterval_;

        // NOTE: Cannot really overflow a `uint40` since `missedIntervals_ * updateInterval_ <= timeElapsed_`.
        missedUntil_ = penalizeFrom_ + (missedIntervals_ * updateInterval_);
    }

Impact

Users could face hefty fines

Code Snippet

https://github.com/sherlock-audit/2023-10-mzero/blob/f567348da0505ba2671224898c69f2d37e60cdc2/protocol/src/MinterGateway.sol#L897

Tool used

Manual Review

Recommendation

The user's penalty should be settled before updating updateInterval.

Duplicate of #79