Farseen0/scar-pairs huggingface dataset:
MEDIUM severity: Uptime loss due to integer division in `UptimeTracker::computeValidatorUptime` can make validator lose entire rewards for an epoch. **Description:** `UptimeTracker::computeValidatorUptime` loses validator uptime due to integer division truncation when distributing uptime across multiple epochs. This results in validators losing reward eligibility for uptime they legitimately earned.

Integer division in Solidity truncates the...

Vulnerable code:
// UptimeTracker::computeValidatorUptime
// Distribute the recorded uptime across multiple epochs
if (elapsedEpochs >= 1) {
    uint256 uptimePerEpoch = uptimeToDistribute / elapsedEpochs; // @audit integer division
    for (uint48 i = 0; i < elapsedEpochs; i++) {
        uint48 epoch = lastUptimeEpoch + i;
        if (isValidatorUptimeSet[epoch][validationID] == true) {
            break;
        }
        validatorUptimePerEpoch[epoch][validationID] = uptimePerEpoch; // @audit time loss due to precision
        isValidatorUptimeSet[epoch][validationID] = true;
    }
}

//Rewards.sol
function _calculateOperatorShare(uint48 epoch, address operator) internal {
    uint256 uptime = uptimeTracker.operatorUptimePerEpoch(epoch, operator);
    if (uptime < minRequiredUptime) {
        operatorBeneficiariesShares[epoch][operator] = 0;  // @audit no rewards
        operatorShares[epoch][operator] = 0;
        return;
    }
    // ... calculate rewards normally
}

function test_UptimeTruncationCausesRewardLoss() public {


        uint256 MIN_REQUIRED_UPTIME = 11_520;

        console2.log("Minimum required uptime per epoch:", MIN_REQUIRED_UPTIME, "seconds");
        console2.log("Epoch duration:", EPOCH_DURATION, "seconds");

        // Demonstrate how small time lost can have big impact
        uint256 totalUptime = (MIN_REQUIRED_UPTIME * 3) - 2; // 34,558 seconds across 3 epochs
        uint256 elapsedEpochs = 3;
        uint256 uptimePerEpoch = totalUptime / elapsedEpochs; // 11,519 per epoch
        uint256 remainder = totalUptime % elapsedEpochs; // 2 seconds lost

        console2.log("3 epochs scenario:");
        console2.log("  Total uptime:", totalUptime, "seconds (9.6 hours!)");
        console2.log("  Epochs:", elapsedEpochs);
        console2.log("  Per epoch after division:", uptimePerEpoch, "seconds");
        console2.log("  Lost to truncation:", remainder, "seconds");
        console2.log("  Result: ALL 3 epochs FAIL threshold!");


        // Verify
        assertFalse(uptimePerEpoch >= MIN_REQUIRED_UPTIME, "Fails threshold due to truncation");
    }
LOW severity: Operator can over allocate the same stake to unlimited nodes within one epoch causing weight inflation and reward theft. **Description:** The `AvalancheL1Middleware::addNode()` function is the entry-point an operator calls to register a new P-chain validator.
Before accepting the request the function asks `_getOperatorAvailableStake()` how much of the operator’s collateral is still free. That helper subtracts only...

Vulnerable code:
function addNode(
        bytes32 nodeId,
        bytes calldata blsKey,
        uint64 registrationExpiry,
        PChainOwner calldata remainingBalanceOwner,
        PChainOwner calldata disableOwner,
        uint256 stakeAmount // optional
    ) external updateStakeCache(getCurrentEpoch(), PRIMARY_ASSET_CLASS) updateGlobalNodeStakeOncePerEpoch {
        ...
        ...

        bytes32 valId = balancerValidatorManager.registeredValidators(abi.encodePacked(uint160(uint256(nodeId))));
        uint256 available = _getOperatorAvailableStake(operator);
        ...
        ...
    }

function _getOperatorAvailableStake(
        address operator
    ) internal view returns (uint256) {
        uint48 epoch = getCurrentEpoch();
        uint256 totalStake = getOperatorStake(operator, epoch, PRIMARY_ASSET_CLASS);

        ...
        ...

        uint256 lockedStake = operatorLockedStake[operator];
        if (totalStake <= lockedStake) {
            return 0;
        }
        return totalStake - lockedStake;
    }

function getActiveNodesForEpoch(
    address operator,
    uint48 epoch
) external view returns (bytes32[] memory activeNodeIds) {
    uint48 epochStartTs = getEpochStartTs(epoch);

    // Gather all nodes from the never-removed set
    bytes32[] memory allNodeIds = operatorNodes[operator].values();

    bytes32[] memory temp = new bytes32[](allNodeIds.length);
    uint256 activeCount;

    for (uint256 i = 0; i < allNodeIds.length; i++) {
        bytes32 nodeId = allNodeIds[i];
        bytes32 validationID =
            balancerValidatorManager.registeredValidators(abi.encodePacked(uint160(uint256(nodeId))));
        Validator memory validator = balancerValidatorManager.getValidator(validationID);

        if (_wasActiveAt(uint48(validator.startedAt), uint48(validator.endedAt), epochStartTs)) {
            temp[activeCount++] = nodeId;
        }
    }

    activeNodeIds = new bytes32[](activeCount);
    for (uint256 j = 0; j < activeCount; j++) {
        activeNodeIds[j] = temp[j];
    }
}
HIGH severity: Missing zero-address validation for burner address during initialization can break slashing. **Description:** The VaultTokenized contract's initialization procedure fails to validate that the burner parameter is a non-zero address. However, the `onSlash` function uses SafeERC20's `safeTransfer` to send tokens to this address, which will revert for most ERC20 implementations if the...

Vulnerable code:
// In _initialize:
vs.burner = params.burner; // No validation that params.burner != address(0)

// In onSlash:
if (slashedAmount > 0) {
    IERC20(vs.collateral).safeTransfer(vs.burner, slashedAmount); // Will revert if vs.burner is address(0)
}
HIGH severity: `BaseDelegator` is not using upgradeable version of `ER`. **Description:** The `BaseDelegator` contract currently inherits from the non-upgradeable version of ERC165. This could limit the contract's ability to adapt to future upgrades or modifications of the ERC165 interface, potentially impacting the contract's upgradability and compatibility with other.....

Vulnerable code:
- abstract contract BaseDelegator is AccessControlUpgradeable, ReentrancyGuardUpgradeable, IBaseDelegator, ERC165 {
+ abstract contract BaseDelegator is AccessControlUpgradeable, ReentrancyGuardUpgradeable, IBaseDelegator, ERC165Upgradeable {
    using ERC165Checker for address;
LOW severity: Vault limit cannot be modified if vault Is already enabled. **Description**

The `updateVaultMaxL1Limit` function reverts with `MapWithTimeData__AlreadyEnabled` when attempting to increase or decrease the vault limit if the vault is already enabled. This behavior occurs due to the call to `vaults.enable(vault)` within the function, which fails when the...

Vulnerable code:
function updateVaultMaxL1Limit(address vault, uint96 assetClassId, uint256 vaultMaxL1Limit) external onlyOwner {
    if (!vaults.contains(vault)) {
        revert AvalancheL1Middleware__NotVault(vault);
    }
    if (vaultToAssetClass[vault] != assetClassId) {
        revert AvalancheL1Middleware__WrongVaultAssetClass();
    }

    _setVaultMaxL1Limit(vault, assetClassId, vaultMaxL1Limit);

    if (vaultMaxL1Limit == 0) {
        vaults.disable(vault);
    } else {
        vaults.enable(vault);
    }
}

function testUpdateVaultMaxL1Limit() public {
    vm.startPrank(validatorManagerAddress);

    // Attempt to update to a new non-zero limit while vault is already enabled
    vaultManager.updateVaultMaxL1Limit(address(vault), 1, 500 ether);

    vm.stopPrank();
}
LOW severity: Incorrect vault status determination in `MiddlewareVaultManager`. **Description:** The `MiddlewareVaultManager::_wasActiveAt()`  determines whether a vault was active at a specific timestamp. This function is used by the `getVaults()` method to filter active vaults for a given epoch.

The current implementation of `_wasActiveAt()` incorrectly considers a vault...

Vulnerable code:
// Current implementation
function _wasActiveAt(uint48 enabledTime, uint48 disabledTime, uint48 timestamp) private pure returns (bool) {
    return enabledTime != 0 && enabledTime <= timestamp && (disabledTime == 0 || disabledTime >= timestamp);
}
LOW severity: Missing validation for zero `epochDuration` in AvalancheMiddleware can break epoch based accounting. **Description:** `AvalancheL1Middleware::EPOCH_DURATION` is an immutable parameter set in the constructor. The current implementation only checks that `slashingWindow` is not less than `epochDuration` but doesn't verify that `epochDuration` itself is greater than zero.

The check...

Vulnerable code:
constructor(
    AvalancheL1MiddlewareSettings memory settings,
    address owner,
    address primaryAsset,
    uint256 primaryAssetMaxStake,
    uint256 primaryAssetMinStake,
    uint256 primaryAssetWeightScaleFactor
) AssetClassRegistry(owner) {
    // Other validations...

    if (settings.slashingWindow < settings.epochDuration) {
        revert AvalancheL1Middleware__SlashingWindowTooShort(settings.slashingWindow, settings.epochDuration);
    }

    //@audit No check for zero epochDuration!

    START_TIME = Time.timestamp();
    EPOCH_DURATION = settings.epochDuration;
    // Other assignments...
}
LOW severity: Insufficient update window validation can cause denial of service in `forceUpdateNodes`. **Description:** The `AvalancheL1Middleware` constructor fails to validate that the `UPDATE_WINDOW` parameter is less than the `EPOCH_DURATION`. This validation is critically important because the `onlyDuringFinalWindowOfEpoch` modifier, which is essential for stake management functionality, will...

Vulnerable code:
modifier onlyDuringFinalWindowOfEpoch() {
    uint48 currentEpoch = getCurrentEpoch();
    uint48 epochStartTs = getEpochStartTs(currentEpoch);
    uint48 timeNow = Time.timestamp();
    uint48 epochUpdatePeriod = epochStartTs + UPDATE_WINDOW;

    if (timeNow < epochUpdatePeriod || timeNow > epochStartTs + EPOCH_DURATION) { //@audit always reverts if UPDATE_WINDOW >= EPOCH_DURATION
        revert AvalancheL1Middleware__NotEpochUpdatePeriod(timeNow, epochUpdatePeriod);
    }
    _;
}

constructor(
    AvalancheL1MiddlewareSettings memory settings,
    // other parameters...
) AssetClassRegistry(owner) {
    // other checks...

    if (settings.slashingWindow < settings.epochDuration) {
        revert AvalancheL1Middleware__SlashingWindowTooShort(settings.slashingWindow, settings.epochDuration);
    }

    // @audit No validation for UPDATE_WINDOW relation to EPOCH_DURATION

    // Initializations...
    EPOCH_DURATION = settings.epochDuration;
    UPDATE_WINDOW = settings.stakeUpdateWindow;
    // other initializations...
}

/**
 * @notice Required relationship between time parameters:
 * 0 < UPDATE_WINDOW < EPOCH_DURATION <= SLASHING_WINDOW
 */
LOW severity: Disabled operators can register new validator nodes. **Description:** The `AvalancheL1Middleware::addNode` function allows an operator to register a new node if msg.sender is included in the operators list. However, there is a potential issue with how the operator lifecycle is handled: before an operator is permanently removed via removeOperator, it.....

Vulnerable code:
(, uint48 disabledTime) = operators.getTimes(operator);
+if (!operators.contains(operator) || disabledTime > 0 ) {
-if (!operators.contains(operator)) {
    revert AvalancheL1Middleware__OperatorNotActive(operator);
}
LOW severity: Hardcoded gas limit for hook in `onSlash` may cause reverts. **Description:** The `onSlash` function in the `BaseDelegator` contract conditionally invokes a hook via a low-level `call` if a hook address is set:

This call uses a **hardcoded gas limit (`HOOK_GAS_LIMIT`)**, and the function enforces that at least `HOOK_RESERVE + HOOK_GAS_LIMIT * 64 / 63` gas...

Vulnerable code:
assembly ("memory-safe") {
    pop(call(HOOK_GAS_LIMIT, hook_, 0, add(calldata_, 0x20), mload(calldata_), 0, 0))
}
LOW severity: `NodeId` truncation can potentially cause validator registration denial of service. **Description:** `AvalancheL1Middleware::addNode` function truncates 32-byte `nodeId` to 20-bytes when checking validator registration status. This truncation occurs when interacting with the `BalancerValidatorManager`.

`uint160(uint256(nodeId))` discards the first 12 bytes of the `nodeId` before.....

Vulnerable code:
// AvalancheL1Middleware.sol
bytes32 valId = balancerValidatorManager.registeredValidators(
    abi.encodePacked(uint160(uint256(nodeId)))  // Truncates 32 bytes to 20 bytes
);

function test_NodeIdCollisionVulnerability() public {
    uint48 epoch = _calcAndWarpOneEpoch();

    // Create two different nodeIds that have the same first 20 bytes
    bytes32 nodeId1 = 0x0000000000000000000000001234567890abcdef1234567890abcdef12345678;
    bytes32 nodeId2 = 0xFFFFFFFFFFFFFFFFFFFFFFFF1234567890abcdef1234567890abcdef12345678;

    // share same first 20 bytes when truncated
    bytes memory truncated1 = abi.encodePacked(uint160(uint256(nodeId1)));
    bytes memory truncated2 = abi.encodePacked(uint160(uint256(nodeId2)));
    assertEq(keccak256(truncated1), keccak256(truncated2), "Truncated nodeIds should be identical");


    // Alice adds the first node
    vm.prank(alice);
    middleware.addNode(
                nodeId1,
                hex"ABABABAB", // dummy BLS
                uint64(block.timestamp + 2 days),
                PChainOwner({threshold: 1, addresses: new address[](1)}),
                PChainOwner({threshold: 1, addresses: new address[](1)}),
                100_000_000_001_000
            );

    // Verify first node was registered
    bytes32 validationId1 = mockValidatorManager.registeredValidators(truncated1);
    assertNotEq(validationId1, bytes32(0), "First node should be registered");

    // Alice tries to add the second node with different nodeId but same truncated bytes
    // This should fail due to collision
    vm.prank(alice);
    vm.expectRevert();
    middleware.addNode(
                nodeId2,
                hex"ABABABAB", // dummy BLS
                uint64(block.timestamp + 2 days),
                PChainOwner({threshold: 1, addresses: new address[](1)}),
                PChainOwner({threshold: 1, addresses: new address[](1)}),
                100_000_000_001_000
            );
}
LOW severity: `remainingBalanceOwner` should be set by the protocol owner. **Description:** The `remainingBalanceOwner` parameter, passed during the invocation of the `addNode` function, is intended to represent the P-Chain owner address that will receive any leftover `$AVAX` from a validator's balance when the validator is removed from the validator set. Currently, this.....

Vulnerable code:
function addNode(
        bytes32 nodeId,
        bytes calldata blsKey,
        uint64 registrationExpiry,
        PChainOwner calldata remainingBalanceOwner, // should be passed by the protocol
        PChainOwner calldata disableOwner,
        uint256 stakeAmount // optional
    ) external updateStakeCache(getCurrentEpoch(), PRIMARY_ASSET_CLASS) updateGlobalNodeStakeOncePerEpoch {

function addNode(
        bytes32 nodeId,
        bytes calldata blsKey,
        uint64 registrationExpiry,
-        PChainOwner calldata remainingBalanceOwner,
        PChainOwner calldata disableOwner,
        uint256 stakeAmount // optional
    ) external updateStakeCache(getCurrentEpoch(), PRIMARY_ASSET_CLASS) updateGlobalNodeStakeOncePerEpoch {

}
LOW severity: Optimisation of elapsed epoch calculation. **Description:** In the `UptimeTracker::computeValidatorUptime` function, there is an opportunity to optimize how the number of elapsed epochs is calculated.

However, since the epoch numbers themselves are already known (lastUptimeEpoch and currentEpoch), the number of full epochs that have...

Vulnerable code:
Currently, the code redundantly retrieves both epoch indices and their corresponding start timestamps to compute the elapsed time between epochs:

uint48 lastUptimeEpoch = l1Middleware.getEpochAtTs(uint48(lastUptimeCheckpoint.timestamp));
uint256 lastUptimeEpochStart = l1Middleware.getEpochStartTs(lastUptimeEpoch);

uint48 currentEpoch = l1Middleware.getEpochAtTs(uint48(block.timestamp));
uint256 currentEpochStart = l1Middleware.getEpochStartTs(currentEpoch);

uint256 elapsedTime = currentEpochStart - lastUptimeEpochStart;
uint256 elapsedEpochs = elapsedTime / epochDuration;

- uint256 elapsedTime = currentEpochStart - lastUptimeEpochStart;
- uint256 elapsedEpochs = elapsedTime / epochDuration;
+ uint256 elapsedEpochs = currentEpoch - lastUptimeEpoch;
LOW severity: Use unchecked block for increment operations in `distributeRewards`. **Description**

In `Rewards::distributeRewards`, an `unchecked` block can be used to optimize gas consumption for increment operations inside the loop.

**Recommended Mitigation**

Introduce an `unchecked` block for increment operations to optimize gas usage.

**Suzaku:**
Fixed in commit...

Vulnerable code:
function distributeRewards(uint48 epoch, uint48 batchSize) external onlyRole(REWARDS_DISTRIBUTOR_ROLE) {
    DistributionBatch storage batch = distributionBatches[epoch];
    uint48 currentEpoch = l1Middleware.getCurrentEpoch();

    if (batch.isComplete) revert AlreadyCompleted(epoch);
    // Rewards can only be distributed after a 2-epoch delay
    if (epoch >= currentEpoch - 2) revert RewardsDistributionTooEarly(epoch, currentEpoch - 2);

    address[] memory operators = l1Middleware.getAllOperators();
    uint256 operatorCount = 0;

    for (uint256 i = batch.lastProcessedOperator; i < operators.length && operatorCount < batchSize; i++) {
        // Calculate operator's total share based on stake and uptime
        _calculateOperatorShare(epoch, operators[i]);

        // Calculate and store vault shares
        _calculateAndStoreVaultShares(epoch, operators[i]);

        batch.lastProcessedOperator = i + 1;
        operatorCount++;
    }

    if (batch.lastProcessedOperator >= operators.length) {
        batch.isComplete = true;
    }
}

function distributeRewards(uint48 epoch, uint48 batchSize) external onlyRole(REWARDS_DISTRIBUTOR_ROLE) {
    DistributionBatch storage batch = distributionBatches[epoch];
    uint48 currentEpoch = l1Middleware.getCurrentEpoch();

    if (batch.isComplete) revert AlreadyCompleted(epoch);
    // Rewards can only be distributed after a 2-epoch delay
    if (epoch >= currentEpoch - 2) revert RewardsDistributionTooEarly(epoch, currentEpoch - 2);

    address[] memory operators = l1Middleware.getAllOperators();
    uint256 operatorCount = 0;

-   for (uint256 i = batch.lastProcessedOperator; i < operators.length && operatorCount < batchSize; i++) {
+   for (uint256 i = batch.lastProcessedOperator; i < operators.length && operatorCount < batchSize;) {
        // Calculate operator's total share based on stake and uptime
        _calculateOperatorShare(epoch, operators[i]);

        // Calculate and store vault shares
        _calculateAndStoreVaultShares(epoch, operators[i]);

+       unchecked {
            batch.lastProcessedOperator = i + 1;
            operatorCount++;
+          i++;
+       }
    }

    if (batch.lastProcessedOperator >= operators.length) {
        batch.isComplete = true;
    }
}
LOW severity: Optimize `_getStakerVaults` to Avoid Redundant External Calls to `activeBalanceOfAt`. **Description:** The `Rewards::_getStakerVaults` function performs two passes over the vault array to filter vaults with non-zero balances, which doubles the number of external calls to `IVaultTokenized.activeBalanceOfAt`.

**Recommended Mitigation:** Make the following change to the...

Vulnerable code:
function _getStakerVaults(address staker, uint48 epoch) internal view returns (address[] memory) {
        address[] memory vaults = middlewareVaultManager.getVaults(epoch);
        uint48 epochStart = l1Middleware.getEpochStartTs(epoch);

        uint256 count = 0;

        // First pass: Count non-zero balance vaults
        for (uint256 i = 0; i < vaults.length; i++) {
            uint256 balance = IVaultTokenized(vaults[i]).activeBalanceOfAt(staker, epochStart, new bytes(0));
            if (balance > 0) {
                count++;
            }
        }

        // Create a new array with the exact number of valid vaults
        address[] memory validVaults = new address[](count);
        uint256 index = 0;

        // Second pass: Populate the new array
        for (uint256 i = 0; i < vaults.length; i++) {
            uint256 balance = IVaultTokenized(vaults[i]).activeBalanceOfAt(staker, epochStart, new bytes(0));
            if (balance > 0) {
                validVaults[index] = vaults[i];
                index++;
            }
        }

        return validVaults;
    }

function _getStakerVaults(address staker, uint48 epoch) internal view returns (address[] memory) {
    address[] memory vaults = middlewareVaultManager.getVaults(epoch);
    uint48 epochStart = l1Middleware.getEpochStartTs(epoch);
+   address[] memory tempVaults = new address[](vaults.length); // Temporary oversized array
    uint256 count = 0;

-   // First pass: Count non-zero balance vaults
-   for (uint256 i = 0; i < vaults.length; i++) {
-       uint256 balance = IVaultTokenized(vaults[i]).activeBalanceOfAt(staker, epochStart, new bytes(0));
-       if (balance > 0) {
-           count++;
-       }
-   }
-
-   // Create a new array with the exact number of valid vaults
+   // Single pass: Collect valid vaults
+   for (uint256 i = 0; i < vaults.length;) {
+       if (IVaultTokenized(vaults[i]).activeBalanceOfAt(staker, epochStart, new bytes(0)) > 0) {
+           tempVaults[count] = vaults[i];
+           count++;
+       }
+       unchecked { i++; }
+   }
+
+   // Copy to correctly sized array
    address[] memory validVaults = new address[](count);
-   uint256 index = 0;
-
-   // Second pass: Populate the new array
-   for (uint256 i = 0; i < vaults.length; i++) {
-       uint256 balance = IVaultTokenized(vaults[i]).activeBalanceOfAt(staker, epochStart, new bytes(0));
-       if (balance > 0) {
-           validVaults[index] = vaults[i];
-           index++;
-       }
+   for (uint256 i = 0; i < count;) {
+       validVaults[i] = tempVaults[i];
+       unchecked { i++; }
    }

    return validVaults;
}
HIGH severity: `guardian` missing `PAUSER_ROLE` grant in `onUSD` deployment. **Description:** Deployment of the `onUSD` token is handled via the `onUSDFactory`, which sets up the token as an upgradeable proxy using the transparent proxy pattern (EIP-1967).

As documented in the contract comments, the `guardian` address is expected to be granted both the...

Vulnerable code:
/**
...
 *         Following the above mentioned deployment, the address of the onUSD_Factory contract will:
 *         i) Grant the `DEFAULT_ADMIN_ROLE` & PAUSER_ROLE to the `guardian` address <<----------------
 *         ii) Revoke the `MINTER_ROLE`, `PAUSER_ROLE` & `DEFAULT_ADMIN_ROLE` from address(this).
 *         iii) Transfer ownership of the ProxyAdmin to that of the `guardian` address.
 */

function deployonUSD( ... ) external onlyGuardian returns (address, address, address) {
    ...

    // @audit `PAUSER_ROLE` not granted to guardian
>>  onusdProxied.grantRole(DEFAULT_ADMIN_ROLE, guardian);

    onusdProxied.revokeRole(MINTER_ROLE, address(this));
    onusdProxied.revokeRole(PAUSER_ROLE, address(this));
    onusdProxied.revokeRole(DEFAULT_ADMIN_ROLE, address(this));

    onusdProxyAdmin.transferOwnership(guardian);
    assert(onusdProxyAdmin.owner() == guardian);
    initialized = true;
    emit onUSDDeployed( ... );

    return ( ... );
  }

onusdProxied.initialize(name, ticker, complianceView);

  onusdProxied.grantRole(DEFAULT_ADMIN_ROLE, guardian);
+ onusdProxied.grantRole(PAUSER_ROLE, guardian);

  onusdProxied.revokeRole(MINTER_ROLE, address(this));
  onusdProxied.revokeRole(PAUSER_ROLE, address(this));