Farseen0/scar-pairs huggingface dataset:
MEDIUM severity: Vester template misconfiguration can potentially block token claims. **Description:** The `WorldLibertyFinancialVester` contract can make user tokens temporarily inaccessible when template `capPerUser` values don't sum to the user's total allocation.

Users transfer their full allocation to the vester during activation, but can only claim back the portion covered...

Vulnerable code:
function _unlockedTotal(uint8 _category, uint112 _allocation) internal view returns (uint256) {
    uint256 totalUnlocked = 0;
    uint256 remainingCap = _allocation;  // Start with full allocation

    for (uint8 i; i < count; ) {
        uint256 segmentCap = t.capPerUser < remainingCap ? t.capPerUser : remainingCap;
        uint256 unlocked = _segmentUnlocked(t, segmentCap);
        totalUnlocked += unlocked;

        // @audit remainingCap reduced by template cap, not allocation
        remainingCap -= segmentCap;

        unchecked { ++i; }
    }

    // @audit Any remaining allocation is ignored and becomes inaccessible
    return totalUnlocked;  // Missing: + remainingCap
}
MEDIUM severity: WLFI owner can DoS legacy users through direct vester activation. **Description:** The `WorldLibertyFinancialVester::ownerActivateVest` function can be used to bypass the normal activation flow, potentially causing a denial-of-service for legacy users. When the owner directly activates a user in the vester with incorrect parameters, the user is unable to...

Vulnerable code:
// WorldLibertyFinancialV2.sol
function _activateAccount(address _account) internal {
    REGISTRY.wlfiActivateAccount(_account);                    // @note -> Mark as activated in registry
    uint8 category = REGISTRY.getLegacyUserCategory(_account);
    uint112 allocation = REGISTRY.getLegacyUserAllocation(_account);

    _approve(_account, address(VESTER), 0);
    _approve(_account, address(VESTER), allocation);           // @note -> Set allowance

    VESTER.wlfiActivateVest(_account, category, allocation);   // @note -> Activate vesting
    assert(allowance(_account, address(VESTER)) == 0);
}

// WorldLibertyFinancialVester.sol
function ownerActivateVest(address _user, uint8 _category, uint112 _amount)
    external
    onlyWorldLibertyOwner(msg.sender)
{
    _activateVest(_user, _category, _amount);  // @audit No coordination with Registry/WLFI V2
    // @audit any amount that is approved by user can be taken -> not registry allocation
   // @audit vesting can be in any category -> not necessarily category in registry
}

function _activateVest(address _user, uint8 _category, uint112 _amount) internal {
    UserInfo storage userInfo = $.users[_user];
    if (userInfo.initialized) {
        revert AlreadyInitialized(_user);      // @audit if user tries to activate later, he will be DOS'ed here     }
    // ... activation logic
}

function ownerActivateVest(address _user, uint8 _category, uint112 _amount) external {
    // For legacy users: validate parameters and sync registry
    if (REGISTRY.isLegacyUser(_user)) {
        // @audit Validate parameters match registry data
        require(_category == REGISTRY.getLegacyUserCategory(_user), "CATEGORY_MISMATCH");
        require(_amount == REGISTRY.getLegacyUserAllocation(_user), "ALLOCATION_MISMATCH");

        // @audit Auto-sync registry state to maintain consistency
        if (!REGISTRY.isLegacyUserAndIsActivated(_user)) {
            REGISTRY.ownerActivateAccount(_user);
        }
    }

    _activateVest(_user, _category, _amount);
}
MEDIUM severity: Addresses excluded from voting power can re-gain their voting power via a delegatee or by transfering tokens. **Description:** WFI V2 token has a way to exclude a user's voting power by marking its `_excludedVotingPower` status to true. The intention is that after this, the user's balance will no longer be usable in voting as it delegates current balance to address(0) so that the current delegatee can no...

Vulnerable code:
function _delegate(
        address _account,
        address _delegatee
    )
        notBlacklisted(_msgSender())
        notBlacklisted(_account)
        notBlacklisted(_delegatee)
        internal
        override
    {
        super._delegate(_account, _delegatee);
    }
LOW severity: Missing zero address validation for authorized signer in `WorldLibertyFinancialV2.initialize()`. **Description:** The `WorldLibertyFinancialV2::initialize()` function does not validate that the `_authorizedSigner` parameter is not the zero address.

This parameter is critical for the activateAccount() function, which allows legacy users to self-activate their accounts.

Same issue also exists.....

Vulnerable code:
function initialize(address _authorizedSigner) external reinitializer(/* version = */ 2) {
    __EIP712_init(name(), "2");

    V2 storage $ = _getStorage();
    _ownerSetAuthorizedSigner($, _authorizedSigner); // @audit No zero address validation
}
HIGH severity: No governance protection for `MAX_VOTING_POWER` changes. **Description:** The `WorldLibertyFinancialV2::ownerSetMaxVotingPower()` allows the owner to change the maximum voting power cap at any time without restrictions. Since this is a governance token that inherits from `ERC20VotesUpgradeable` and is likely used with external governance systems (OZ...

Vulnerable code:
//WorldLibertyFinancialV2.sol
function ownerSetMaxVotingPower(uint256 _maxVotingPower) external onlyOwner {
    require(
        _maxVotingPower > 0 && _maxVotingPower <= (5_000_000_000 * 1 ether),
        "Invalid max voting power"
    );
    MAX_VOTING_POWER = _maxVotingPower; // @audit No governance protections
    emit SetMaxVotingPower(_maxVotingPower);
}

if (votingPower > MAX_VOTING_POWER) {
    return MAX_VOTING_POWER; // @audit Immediate capping on voting power
}
LOW severity: Weak signature validation in account activation. **Description:** The `WorldLibertyFinancialV2::activateAccount` function uses a simple hash of the account address for signature validation instead of following EIP-712 standards.

While the contract initializes EIP-712 infrastructure during setup, the activation function bypasses this standard...

Vulnerable code:
function activateAccount(bytes calldata _signature) external {
    address account = _msgSender();
    bytes32 hash = keccak256(abi.encode(account)); // @account simple hash, no EIP-712

    if (authorizedSigner() != ECDSA.recover(hash, _signature)) {
        revert InvalidSignature();
    }

    _activateAccount(account);
}
LOW severity: Guardian can override owner's emergency pause. **Description:** The contract implements symmetric `pause/unpause` powers between the owner and guardians, allowing guardians to unpause the contract even when the owner intentionally paused it for security or operational reasons. This creates an authority hierarchy conflict where guardians can...

Vulnerable code:
function guardianUnpause() external onlyGuardian whenPaused {
    // @audit - do you think only the owner should be able to unpause?
    _unpause();
}

Multiple parties can pause (defensive action, low risk)
Only highest authority can unpause (requires careful consideration)
HIGH severity: Use `SafeCast` to safely downcast amounts. **Description:** Use [SafeCast](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/math/SafeCast.sol) to safely downcast amounts. or add a comment indicate that this downcast is safe:

**WLFI:**
Fixed in commit...

Vulnerable code:
wlfi/WorldLibertyFinancialRegistry.sol
64:                amount: uint112(_amounts[i]),
MEDIUM severity: Combination of Ownable and AccessControl can cause loss of admin functionality. **Description:** `BasisTradeTailor` and `BasisTradeVault` mix `Ownable(2Step)Upgradeable` with `AccessControlUpgradeable`. Several admin wrappers are `onlyOwner` but internally call `grantRole` / `revokeRole`, which themselves require the caller to hold the role’s admin (usually...

Vulnerable code:
function grantAdmin(address account) external onlyOwner {
    grantRole(ADMIN_ROLE, account); // requires DEFAULT_ADMIN_ROLE too
}

function test_OwnerLosesGrantAbility() public {
    _setupComplete();

    // owner transfers ownership
    vm.prank(deployer);
    vault.transferOwnership(alice);
    assertEq(vault.owner(), alice);

    // new owner cannot grant admin (or agent) roles
    vm.prank(alice);
    vm.expectRevert(
        abi.encodeWithSelector(
            IAccessControl.AccessControlUnauthorizedAccount.selector,
            alice,
            bytes32(0x00)
        )
    );
    vault.grantAdmin(bob);
}

function grantAdmin(address account) external onlyRole(DEFAULT_ADMIN_ROLE) {
    grantRole(ADMIN_ROLE, account);
}

function _authorizeUpgrade(address impl) internal override onlyRole(DEFAULT_ADMIN_ROLE) {}
HIGH severity: `BasisTradeTailor` is ERnon compliant. **Description:** The `BasisTradeTailor` contract inherits from and implements the `ITailor` interface. According to the ERC-165 standard, the `supportsInterface` function should return `true` when queried with the interface ID of `ITailor` and `IERC1822Proxiable` (coming from...

Vulnerable code:
// ...existing code...

import {IERC1822Proxiable} from "@openzeppelin/contracts/interfaces/draft-IERC1822.sol";

// ...existing code...

    /**
     * @notice Override supportsInterface to resolve multiple inheritance
     */
    function supportsInterface(bytes4 interfaceId)
        public
        view
        override(AccessControlUpgradeable)
        returns (bool)
    {
        return interfaceId == type(ITailor).interfaceId ||
        return interfaceId == type(IERC1822Proxiable).interfaceId ||
        super.supportsInterface(interfaceId);
    }

// ...existing code...
LOW severity: Deployment script requires unencrypted private keys. **Description:** Several deployment/ops scripts require private keys to be loaded from environment variables and used directly inside the script, e.g.:

Storing and loading raw private keys via `.env` (plain text) is an operational security risk: keys can be leaked through version control, logs,...

Vulnerable code:
// DeployBasisTradeTailor.s.sol
uint256 deployerPrivateKey = vm.envUint("DEPLOYER_PRIVATE_KEY");
uint256 adminPrivateKey    = vm.envUint("ADMIN_PRIVATE_KEY");
...
vm.startBroadcast(deployerPrivateKey);
...
vm.startBroadcast(adminPrivateKey);

// DeployBasisTradeVault.s.sol
uint256 deployerPrivateKey = vm.envUint("DEPLOYER_PRIVATE_KEY");
...
vm.startBroadcast(deployerPrivateKey);

// DeployMockPocketOracle.s.sol
uint256 deployerPrivateKey = vm.envUint("DEPLOYER_PRIVATE_KEY");
...
vm.startBroadcast(deployerPrivateKey);

// DeployMocks.s.sol
uint256 deployerPrivateKey = vm.envUint("DEPLOYER_PRIVATE_KEY");
...
vm.startBroadcast(deployerPrivateKey);

cast wallet import deployerKey --interactive
cast wallet import adminKey --interactive
cast wallet import agentKey --interactive   # if needed

// before: vm.startBroadcast(deployerPrivateKey);
vm.startBroadcast();
// ...
vm.stopBroadcast();
LOW severity: Lack of check for 0 shares minted. **Description:** `previewDeposit(assets)` can legitimately return `0` shares for tiny deposits due to rounding and/or deposit fees. If the deposit path doesn’t guard against this, a user could transfer assets to the vault and receive 0 shares (an unintended “donation”).

Consider adding a check...

Vulnerable code:
function previewDeposit(uint256 assets) public view virtual override returns (uint256) {
    uint256 fee = _extractFeeFromTotal(assets, depositFeeBps);
    require(fee < assets, "Deposit fee exceeds assets");
    uint256 shares = super.previewDeposit(assets - fee);
    require(shares > 0, "0 shares");
    return shares;
}
HIGH severity: Redundant variable statements. **Description:** In `BasisTradeVault` the functions `maxMint`, `mint`, `withdraw`, and `redeem` are overrides of the standard ERC4626 interface. In this contract, these functions are intentionally disabled to enforce a custom deposit and withdrawal flow (e.g., using `requestWithdraw` and...

Vulnerable code:
// ...existing code...
    /**
     * @notice Mint function is disabled
     * @dev This vault only supports asset-based deposits
     */
    function mint(uint256 /*shares*/, address /*receiver*/) public virtual override returns (uint256) {
        revert("Mint disabled: use deposit");
    }

    // ============================================
// ...existing code...
    /**
     * @notice Standard withdraw function is disabled
     * @dev Users must use requestWithdraw instead
     */
    function withdraw(
        uint256 /*assets*/,
        address /*receiver*/,
        address /*owner*/
    ) public virtual override returns (uint256) {
        revert("Withdraw disabled: use requestWithdraw");
    }

    /**
     * @notice Standard redeem function is disabled
     * @dev Users must use requestRedeem instead
     */
    function redeem(
        uint256 /*shares*/,
        address /*receiver*/,
        address /*owner*/
    ) public virtual override returns (uint256) {
        revert("Redeem disabled: use requestRedeem");
    }

// ...existing code...
    /**
     * @notice Returns the maximum shares that can be minted
     * @dev Always returns 0 as mint is disabled
     * @param receiver Address that would receive the shares
     * @return Always 0 (mint disabled)
     */
    function maxMint(address /*receiver*/) public view virtual override returns (uint256) {
        return 0; // Mint is disabled
    }
}
HIGH severity: Consider enabling on-behalf-of withdrawals. **Description:** `BasisTradeVault`’s withdrawal queue only supports self-initiated requests that always pay the requester (`receiver = msg.sender`). For better ERC-4626 alignment and composability (routers, managers, third party integrations), extend the existing queue API to accept an explicit...

Vulnerable code:
struct WithdrawalRequest {
    address owner;     // share owner whose shares were escrowed
    address receiver;  // recipient of assets on payout
    address caller;    // who enqueued the request
    uint256 shares;
    uint256 assets;
    uint256 timestamp;
}

struct WithdrawalRequested {
    address owner;
    address receiver;
    address caller;
    uint256 shares;
    uint256 assets;
    uint256 timestamp;
}

function requestWithdraw(
    uint256 assets,
    address receiver,
    address owner
) external returns (uint256 queuePosition) {
    require(assets > 0, "Cannot withdraw 0 assets");
    uint256 grossAssets = assets + _calculateFeeAmount(assets, withdrawalFeeBps);
    uint256 shares = _convertToShares(grossAssets, Math.Rounding.Ceil);
    return requestRedeem(shares, receiver, owner);
}

function requestRedeem(
    uint256 shares,
    address receiver,
    address owner
) public requirePocket returns (uint256 queuePosition) {
    address caller = msg.sender;

    require(receiver != address(0), "Invalid receiver");
    require(owner != address(0), "Invalid owner");
    require(shares > 0, "Cannot redeem 0 shares");

    // spend allowance when acting on behalf of `owner`
    if (caller != owner) {
        _spendAllowance(owner, caller, shares);
    }

    // ...

    emit WithdrawalRequested(caller, owner, receiver, assetsAfterFee, shares, fee, queuePosition);
}
HIGH severity: Use named mapping parameters to explicitly note the purpose of keys and values. **Description:** Use named mapping parameters to explicitly note the purpose of keys and values:

* [`BasisTradeTailor`](https://github.com/buttonxyz/button-protocol/blob/9002f2b0d05ba80039bd942c809dbe5bc1a252c9/src/BasisTradeTailor.sol#L41-L47)
  

*...

Vulnerable code:
// Mappings
  /// @notice Maps pocket address to the user who controls it
  mapping(address => address) public pocketUser;
  /// @notice Tracks pending withdrawal amounts for each pocket
  mapping(address => uint256) public withdrawalRequests;
  /// @notice Whitelist of addresses allowed to create pockets
  mapping(address => bool) public creationWhitelist;

mapping(address => bool) public depositWhitelist;

mapping(address => bool) public approvedTailors;
HIGH severity: Remove obsolete return statements when using named return variables. **Description:** Remove either the named return value or the `return` statement.

* [`BasisTradeVault::requestWithdraw`](https://github.com/buttonxyz/button-protocol/blob/9002f2b0d05ba80039bd942c809dbe5bc1a252c9/src/BasisTradeVault.sol#L396-L404)
  

*...

Vulnerable code:
function requestWithdraw(uint256 assets) external returns (uint256 queuePosition) {
      // ...

      return requestRedeem(shares);
  }

function requestRedeem(uint256 shares) public requirePocket returns (uint256 queuePosition) {
      // ...

      return queuePosition;
  }

function exec(address target, bytes calldata data) external onlyOwner returns (bytes memory result) {
      // ...

      return result;
  }
HIGH severity: Redundant `approve(0)` in `BasisTradeVault::depositToTailor`. **Description:** `BasisTradeVault::depositToTailor` grants `tailor` an allowance, calls `tailor.deposit(pocket, amount)`, and then sets the allowance back to zero:

`BasisTradeTailor::deposit` pulls exactly `amount` via `safeTransferFrom(msg.sender, pocket, amount)`, which consumes the entire...

Vulnerable code:
IERC20(asset()).forceApprove(address(tailor), amount);
tailor.deposit(pocket, amount);
IERC20(asset()).forceApprove(address(tailor), 0); // redundant
LOW severity: All rewards can be stolen due to incorrect active liquidity calculations when the current tick is an exact multiple of the tick spacing at the upper end of a liquidity range. **Description:** Consider the scenario in which the current tick, `t`, is an exact multiple of the tick spacing, `s`, residing at the upper bound of the active liquidity range `[t0, t1)`, where: `s = 10`, `t0 = 0`, and `t = t1 = 30`. Execution of a zero-for-one swap causes...

Vulnerable code:
function _advanceToNextDown(TickIteratorDown memory self) private view {
    do {
        (int16 wordPos, uint8 bitPos) =
@>          TickLib.position((self.currentTick - 1).compress(self.tickSpacing));

        if (bitPos == 255) {
            self.currentWord = self.manager.getPoolBitmapInfo(self.poolId, wordPos);
        }

        bool initialized;
        (initialized, bitPos) = self.currentWord.nextBitPosLte(bitPos);
@>      self.currentTick = TickLib.toTick(wordPos, bitPos, self.tickSpacing);
        if (initialized) break;
    } while (self.endTick < self.currentTick);
}

// Distribute remainder to last range and update global accumulator.
unchecked {
    cumulativeGrowthX128 += PoolRewardsLib.getGrowthDelta(taxInEther, liquidity);
    rewards[ticks.poolId].globalGrowthX128 += cumulativeGrowthX128;
}

growthInsideX128
== growthGlobalX128 - rewardsGrowthOutsideX128[T0] - rewardsGrowthOutsideX128[T1]
== (G + g) - 0 - G
== g

    g * L'
== g * L * R / swapTax
== g * L * R / (g * L)
== R
LOW severity: All swaps other than the top-of-block swap will revert. **Description:** For swaps that are not top-of-block, `AngstromL2::afterSwap` short-circuits; however, this occurs too late in the execution with an incorrect `hookDeltaUnspecified` after a debt is erroneously created in the invocation of `_computeAndCollectProtocolSwapFee()`. This happens because.....

Vulnerable code:
function afterSwap(
    address,
    PoolKey calldata key,
    SwapParams calldata params,
    BalanceDelta swapDelta,
    bytes calldata
) external override returns (bytes4, int128 hookDeltaUnspecified) {
    _onlyUniV4();

    PoolId id = key.calldataToId();
@>  uint256 taxInEther = _getSwapTaxAmount();
@>  hookDeltaUnspecified =
        _computeAndCollectProtocolSwapFee(key, id, params, swapDelta, taxInEther);

    Slot0 slot0BeforeSwap = Slot0.wrap(slot0BeforeSwapStore.get());
    Slot0 slot0AfterSwap = UNI_V4.getSlot0(id);
    rewards[id].updateAfterTickMove(
        id, UNI_V4, slot0BeforeSwap.tick(), slot0AfterSwap.tick(), key.tickSpacing
    );

    uint128 blockNumber = _getBlock();
    if (taxInEther == 0 || blockNumber == _blockOfLastTopOfBlock) {
@>      return (this.afterSwap.selector, hookDeltaUnspecified);
    }
    _blockOfLastTopOfBlock = blockNumber;

    params.zeroForOne
        ? _zeroForOneDistributeTax(id, key.tickSpacing, slot0BeforeSwap, slot0AfterSwap)
        : _oneForZeroDistributeTax(id, key.tickSpacing, slot0BeforeSwap, slot0AfterSwap);

    return (this.afterSwap.selector, hookDeltaUnspecified);
}

// Hook must maintain zero balance deltas for all currencies (delta neutral)
invariant hookDeltaNeutrality(env e)
    forall PoolManager.Currency currency.
        ghostCurrencyDeltas[_AngstromL2][currency] == 0

function test_cyfrin_multipleSwaps() public {
    setPriorityFee(0.7 gwei);

    PoolKey memory key = initializePool(address(token), 10, 3);
    setupSimpleZeroForOnePositions(key);

    router.swap(key, true, -100e18, int24(-20).getSqrtPriceAtTick());

    setPriorityFee(0.6 gwei);

    vm.expectRevert("CurrencyNotSettled()");
    router.swap(key, true, -100e18, int24(-35).getSqrtPriceAtTick());
}
LOW severity: All swaps will revert if the dynamic protocol fee is enabled since `hook-config.sol` does not encode the `afterSwapReturnDelta` permission. **Description:** If `AngstromL2::setPoolHookSwapFee` is called by the owner to configure the dynamic hook protocol fee to a non-zero value then the Uniswap V4 delta accounting will result in a revert with `CurrencyNotSettled()`.

This happens because the non-zero fee delta will be accounted to the.....

Vulnerable code:
if (feeCurrencyId == NATIVE_CURRENCY_ID) {
        unclaimedProtocolRevenueInEther += fee.toUint128();
@>      UNI_V4.mint(address(this), feeCurrencyId, fee + taxInEther);
    } else {
@>      UNI_V4.mint(address(this), feeCurrencyId, fee);
        UNI_V4.mint(address(this), NATIVE_CURRENCY_ID, taxInEther);
    }

Hooks.validateHookPermissions(IHooks(address(this)), getRequiredHookPermissions());

if (self.hasPermission(AFTER_SWAP_FLAG)) {
        hookDeltaUnspecified += self.callHookWithReturnDelta(
            abi.encodeCall(IHooks.afterSwap, (msg.sender, key, params, swapDelta, hookData)),
@>          self.hasPermission(AFTER_SWAP_RETURNS_DELTA_FLAG)
        ).toInt128();
    }

    function callHookWithReturnDelta(IHooks self, bytes memory data, bool parseReturn) internal returns (int256) {
        bytes memory result = callHook(self, data);

        // If this hook wasn't meant to return something, default to 0 delta
@>      if (!parseReturn) return 0;

        // A length of 64 bytes is required to return a bytes4, and a 32 byte delta
        if (result.length != 64) InvalidHookResponse.selector.revertWith();
        return result.parseReturnDelta();
    }
MEDIUM severity: Swaps will revert when `A = B + Xhat - x = 0`. **Description:** `CompensationPriceFinder::_zeroForOneGetFinalCompensationPrice` implements two branches depending on the sign of `A`. The following logic executes when it is positive, but note that `A` will equal zero when `sumX` is exactly equal to `rangeVirtualReserves0`, i.e. $$A = B + \hat X...

Vulnerable code:
if (sumX >= rangeVirtualReserves0) {
        // `A` is positive, compute `D = y * (Xhat + B) + A * Yhat`, `p* = (-L + sqrt(D)) / A`.
@>      uint256 a = sumX - rangeVirtualReserves0;
        {
            (uint256 ay1, uint256 ay0) = Math512Lib.fullMul(a, sumUpToThisRange1);
            (d1, d0) = Math512Lib.checkedAdd(d1, d0, ay1, ay0);
        }
        // Compute `sqrtDX96 := sqrt(D) * 2^96 <> sqrt(D * 2^192)`
        (d1, d0) = Math512Lib.checkedMul2Pow192(d1, d0);
        // Reuse `d1, d0` to store numerator `-L + sqrt(D)`.
        (d1, d0) =
            Math512Lib.checkedSub(0, Math512Lib.sqrt512(d1, d0), 0, uint256(liquidity) << 96);
@>      (uint256 upperBits, uint256 p1) = Math512Lib.div512by256(d1, d0, a);
        assert(upperBits == 0);

        return p1.toUint160();
    } else {