Farseen0/scar-pairs huggingface dataset:
HIGH severity: Hardcoded cluster size in `withdrawValidator` can cause losses to operators or protocol for strategies with larger cluster sizes. **Description:** `CasimirManager::withdrawValidator` calculates the owed balance on withdrawal, ie. shortfall from the initial 32 ether. It then tries to recover the owed amount from the operators tagged to the validator. However, while calculating recovery amount, a hardcoded cluster size of `4`...

Vulnerable code:
function withdrawValidator(
    uint256 stakedValidatorIndex,
    WithdrawalProofs memory proofs,
    ISSVClusters.Cluster memory cluster
) external {
    onlyReporter();

    // ... more code

    uint256 owedAmount = VALIDATOR_CAPACITY - finalEffectiveBalance;
    if (owedAmount > 0) {
        uint256 availableCollateral = registry.collateralUnit() * 4;
        owedAmount = owedAmount > availableCollateral ? availableCollateral : owedAmount;
>       uint256 recoverAmount = owedAmount / 4; //@audit hardcoded operator size
        for (uint256 i; i < validator.operatorIds.length; i++) {
            registry.removeOperatorValidator(validator.operatorIds[i], validatorId, recoverAmount);
        }
    }

    // .... more code
HIGH severity: A malicious staker can force validator withdrawals by instantly staking and unstaking. **Description:** When a user unstakes via `CasimirManager::requestUnstake`, the number of required validator exits is calculated using the prevailing expected withdrawable balance as follows:

Consider the following simplified scenario:

`unAssignedBalance = 31 ETH withdrawnBalance = 0...

Vulnerable code:
function requestUnstake(uint256 amount) external nonReentrant {
    // code ....
    uint256 expectedWithdrawableBalance =
        getWithdrawableBalance() + requestedExits * VALIDATOR_CAPACITY + delayedEffectiveBalance;
    if (unstakeQueueAmount > expectedWithdrawableBalance) {
        uint256 requiredAmount = unstakeQueueAmount - expectedWithdrawableBalance;
>       uint256 requiredExits = requiredAmount / VALIDATOR_CAPACITY; //@audit required exits calculated here
        if (requiredAmount % VALIDATOR_CAPACITY > 0) {
            requiredExits++;
        }
        exitValidators(requiredExits);
    }

    emit UnstakeRequested(msg.sender, amount);
}
HIGH severity: Operator is not removed in Registry when validator has `owedAmount == 0`. **Description:** `CasimirManager::withdrawValidator()` function is designed to remove a validator after a full withdrawal. It checks whether the final effective balance of the removed validator is sufficient to cover the initial 32 ETH deposit. If for some reason such as slashing, the final...

Vulnerable code:
uint256 owedAmount = VALIDATOR_CAPACITY - finalEffectiveBalance;
if (owedAmount > 0) {
    uint256 availableCollateral = registry.collateralUnit() * 4;
    owedAmount = owedAmount > availableCollateral ? availableCollateral : owedAmount;
    uint256 recoverAmount = owedAmount / 4;
    for (uint256 i; i < validator.operatorIds.length; i++) {
        // @audit if owedAmount == 0, this function is not called
        registry.removeOperatorValidator(validator.operatorIds[i], validatorId, recoverAmount);
    }
}
HIGH severity: Accounting for `rewardStakeRatioSum` is incorrect when a delayed balance or rewards are unclaimed. **Description:** The current accounting incorrectly assumes that the delayed effective balance and delayed rewards will be claimed before any new report begins. This is inaccurate as these delayed funds require a few days before they can be claimed. If a new report starts before these funds are...

Vulnerable code:
// Before start
latestActiveBalanceAfterFee = 32 ETH
latestActiveRewards = 0

// startReport()
reportSweptBalance = 0 (rewards is in BeaconChain)

// syncValidators()
reportActiveBalance = 32.105 ETH

// finalizeReport()
rewards = 0.105 ETH
change = rewards - latestActiveRewards = 0.105 ETH
gainAfterFee = 0.1 ETH
=> rewardStakeRatioSum is increased
=> latestActiveBalanceAfterFee = 32.1

sweptRewards = 0
=> latestActiveRewards = 0.105

// Before start
latestActiveBalanceAfterFee = 32.1 ETH
latestActiveRewards = 0.105

// startReport()
reportSweptBalance = 0.105 (rewards is in EigenPod)

// syncValidators()
reportActiveBalance = 32 ETH

// finalizeReport()
rewards = 0.105 ETH
change = rewards - latestActiveRewards = 0
=> No update to rewardStakeRatioSum and latestActiveBalanceAfterFee

sweptRewards = 0.105
=> latestActiveBalanceAfterFee = 32 ETH (subtracted sweptReward without fee)
=> latestActiveRewards = rewards - sweptRewards = 0

// Before start
latestActiveBalanceAfterFee = 32 ETH
latestActiveRewards = 0

// startReport()
reportSweptBalance = 0.105 (No action happens so rewards is still in EigenPod)

// syncValidators()
reportActiveBalance = 32 ETH

// finalizeReport()
rewards = 0.105 ETH
change = rewards - latestActiveRewards = 0.105
=> rewardStakeRatioSum is increased
=> latestActiveBalanceAfterFee = 32.1

sweptRewards = 0.105
=> latestActiveBalanceAfterFee = 32 ETH (subtracted sweptReward without fee)
=> latestActiveRewards = rewards - sweptRewards = 0
HIGH severity: Incorrect accounting of `reportRecoveredEffectiveBalance` can prevent report from being finalized when a validator is slashed. **Description:** When a validator is slashed, a loss is incurred. In the `finalizeReport()` function, the `rewardStakeRatioSum` and `latestActiveBalanceAfterFee` variables are reduced to reflect this loss. The change could be positive if the rewards are larger than the slashed amount, but for...

Vulnerable code:
} else if (change < 0) {
    uint256 loss = uint256(-change);
    rewardStakeRatioSum -= Math.mulDiv(rewardStakeRatioSum, loss, totalStake);
    latestActiveBalanceAfterFee -= loss;
}

_Report Period 1_
Rewards: 0.1 per validator on BC.
Withdrawal: 32
Unstake request: 15

_Report Period 2_
unstake request: 20
last validator exited with slashing of 0.2
MEDIUM severity: Infinite loop in the `exitValidators()` prevents users from calling `requestUnstake()`. **Description:** When users call function `requestUnstake()` to request to unstake their ETH, the `CasimirManager` contract will calculate if the current expected withdrawable balance is enough to cover all the queued unstaking requests. If it is not enough, the function `exitValidators()` will be.....

Vulnerable code:
function exitValidators(uint256 count) private {
    uint256 index = 0;
    while (count > 0) {
        uint32 validatorId = stakedValidatorIds[index];
        Validator storage validator = validators[validatorId];

        // @audit if status != ACTIVE, count and index won't be updated => Infinite loop
        if (validator.status == ValidatorStatus.ACTIVE) {
            count--;
            index++;
            requestedExits++;
            validator.status = ValidatorStatus.EXITING;
            ssvClusters.exitValidator(validator.publicKey, validator.operatorIds);
            emit ValidatorExited(validatorId);
        }
    }
}
MEDIUM severity: Multiple unstake requests can cause denial of service because withdrawn balance is not adjusted after every unstake request is fulfilled. **Description:** A `while` loop runs over a specific number of unstake requests, and in every iteration, it checks if an unstake request is fulfillable. If it is, the unstaked amount is transferred back to the staker who requested unstaking. Honoring every unstaking request reduces the effective...

Vulnerable code:
function fulfillUnstakes(uint256 count) external {
    //@note called when report status is in fulfilling unstakes
    onlyReporter();

    if (reportStatus != ReportStatus.FULFILLING_UNSTAKES) {
        revert ReportNotFulfilling();
    } //@note ok => report has to be in this state

    uint256 unstakedAmount;
    while (count > 0) {
        count--;

>       (Unstake memory unstake, bool fulfillable) = getNextUnstake(); //@audit uses the stale withdrawn balance
        if (!fulfillable) {
            break;
        }

        unstakeQueue.remove(0);
>       unstakedAmount += unstake.amount; //@audit unstakedAmount is increased here
>       fulfillUnstake(unstake.userAddress, unstake.amount); //@audit even after ETH is transferred, withdrawn balance is same
    }

    (, bool nextFulfillable) = getNextUnstake();
    if (!nextFulfillable) {
        reportStatus = ReportStatus.FINALIZING;
    }

>   if (unstakedAmount <= withdrawnEffectiveBalance) { //@audit withdrawn balance and unassigned balance adjustment happens here
        withdrawnEffectiveBalance -= unstakedAmount;
    } else {
        uint256 remainder = unstakedAmount - withdrawnEffectiveBalance;
        withdrawnEffectiveBalance = 0;
        unassignedBalance -= remainder;
    }

    unstakeQueueAmount -= unstakedAmount;
}
MEDIUM severity: Users could avoid loss by frontrunning to request unstake. **Description:** A loss can occur when a validator is slashed, which is reflected in the `finalizeReport()` function. If the `change` is less than 0, this indicates that a loss has occurred. Consequently, the accounting updates the `rewardStakeRatioSum` to decrease the stake value of all users in...

Vulnerable code:
} else if (change < 0) {
    uint256 loss = uint256(-change);
    rewardStakeRatioSum -= Math.mulDiv(rewardStakeRatioSum, loss, totalStake);
    latestActiveBalanceAfterFee -= loss;
}

function getNextUnstake() public view returns (Unstake memory unstake, bool fulfillable) {
    // @audit Allow to create and fulfill unstake within the same `reportPeriod`
    if (unstakeQueue.length > 0) {
        unstake = unstakeQueue[0];
        fulfillable = unstake.period <= reportPeriod && unstake.amount <= getWithdrawableBalance();
    }
}
LOW severity: Operator can set his operatorID status to active by transferring 0 Wei. **Description:** When withdrawing collateral, logic checks if collateral balance is 0 & makes the operator Id inactive.

However while depositing, there is no check on the amount deposited. An operator can deposit 0 Wei and set operatorID to active. Deposit and withdrawal states are...

Vulnerable code:
function withdrawCollateral(uint64 operatorId, uint256 amount) external {
        onlyOperatorOwner(operatorId);

        Operator storage operator = operators[operatorId];
        uint256 availableCollateral = operator.collateralBalance - operator.validatorCount * collateralUnit; //@note can cause underflow here if validator count > 0
        if (availableCollateral < amount) {
            revert InvalidAmount();
        }

        operator.collateralBalance -= amount;
        if (operator.collateralBalance == 0) {
            operator.active = false;
        }

        (bool success,) = msg.sender.call{value: amount}("");
        if (!success) {
            revert TransferFailed();
        }

        emit CollateralWithdrawn(operatorId, amount);
    }

function depositCollateral(uint64 operatorId) external payable {
        onlyOperatorOwner(operatorId);

        Operator storage operator = operators[operatorId];
        if (!operator.registered) {
            operatorIds.push(operatorId);
            operator.registered = true;
            emit OperatorRegistered(operatorId);
        }
        if (!operator.active) {
>            operator.active = true; //@audit -> can make operator active even with 0 wei
        }
        operator.collateralBalance += msg.value;

        emit CollateralDeposited(operatorId, msg.value);
    }**
LOW severity: Reporter trying to reshare a pending validator will lead to denial of service. **Description:** A validator can reshare an operator if its either in `PENDING` or `ACTIVE` status. When resharing is executed for a validtor in `PENDING` state , the existing operators are removed from the SSV cluster -> however,  no such operators are registered in the first place. This is...

Vulnerable code:
function reshareValidator(
        uint32 validatorId,
        uint64[] memory operatorIds,
        uint64 newOperatorId,
        uint64 oldOperatorId,
        bytes memory shares,
        ISSVClusters.Cluster memory cluster,
        ISSVClusters.Cluster memory oldCluster,
        uint256 feeAmount,
        uint256 minTokenAmount,
        bool processed
    ) external {
        onlyReporter();

        Validator storage validator = validators[validatorId];
        if (validator.status != ValidatorStatus.ACTIVE && validator.status != ValidatorStatus.PENDING) {
            revert ValidatorNotActive();
        }

       // ... code

        uint256 ssvAmount = retrieveFees(feeAmount, minTokenAmount, address(ssvToken), processed);
        ssvToken.approve(address(ssvClusters), ssvAmount);
>        ssvClusters.removeValidator(validator.publicKey, validator.operatorIds, oldCluster); //@audit validtor key is not registered when the validator is in pending state
       ssvClusters.registerValidator(validator.publicKey, operatorIds, shares, ssvAmount, cluster); //@audit new operators registered

        validator.operatorIds = operatorIds;
        validator.reshares++;

        registry.removeOperatorValidator(oldOperatorId, validatorId, 0);
        registry.addOperatorValidator(newOperatorId, validatorId);

        emit ValidatorReshared(validatorId);
    }

function removeValidator(
        bytes calldata publicKey,
        uint64[] memory operatorIds,
        Cluster memory cluster
    ) external override {
        StorageData storage s = SSVStorage.load();

        bytes32 hashedCluster = cluster.validateHashedCluster(msg.sender, operatorIds, s);
        bytes32 hashedOperatorIds = ValidatorLib.hashOperatorIds(operatorIds);

        bytes32 hashedValidator = keccak256(abi.encodePacked(publicKey, msg.sender));
        bytes32 validatorData = s.validatorPKs[hashedValidator];

        if (validatorData == bytes32(0)) {
>            revert ISSVNetworkCore.ValidatorDoesNotExist(); //@audit reverts when no key exists
        }

        if (!ValidatorLib.validateCorrectState(validatorData, hashedOperatorIds))
            revert ISSVNetworkCore.IncorrectValidatorStateWithData(publicKey);

        delete s.validatorPKs[hashedValidator];

        if (cluster.active) {
            StorageProtocol storage sp = SSVStorageProtocol.load();
            (uint64 clusterIndex, ) = OperatorLib.updateClusterOperators(operatorIds, false, false, 1, s, sp);

            cluster.updateClusterData(clusterIndex, sp.currentNetworkFeeIndex());

            sp.updateDAO(false, 1);
        }

        --cluster.validatorCount;

        s.clusters[hashedCluster] = cluster.hashClusterData();

        emit ValidatorRemoved(msg.sender, operatorIds, publicKey, cluster);
    }
LOW severity: Function `withdrawRewards()` may lead to inaccuracy in `delayedRewards` if there's no withdrawal to process. **Description:** In the `CasimirManager`, the `withdrawRewards()` function can be used by the reporter to process swept validator rewards. The reporter must provide `WithdrawalProofs`, which the function uses to call `eigenPod.verifyAndProcessWithdrawals()`.

The `verifyAndProcessWithdrawals()`...

Vulnerable code:
function withdrawRewards(WithdrawalProofs memory proofs) external {
    onlyReporter();

    eigenPod.verifyAndProcessWithdrawals(
        proofs.oracleTimestamp,
        proofs.stateRootProof,
        proofs.withdrawalProofs,
        proofs.validatorFieldsProofs,
        proofs.validatorFields,
        proofs.withdrawalFields
    );

    // @audit Not check if the delayed withdrawal length has changed or not
    uint256 delayedAmount = eigenWithdrawals.userDelayedWithdrawalByIndex(
        address(this), eigenWithdrawals.userWithdrawalsLength(address(this)) - 1
    ).amount;
    delayedRewards += delayedAmount;

    emit RewardsDelayed(delayedAmount);
}

if (withdrawalSummary.amountToSendGwei != 0) {
    _sendETH_AsDelayedWithdrawal(podOwner, withdrawalSummary.amountToSendGwei * GWEI_TO_WEI);
}

uint256 initialDelayedRewardsLength = eigenWithdrawals.userWithdrawalsLength(address(this));
uint64 initialDelayedEffectiveBalanceGwei = eigenPod.withdrawableRestakedExecutionLayerGwei();

eigenPod.verifyAndProcessWithdrawals(
    ...
);

{
    uint256 updatedDelayedRewardsLength = eigenWithdrawals.userWithdrawalsLength(address(this));
    if (updatedDelayedRewardsLength > initialDelayedRewardsLength) {
        IDelayedWithdrawalRouter.DelayedWithdrawal memory withdrawal =
            eigenWithdrawals.userDelayedWithdrawalByIndex(address(this), updatedDelayedRewardsLength - 1);
        if (withdrawal.blockCreated == block.number) {
            delayedRewards += withdrawal.amount;

            emit RewardsDelayed(withdrawal.amount);
        }
    }
}
LOW severity: Incorrect test setup leads to false test outcomes. **Description:** `IntegrationTest.t.sol` includes an integrated test that verifies the entire staking lifecycle. However, the current test setup, in several places, advances the blocks using foundry's `vm.roll` but neglects to adjust the timestamp using `vm.warp`.

This allows the test setup to...

Vulnerable code:
>       vm.roll(block.number + eigenWithdrawals.withdrawalDelayBlocks() + 1); //@audit changing block without changing timestamp
        vm.prank(reporterAddress);
>       manager.claimRewards(); //@audit claiming at the same timestamp

        // Reporter runs after the heartbeat duration
        vm.warp(block.timestamp + 24 hours);
        timeMachine.setProofGenStartTime(0.5 hours);
        beaconChain.setNextTimestamp(timeMachine.proofGenStartTime());
        vm.startPrank(reporterAddress);
        manager.startReport();
        manager.syncValidators(abi.encode(beaconChain.getActiveBalanceSum(), 0));
        manager.finalizeReport();
        vm.stopPrank();
MEDIUM severity: `getUserStake` function failure for non-staker accounts. **Description:** The function `getUserStake` in the smart contract throws an error when invoked for an address that does not have any stakes. Specifically, the function fails due to a division by zero error. This occurs because the divisor, `users[userAddress].rewardStakeRatioSum0`, can be zero if.....

Vulnerable code:
function getUserStake(address userAddress) public view returns (uint256 userStake) {
    userStake = Math.mulDiv(users[userAddress].stake0, rewardStakeRatioSum, users[userAddress].rewardStakeRatioSum0);
}
HIGH severity: `TradingAccount::withdrawMarginUsd` transfers an incorrectly larger amount of margin collateral for tokens with less than 18 decimals. **Description:** The `UD60x18` values are [scaled up to 18 decimal places](https://github.com/zaros-labs/zaros-core-audit/blob/de09d030c780942b70f1bebcb2d245214144acd2/src/perpetuals/leaves/MarginCollateralConfiguration.sol#L46-L50) for collateral tokens with less than 18 decimals places. But when.....

Vulnerable code:
function withdrawMarginUsd(
    Data storage self,
    address collateralType,
    UD60x18 marginCollateralPriceUsdX18,
    UD60x18 amountUsdX18,
    address recipient
)
    internal
    returns (UD60x18 withdrawnMarginUsdX18, bool isMissingMargin)
{
    UD60x18 marginCollateralBalanceX18 = getMarginCollateralBalance(self, collateralType);
    UD60x18 requiredMarginInCollateralX18 = amountUsdX18.div(marginCollateralPriceUsdX18);
    if (marginCollateralBalanceX18.gte(requiredMarginInCollateralX18)) {
        withdraw(self, collateralType, requiredMarginInCollateralX18);

        withdrawnMarginUsdX18 = withdrawnMarginUsdX18.add(amountUsdX18);

        // @audit wrong amount for collateral tokens with less than 18 decimals
        // needs to be scaled down to collateral token's native precision
        IERC20(collateralType).safeTransfer(recipient, requiredMarginInCollateralX18.intoUint256());

        isMissingMargin = false;
        return (withdrawnMarginUsdX18, isMissingMargin);
    } else {
        UD60x18 marginToWithdrawUsdX18 = marginCollateralPriceUsdX18.mul(marginCollateralBalanceX18);
        withdraw(self, collateralType, marginCollateralBalanceX18);
        withdrawnMarginUsdX18 = withdrawnMarginUsdX18.add(marginToWithdrawUsdX18);

        // @audit wrong amount for collateral tokens with less than 18 decimals
        // needs to be scaled down to collateral token's native precision
        IERC20(collateralType).safeTransfer(recipient, marginCollateralBalanceX18.intoUint256());

        isMissingMargin = true;
        return (withdrawnMarginUsdX18, isMissingMargin);
    }
}
HIGH severity: Attacker can perform a risk-free trade to mint free USDz tokens by opening then quickly closing positions for markets using negative `makerFee`. **Description:** Attacker can perform a risk-free trade to mint free USDz tokens by opening then quickly closing positions in markets using negative `makerFee`; this is effectively a free mint exploit dressed up as a risk-free "trade".

**Proof of Concept:** First change...

Vulnerable code:
-    OrderFees.Data internal btcUsdOrderFees = OrderFees.Data({ makerFee: 0.0004e18, takerFee: 0.0008e18 });
+    OrderFees.Data internal btcUsdOrderFees = OrderFees.Data({ makerFee: -0.0004e18, takerFee: 0.0008e18 });

// new import at the top
import {console} from "forge-std/console.sol";

function test_AttackerMintsFreeUSDzOpenThenQuicklyClosePositionMarketNegMakerFee() external {
    // In a market with a negative maker fee, an attacker can perform
    // a risk-free "trade" by opening then quickly closing a position.
    // This allows attackers to mint free USDz without
    // any risk; it is essentially a free mint exploit dressed up
    // as a risk-free "trade"

    // give naruto some tokens
    uint256 USER_STARTING_BALANCE = 100_000e18;
    int128  USER_POS_SIZE_DELTA   = 10e18;
    deal({ token: address(usdToken), to: users.naruto, give: USER_STARTING_BALANCE });

    // naruto creates a trading account and deposits their tokens as collateral
    changePrank({ msgSender: users.naruto });
    uint128 tradingAccountId = createAccountAndDeposit(USER_STARTING_BALANCE, address(usdToken));

    // naruto opens position in BTC market
    openManualPosition(BTC_USD_MARKET_ID, BTC_USD_STREAM_ID, MOCK_BTC_USD_PRICE, tradingAccountId, USER_POS_SIZE_DELTA);

    // naruto closes position in BTC market immediately after
    // in practice this would occur one or more blocks after the
    // first order had been filled
    openManualPosition(BTC_USD_MARKET_ID, BTC_USD_STREAM_ID, MOCK_BTC_USD_PRICE, tradingAccountId, -USER_POS_SIZE_DELTA);

    // verify that now naruto has MORE USDz than they started with!
    uint256 traderCollateralBalance = perpsEngine.getAccountMarginCollateralBalance(tradingAccountId, address(usdToken)).intoUint256();
    assert(traderCollateralBalance > USER_STARTING_BALANCE);

    // naruto now withdraws all their collateral
    perpsEngine.withdrawMargin(tradingAccountId, address(usdToken), traderCollateralBalance);

    // verify that naruto has withdrawn more USDz than they deposited
    uint256 traderFinalUsdzBalance = usdToken.balanceOf(users.naruto);
    assert(traderFinalUsdzBalance > USER_STARTING_BALANCE);

    // output the profit
    console.log("Start USDz  : %s", USER_STARTING_BALANCE);
    console.log("Final USDz  : %s", traderFinalUsdzBalance);
    console.log("USDz profit : %s", traderFinalUsdzBalance - USER_STARTING_BALANCE);

    // profit = 796 040000000000000000
    //        = $796
}