Farseen0/scar-pairs huggingface dataset:
HIGH severity: Tick spacing type mismatch in ExactInputSingleParams. **Description:** V3DexSwap uses the [Ramses V3 Swap Router](https://lineascan.build/address/0x8BE024b5c546B5d45CbB23163e1a4dca8fA5052A#code) to perform WETH to Linea swaps.

However, the ExactInputSingleParams struct definition passed as parameter to the exactInputSingle function differs between...

Vulnerable code:
struct ExactInputSingleParams {
    address tokenIn;
    address tokenOut;
    uint24 tickSpacing; <<
    address recipient;
    uint256 deadline;
    uint256 amountIn;
    uint256 amountOutMinimum;
    uint160 sqrtPriceLimitX96;
  }

struct ExactInputSingleParams {
        address tokenIn;
        address tokenOut;
        int24 tickSpacing; <<
        address recipient;
        uint256 deadline;
        uint256 amountIn;
        uint256 amountOutMinimum;
        uint160 sqrtPriceLimitX96;
    }
HIGH severity: Inconsistent deadline check in `V3DexSwap.swap `. **Description:** The swap() function in V3DexSwap implements the require check in the snippet below which ensures block.timestamp is strictly less than the `_deadline`.

However, the [Ramses V3 Swap Router](https://lineascan.build/address/0x8BE024b5c546B5d45CbB23163e1a4dca8fA5052A#code) allows...

Vulnerable code:
require(_deadline > block.timestamp, DeadlineInThePast());

modifier checkDeadline(uint256 deadline) {
        if (_blockTimestamp() > deadline) revert Old();
        _;
    }
LOW severity: Function updateInvoiceArrears() does not emit old values. **Description:** Function updateInvoiceArrears() should consider emitting previous `invoiceArrears` and `lastInvoiceDate` values to maintain consistency with other setter functions updateL1LineaTokenBurner, updateDex and updateInvoicePaymentReceiver that emit both old and new values in their...

Vulnerable code:
function updateInvoiceArrears(
    uint256 _newInvoiceArrears,
    uint256 _lastInvoiceDate
  ) external onlyRole(DEFAULT_ADMIN_ROLE) {
    require(_lastInvoiceDate >= lastInvoiceDate, InvoiceDateTooOld());

    invoiceArrears = _newInvoiceArrears;
    lastInvoiceDate = _lastInvoiceDate;

    emit InvoiceArrearsUpdated(_newInvoiceArrears, _lastInvoiceDate);
  }
HIGH severity: Unnecessary ETH to WETH conversion during swap. **Description:** In the V3DexSwap contract, ETH is converted to WETH before processing the swap through the router as seen in the snippet below.

However, this is not required since the router supports direct ETH to Linea swaps. As we can observe below, the exactInputSingle function is marked as...

Vulnerable code:
IWETH9(WETH_TOKEN).deposit{ value: msg.value }();
IWETH9(WETH_TOKEN).approve(ROUTER, msg.value);

File: SwapRouter.sol

/// @inheritdoc ISwapRouter
    function exactInputSingle(
        ExactInputSingleParams calldata params
    ) external payable override checkDeadline(params.deadline) returns (uint256 amountOut) {


File: PeripheryPayments.sol

/// @param token The token to pay
    /// @param payer The entity that must pay
    /// @param recipient The entity that will receive payment
    /// @param value The amount to pay
    function pay(
        address token,
        address payer,
        address recipient,
        uint256 value
    ) internal {
        if (token == WETH9 && address(this).balance >= value) {
            // pay with WETH9
            IWETH9(WETH9).deposit{value: value}(); // wrap only what is needed to pay
            IWETH9(WETH9).transfer(recipient, value);
        } else if (payer == address(this)) {
            // pay with tokens already in the contract (for the exact input multihop case)
            TransferHelper.safeTransfer(token, recipient, value);
        } else {
            // pull payment
            TransferHelper.safeTransferFrom(token, payer, recipient, value);
        }
    }
MEDIUM severity: Same wallet can be added multiple times to an investor, artificially increasing their wallet count causing adding new wallets to revert. **Description:** `GlobalRegistryService::_updateInvestor` calls `_addWallet` when the wallet being added is already registered to this investor:

`GlobalRegistryService::_addWallet` in turn increments the investor's `walletCount` and reverts once the max are reached:

**Impact:** An investor's...

Vulnerable code:
for (uint8 i = 0; i < walletAddresses.length; i++) {
    // @audit if it is a wallet and it doesn't belong to this investor, revert
    if (isWallet(walletAddresses[i]) && !CommonUtils.isEqualString(getInvestor(walletAddresses[i]), id)) {
        revert WalletBelongsToAnotherInvestor();
    }
    // @audit otherwise add it - even if it is a wallet that already belongs to this investor!
    else {
        _addWallet(walletAddresses[i], id);
    }
}

function _addWallet(address walletAddress, string memory id) internal addressNotZero(walletAddress) returns (bool) {
    if (investors[id].walletCount >= MAX_WALLETS_PER_INVESTOR) {
        revert MaxWalletsReached();
    }
    address sender = _msgSender();
    investorsWallets[walletAddress] = Wallet(id, sender);
    investors[id].walletCount++;

    emit GlobalWalletAdded(walletAddress, id, sender);

    return true;
}

function walletCountByInvestor(string calldata investorId) public view returns (uint256) {
        return investors[investorId].walletCount;
    }
MEDIUM severity: `ComplianceServiceGlobalWhitelisted::newPreTransferCheck` and `preTransferCheck` allow blacklisted users to transfer tokens. **Description:** `ComplianceServiceGlobalWhitelisted::newPreTransferCheck` and `preTransferCheck` only enforce that the recipient of tokens is not blacklisted, but they don't check that the sender is also not blacklisted:

**Impact:** A blacklisted user can transfer their tokens to a...

Vulnerable code:
function newPreTransferCheck(
    address _from,
    address _to,
    uint256 _value,
    uint256 _balanceFrom,
    bool _pausedToken
) public view virtual override returns (uint256 code, string memory reason) {
    // First check if recipient is blacklisted
    if (getBlackListManager().isBlacklisted(_to)) {
        return (100, WALLET_BLACKLISTED);
    }

    // Then perform the standard whitelist check
    return super.newPreTransferCheck(_from, _to, _value, _balanceFrom, _pausedToken);
}

function preTransferCheck(address _from, address _to, uint256 _value) public view virtual override returns (uint256 code, string memory reason) {
    // First check if recipient is blacklisted
    if (getBlackListManager().isBlacklisted(_to)) {
        return (100, WALLET_BLACKLISTED);
    }

    // Then perform the standard whitelist check
    return super.preTransferCheck(_from, _to, _value);
}
HIGH severity: `StandardToken::transferWithPermit` can be DoS attacked by front-running to directly call `ERPermitMixin::permit`. **Description:** `StandardToken::transferWithPermit` contains two calls:
* first to `ERC20PermitMixin::permit`
* second to the `StandardToken::transferFrom`

**Impact:** Since the permit signature and parameters are visible in the mempool before execution, an attacker can extract these values and...

Vulnerable code:
it('front-running attack on transferWithPermit()', async () => {
        const [owner, spender, recipient, attacker] = await hre.ethers.getSigners();
        const { dsToken, registryService } = await loadFixture(deployDSTokenRegulated);
        const value = 100;
        const deadline = BigInt(Math.floor(Date.now() / 1000) + 3600);

        // Owner creates a signature to allow spender to transfer tokens to recipient
        const message = {
          owner: owner.address,
          spender: spender.address,
          value,
          nonce: await dsToken.nonces(owner.address),
          deadline,
        };
        const { v, r, s } = await buildPermitSignature(owner, message, await dsToken.name(), await dsToken.getAddress());

        // Register investors and issue tokens to owner; see that the attacker is not even an ibnvestor so it could be any address
        await registerInvestor(INVESTORS.INVESTOR_ID.INVESTOR_ID_1, owner, registryService);
        await registerInvestor(INVESTORS.INVESTOR_ID.INVESTOR_ID_2, recipient, registryService);

        await dsToken.issueTokens(owner, value);

        // ATTACK SCENARIO 1: Attacker front-runs by calling permit() directly
        await dsToken.connect(attacker).permit(owner.address, spender.address, value, deadline, v, r, s);


        // When the original transferWithPermit() executes, it FAILS
        // because the nonce has already been used
        await expect(
          dsToken.connect(spender).transferWithPermit(owner.address, recipient.address, value, deadline, v, r, s)
        ).to.be.revertedWith('Permit: invalid signature');
      });

function transferWithPermit(
    address from,
    address to,
    uint256 value,
    uint256 deadline,
    uint8 v,
    bytes32 r,
    bytes32 s
) external returns (bool) {
    // Try to execute permit, but don't revert if it fails
    try this.permit(from, msg.sender, value, deadline, v, r, s) {
        // Permit succeeded
    } catch {
        // Permit failed (possibly due to front-running or already executed)
        // Verify we have sufficient allowance to proceed
        require(allowance(from, msg.sender) >= value, "Insufficient allowance");
    }

    // Perform the actual transferFrom
    return transferFrom(from, to, value);
}
LOW severity: Cross-chain incompatibility with `block.number` based timeout mechanism. **Description:** `GlobalRegistryService::addGlobalInvestorWallet` function uses `block.number` to validate transaction freshness through a `blockLimit parameter`:

According to the hardhat configuration, this contract is designed to be deployed on multiple chains with vastly different block...

Vulnerable code:
function addGlobalInvestorWallet(
    string calldata id,
    address walletAddress,
    uint256 blockLimit
) external override whenNotPaused onlySelf newWallet(walletAddress) returns (bool) {
    if (blockLimit < block.number) {
        revert TransactionTooOld();
    }
    // ...
}
LOW severity: Distribution logic can be broken by upgrades to the `GaugeRegistrar` by the DAO to add logic to modify the `enableUpdateVotingPowerHook` configuration. **Description:** The `DistributionManager` contract hardcodes epoch 0 when reading gauge votes from the `AddressGaugeVoter ` contract within `_collectVotes()`:

However, `AddressGaugeVoter` conditionally stores votes in either epoch 0 or the current epoch based on the `enableUpdateVotingPowerHook`.....

Vulnerable code:
function _collectVotes() internal view returns (...) {
    ...
    for (uint256 j = 0; j < gaugeAddresses.length; j++) {
        address gaugeAddress = gaugeAddresses[j];
        IGaugeRegistrar.RegisteredGauge memory gaugeInfo = gaugeRegistrar.getGaugeInfo(gaugeAddress);

@>      uint256 votes = gaugeVoter.epochGaugeVotes(0, gaugeAddress);

        gaugesByController[i][j] = IDistributionGaugeVote.GaugeVote({
            gaugeAddress: gaugeAddress,
            qiToken: gaugeInfo.qiToken,
            incentive: gaugeInfo.incentive,
            votes: votes
        });
        voteTotals[i] += votes;
    }
}

function _vote(address _account, GaugeVote[] memory _votes) internal {
    ...
    uint256 epoch = getWriteEpochId(); // ← Uses conditional epoch
    AddressVoteData storage voteData = epochVoteData[epoch][_account];
    ...
}

function getWriteEpochId() public view returns (uint256) {
    return enableUpdateVotingPowerHook ? 0 : epochId();
}
HIGH severity: Missing interface compliance validation in `DistributorManager`. **Description:** The `DistributionManager` contract does not currently validate that the `budgetAllocator` and controller module contracts implement the required interfaces when set during initialization or via `setBudgetAllocator()` and `setModuleForController()` respectively. Both the...

Vulnerable code:
// DistributionManager.sol
function _setBudgetAllocator(IBudgetAllocator _budgetAllocator) internal {
    if (address(_budgetAllocator) == address(0)) revert InvalidAddress("budgetAllocator");

    // @audit -  missing interface validation check

    address oldAllocator = address(budgetAllocator);
    budgetAllocator = _budgetAllocator;

    emit BudgetAllocatorSet(oldAllocator, address(_budgetAllocator));
}

function _setModuleForController(
    address _rewardController,
    IDistributionModule _module
) internal {
    if (_rewardController == address(0)) revert InvalidAddress("rewardController");
    if (address(_module) == address(0)) revert InvalidAddress("module");

    // @audit -  missing interface validation check

    if (address(rewardControllerToModule[_rewardController]) != address(0)) {
        revert ControllerAlreadyRegistered(_rewardController);
    }

    _activeRewardControllers.add(_rewardController);
    rewardControllerToModule[_rewardController] = _module;

    emit ModuleForControllerSet(_rewardController, address(_module));
}

if (!IERC165(address(_budgetAllocator)).supportsInterface(type(IBudgetAllocator).interfaceId)) {
      revert InvalidInterface("budgetAllocator");
}
HIGH severity: `BenqiEcosystemModule` and `UnifiedBudgetAllocator` are not fully ERcompliant. **Description:** The `BenqiEcosystemModule` contract inherits both `IDistributionModule` and `IBenqiEcosystemModuleErrors`. According to the ERC-165 specification for interface detection, the `supportsInterface()` function should return `true` for every interface the contract implements. However,...

Vulnerable code:
// BenqiEcosystemModule.sol
function supportsInterface(bytes4 _interfaceId) public view virtual override returns (bool) {
    return
        _interfaceId == type(IDistributionModule).interfaceId ||
        _interfaceId == type(IBenqiEcosystemModuleErrors).interfaceId ||
        super.supportsInterface(_interfaceId);
}

// UnifiedBudgetAllocator.sol
function supportsInterface(bytes4 interfaceId) public view virtual override(DaoAuthorizableUpgradeable, IERC165) returns (bool) {
    return
        interfaceId == type(IBudgetAllocator).interfaceId ||
        interfaceId == type(IUnifiedBudgetAllocatorEventsAndErrors).interfaceId ||
        interfaceId == type(IERC1822Proxiable).interfaceId ||
        interfaceId == type(IERC165).interfaceId ||
}
LOW severity: Maximum number of Aragon DAO actions is not checked within the `DistributorManager`. **Description:** The `DistributionManager` generates actions for reward distribution but does not currently validate against the DAO's `MAX_ACTIONS` limit before calling `dao.execute()`. The Aragon DAO has a hard limit of 256 actions per execution:

But `DistributionManager` calculates the action...

Vulnerable code:
// lib/conditions/lib/osx/packages/contracts/src/core/dao/DAO.sol
uint256 internal constant MAX_ACTIONS = 256;

function execute(
        bytes32 _callId,
        Action[] calldata _actions,
        uint256 _allowFailureMap
    )
        external
        override
        nonReentrant
        auth(EXECUTE_PERMISSION_ID)
        returns (bytes[] memory execResults, uint256 failureMap)
    {
        // Check that the action array length is within bounds.
@>      if (_actions.length > MAX_ACTIONS) {
            revert TooManyActions();
        }
        ...
}

function distribute() external auth(DISTRIBUTOR_ROLE) {
    ...
    IExecutor(address(dao())).execute({
        _callId: _generateCallId(),
        _actions: actions, // @audit - no length validation
        _allowFailureMap: 0
    });
    emit RewardsDistributed(currentEpochId);
}
LOW severity: Single `rewardToken` design is incompatible with multi-reward emission support. **Description:** `BenqiEcosystemModule` assumes QI as the single immutable `rewardToken` always referenced directly within `_processGauge()`:

However, `MultiRewardDistributor` supports multiple reward tokens per `qiToken` market:

> Each market has an array of configs, each with a unique emission.....

Vulnerable code:
function _processGauge(
    IMultiRewardDistributor _distributor,
    GaugeVote memory _gauge,
    uint256 _totalVotes,
    uint256 _budget,
    uint256 _epochDuration,
    uint256 _emissionCap
) internal view returns (ProcessGaugeReturnValue memory returnData) {
    ...

    if (_gauge.incentive == IGaugeRegistrar.Incentive.Supply) {
        returnData.actionData = abi.encodeCall(
            IMultiRewardDistributor._updateSupplySpeed,
@>          (QiToken(_gauge.qiToken), rewardToken, newSpeed)
        );
    } else {
        returnData.actionData = abi.encodeCall(
            IMultiRewardDistributor._updateBorrowSpeed,
@>          (QiToken(_gauge.qiToken), rewardToken, newSpeed)
        );
    }
}

/// @notice The main data storage for this contract, holds a mapping of qiToken to array
//          of market configs
mapping(address => MarketEmissionConfig[]) public marketConfigs;

function _updateSupplySpeed(
        QiToken _qiToken,
        address _emissionToken,
        uint256 _newSupplySpeed
    ) external onlyEmissionConfigOwnerOrAdmin(_qiToken, _emissionToken) {
        MarketEmissionConfig storage emissionConfig = fetchConfigByEmissionToken(
@>          _qiToken,
@>          _emissionToken
        );
        ...
    }
LOW severity: Reward distribution can be skipped when executed within the vote window buffer of a new epoch. **Description:** `DistributionManager::distribute` is validated to only be callable when voting is not active. This is achieved by calling `_validateCanDistribute()`; however, this fails to account for the `VOTE_WINDOW_BUFFER` period at the start of each epoch.

The Clock contract defines epochs...

Vulnerable code:
function _validateCanDistribute(IClock _clock) internal view {
        if (_activeRewardControllers.length() == 0) revert NoModulesConfigured();
@>      if (_clock.votingActive()) revert VotingStillActive(); // @audit - returns false in buffer period

@>      uint256 currentEpochId = _clock.currentEpoch(); // @audit - returns N+1
@>      if (_isEpochDistributed(currentEpochId)) { // @audit - checks if N+1 distributed, not N
            revert EpochAlreadyDistributed(currentEpochId);
        }
    }

function testPoC_bufferWindowSkipsEpoch() public {
    // Setup gauge and votes for epoch 0
    vm.prank(ADMIN);
    address gauge = gaugeRegistrar.registerGauge(
        QI_TOKEN_1, IGaugeRegistrar.Incentive.Supply, address(coreComptroller), "Gauge"
    );
    coreComptroller.setMarketListed(QI_TOKEN_1, true);
    addressGaugeVoter.setEpochGaugeVotes(0, gauge, 1000 ether);

    // Warp to 30 minutes into epoch 1 (in VOTE_WINDOW_BUFFER)
    vm.warp(2 weeks + 30 minutes);

    // Verify preconditions: epoch 1, voting inactive, in buffer period
    assertEq(clock.currentEpoch(), 1);
    assertFalse(clock.votingActive());
    assertLt(clock.elapsedInEpoch(), 1 hours);

    // Call distribute() - should fail but doesn't
    vm.prank(DISTRIBUTOR);
    distributionManager.distribute();

    // BUG: Epoch 1 marked distributed, Epoch 0 skipped forever
    assertFalse(distributionManager.isDistributed(0), "Epoch 0 never distributed!");
    assertTrue(distributionManager.isDistributed(1), "Epoch 1 wrongly marked distributed");
}
LOW severity: Asymmetric gauge unregistration can result in misallocation of tokens. **Description:** The reward speed preservation logic in the `BenqiCoreModule` contract enables indefinite QI token distributions for markets associated with asymmetrically unregistered or otherwise expired supply/borrow gauges. Specifically, when a gauge is unregistered via...

Vulnerable code:
function test_deactivateSupplyGauge_usesExistingSpeed() public {
    // Set existing speeds
    comptroller.setRewardSpeeds(QITOKEN_1, 1000, 2000);

    // Only provide borrow gauge, not supply
    IDistributionGaugeVote.GaugeVote[] memory gauges = new IDistributionGaugeVote.GaugeVote[](1);
    gauges[0] = IDistributionGaugeVote.GaugeVote({
    	gaugeAddress: GAUGE_1,
    	qiToken: QITOKEN_1,
    	incentive: IGaugeRegistrar.Incentive.Borrow,
    	votes: 100
    });

    (Action[] memory actions, ) = module.generateActions(
    	address(comptroller),
    	gauges,
    	MODULE_BUDGET,
    	EPOCH_DURATION
    );

    // Decode the action to check speeds
    (, , uint256 supplySpeed, uint256 borrowSpeed) = decodeSetRewardSpeed(actions[0].data);

    // Borrow speed should be calculated from votes
    uint256 expectedBorrowSpeed = ((MODULE_BUDGET / EPOCH_DURATION) * 100) / 100;
    assertEq(borrowSpeed, expectedBorrowSpeed);
    assertGt(borrowSpeed, 0);
    assertNotEq(borrowSpeed, 2000);

    // Supply speed should be preserved
    assertEq(supplySpeed, 1000);
}
LOW severity: `DistributionManagerSetup::canDistribute` should return false rather than reverting. **Description:** `DistributionManagerSetup::canDistribute` currently returns true if distribution can proceed but reverts from within `_validateCanDistribute()` if it cannot:

Rather than reverting, this view function should return false so it can be reliably called by consumers.

**BENQI:** Fixed.....

Vulnerable code:
function canDistribute() public view returns (bool) {
        IClock clock = IClock(gaugeVoter.clock());
@>      _validateCanDistribute(clock);
        return true;
    }

    function _validateCanDistribute(IClock _clock) internal view {
@>      if (_activeRewardControllers.length() == 0) revert NoModulesConfigured();
@>      if (_clock.votingActive()) revert VotingStillActive();

        uint256 currentEpochId = _clock.currentEpoch();
        if (_isEpochDistributed(currentEpochId)) {
@>          revert EpochAlreadyDistributed(currentEpochId);
        }
    }
LOW severity: Arbitrary controller addresses can be passed to `DistributionManager::setModuleForController` without validation of the corresponding gauge addresses. **Description:** The DAO can currently pass any arbitrary controller address in calls to `DistributionManager::setModuleForController`; however, this logic should additionally validate that the gauges of the controller are registered on the `GaugeRegistrar` and `AddressGaugeVoter` to avoid adding...

Vulnerable code:
/// @dev Internal function to set module for controller
function _setModuleForController(
    address _rewardController,
    IDistributionModule _module
) internal {
    if (_rewardController == address(0)) revert InvalidAddress("rewardController");
    if (address(_module) == address(0)) revert InvalidAddress("module");

    // Enforce one module per controller constraint
    if (address(rewardControllerToModule[_rewardController]) != address(0)) {
        revert ControllerAlreadyRegistered(_rewardController);
    }

@>  _activeRewardControllers.add(_rewardController);

    rewardControllerToModule[_rewardController] = _module;

    emit ModuleForControllerSet(_rewardController, address(_module));
}

function _validateCanDistribute(IClock _clock) internal view {
@>      if (_activeRewardControllers.length() == 0) revert NoModulesConfigured();
        if (_clock.votingActive()) revert VotingStillActive();

        uint256 currentEpochId = _clock.currentEpoch();
        if (_isEpochDistributed(currentEpochId)) {
            revert EpochAlreadyDistributed(currentEpochId);
        }
    }

address controller = _activeRewardControllers.at(i);
address[] memory gaugeAddresses = gaugeRegistrar.getGaugesByRewardController(
    controller
);
HIGH severity: Unsafe transfer of non-standard ERtokens. **Description:** The `DistributionManager` uses the standard ERC-20 `transfer()` function instead of a safe transfer equivalent when distributing reward tokens to controllers. While the reward token (QI) is a known, trusted contract that conforms to the ERC-20 specification, direct use of...

Vulnerable code:
// DistributionManager.sol - Lines 416-424
actions[actionCount++] = Action({
    to: address(rewardToken),
    value: 0,
    data: abi.encodeWithSelector(
        IERC20.transfer.selector, // @audit - uses transfer, not safeTransfer
        controller,
        amountWithBuffer
    )
});