code-423n4 / 2024-03-abracadabra-money-findings

9 stars 7 forks source link

Denial of Service #243

Closed c4-bot-2 closed 5 months ago

c4-bot-2 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/staking/LockingMultiRewards.sol#L397

Vulnerability details

Impact

Detailed description of the impact of this finding.

Denial of Service: processExpiredLocks could be blocked by a single user with an expired lock, preventing others from processing.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

function processExpiredLocks(address[] memory users, uint256[] calldata lockIndexes) external onlyOperators { if (users.length != lockIndexes.length) { revert ErrLengthMismatch(); }

    _updateRewardsForUsers(users);

    // Release all expired users' locks
    for (uint256 i; i < users.length; ) {
        address user = users[i];
        Balances storage bal = _balances[user];
        LockedBalance[] storage locks = _userLocks[user];

        if (locks.length == 0) {
            revert ErrNoLocks();
        }

        uint256 index = lockIndexes[i];

        // Prevents processing `lastLockIndex` out of order
        if (index == lastLockIndex[user] && locks.length > 1) {
            revert ErrInvalidLockIndex();
        }

        // prohibit releasing non-expired locks
  @>      if (locks[index].unlockTime > block.timestamp) {
            revert ErrLockNotExpired();
        }

        uint256 amount = locks[index].amount;
        uint256 lastIndex = locks.length - 1;

        /// Last lock index changed place with the one we just swapped.
        if (lastLockIndex[user] == lastIndex) {
            lastLockIndex[user] = index;
        }

        if (index != lastIndex) {
            locks[index] = locks[lastIndex];
            emit LogLockIndexChanged(user, lastIndex, index);
        }

        locks.pop();

        unlockedSupply += amount;
        lockedSupply -= amount;

        bal.unlocked += amount;
        bal.locked -= amount;

        emit LogUnlocked(user, amount, index);

        unchecked {
            ++i;
        }
    }
}

Tools Used

Recommended Mitigation Steps

we should call each function individual so that revert cannot cause DOS.

Assessed type

Context

0xm3rlin commented 6 months ago

as far as I am aware this is a standard inclusion

c4-pre-sort commented 6 months ago

141345 marked the issue as insufficient quality report

141345 commented 6 months ago

lack detailed POC

0xCalibur commented 6 months ago

This function is handle by an authorized gelato operator and will make sure the call is not reverting when it's called.

c4-judge commented 5 months ago

thereksfour marked the issue as unsatisfactory: Insufficient quality

c4-sponsor commented 5 months ago

0xCalibur (sponsor) disputed