Farseen0/scar-pairs huggingface dataset:
HIGH severity: Compliance check discrepancy between `onUSDManager` and `onUSD` transfers. **Description:** When minting or redeeming `onUSD` via `onUSDManager`, the contract extends `BaseRWAManager`, which performs a compliance check using the `onUSD` token address (`address(onUSD)`) as the `rwaToken` identifier. This happens in...

Vulnerable code:
// Reverts if user address is not compliant
ondoCompliance.checkIsCompliant(rwaToken, _msgSender());

function checkIsCompliant(address user) external override {
  compliance.checkIsCompliant(gmIdentifier, user);
}

address public gmIdentifier =
  address(uint160(uint256(keccak256(abi.encodePacked("global_markets")))));
HIGH severity: `OndoSanityCheckOracle::setAllowedDeviationBps` is not checking zero value as input which will introduce problems using it. **Description:** In `OndoSanityCheckOracle`, there are two types of deviation values: a default deviation applied to all tokens by default, and a token-specific deviation set per asset via `setAllowedDeviationBps()`.

The default deviation value is validated to be non-zero, while token-specific...

Vulnerable code:
function setAllowedDeviationBps(...) external onlyRole(CONFIGURER_ROLE) {
  if (bps >= BPS_DENOMINATOR) revert InvalidDeviationBps();
  prices[token].allowedDeviationBps = bps;
  emit AllowedDeviationSet(token, bps);
}

function setDefaultAllowedDeviationBps(...) public onlyRole(CONFIGURER_ROLE) {
  if (bps == 0) revert InvalidDeviationBps(); // enforced here
  if (bps >= BPS_DENOMINATOR) revert InvalidDeviationBps();
  emit DefaultAllowedDeviationSet(defaultDeviationBps, bps);
  defaultDeviationBps = bps;
}

if (priceData.allowedDeviationBps == 0) {
  priceData.allowedDeviationBps = defaultDeviationBps;
  emit AllowedDeviationSet(token, priceData.allowedDeviationBps);
}
HIGH severity: Inconsistent unpause role in `onUSD`. **Description:** [`onUSD::unpause`](https://github.com/ondoprotocol/rwa-internal/blob/a74d03f4a71bd9cac09e8223377b47f7d64ca8d4/contracts/globalMarkets/onUSD.sol#L200-L202) is restricted to `DEFAULT_ADMIN_ROLE`, unlike other contracts in the system that use a dedicated `UNPAUSER_ROLE`. This breaks...

Vulnerable code:
function unpause() public override onlyRole(DEFAULT_ADMIN_ROLE) {
  _unpause();
}
HIGH severity: Unnecessary boolean comparisons in `GMTokenManager`. **Description:** Both in [`GMTokenManager::_verifyQuote#L329`](https://github.com/ondoprotocol/rwa-internal/blob/a74d03f4a71bd9cac09e8223377b47f7d64ca8d4/contracts/globalMarkets/tokenManager/GMTokenManager.sol#L329) and...

Vulnerable code:
if (gmTokenAccepted[gmToken] == false) revert GMTokenNotRegistered();

if (!gmTokenAccepted[gmToken]) revert GMTokenNotRegistered();
HIGH severity: Inconsistent type usage for `IssuanceHours.HOUR_IN_SECONDS`. **Description:** In `IssuanceHours` the constant [`IssuanceHours.HOUR_IN_SECONDS`](https://github.com/ondoprotocol/rwa-internal/blob/a74d03f4a71bd9cac09e8223377b47f7d64ca8d4/contracts/globalMarkets/tokenManager/issuanceHours/IssuanceHours.sol#L38) field is declared as `uint`, while the rest of the.....

Vulnerable code:
/// Constant for the number of seconds in an hour
uint constant HOUR_IN_SECONDS = 3_600;
HIGH severity: Test enhancements. **Description:** * `GMIntegrationTest_GM_ETH`: Both tests [`test_hitRateLimits_onUSDInGMFlow_Subscribe`](https://github.com/ondoprotocol/rwa-internal/blob/a74d03f4a71bd9cac09e8223377b47f7d64ca8d4/forge-tests/globalMarkets/GM_IntegrationTest.t.sol#L1209-L1210) and...

Vulnerable code:
// Should fail due to onUSD rate limit
   vm.expectRevert();
   gmTokenManager.mintWithAttestation(
     quote,
     signature,
     address(USDC),
     usdcAmount
   );

// Should fail due to onUSD rate limit
   -   vm.expectRevert();
   +   vm.expectRevert(OndoRateLimiter.RateLimitExceeded.selector);
       gmTokenManager.mintWithAttestation(
         quote,
         signature,
         address(USDC),
         usdcAmount
       );
HIGH severity: Fees can be stolen from partially unwrapped `UniswapV4Wrapper` positions. **Description:** `ERC721WrapperBase` exposes two overloads of the `unwrap()` function to perform full and partial unwrap of the ERC-6909 position for a given `tokenId`:

The full unwrap assumes the caller owns the entire ERC-6909 token supply and will transfer the underlying Uniswap position after.....

Vulnerable code:
function unwrap(address from, uint256 tokenId, address to) external callThroughEVC {
    _burnFrom(from, tokenId, totalSupply(tokenId));
    underlying.transferFrom(address(this), to, tokenId);
}

function unwrap(address from, uint256 tokenId, address to, uint256 amount, bytes calldata extraData)
    external
    callThroughEVC
{
    _unwrap(to, tokenId, amount, extraData);
    _burnFrom(from, tokenId, amount);
}

function _unwrap(address to, uint256 tokenId, uint256 amount, bytes calldata extraData) internal override {
    (,,,,,,, uint128 liquidity,,,,) = INonfungiblePositionManager(address(underlying)).positions(tokenId);

    (uint256 amount0, uint256 amount1) =
        _decreaseLiquidity(tokenId, proportionalShare(tokenId, uint256(liquidity), amount).toUint128(), extraData);

    (,,,,,,,,,, uint256 tokensOwed0, uint256 tokensOwed1) =
        INonfungiblePositionManager(address(underlying)).positions(tokenId);

    //amount0 and amount1 is the part of the liquidity
    //token0Owed - amount0 and token1Owed - amount1 are the total fees (the principal is always collected in the same tx). part of the fees needs to be sent to the recipient as well

    INonfungiblePositionManager(address(underlying)).collect(
        INonfungiblePositionManager.CollectParams({
            tokenId: tokenId,
            recipient: to,
            amount0Max: (amount0 + proportionalShare(tokenId, (tokensOwed0 - amount0), amount)).toUint128(),
            amount1Max: (amount1 + proportionalShare(tokenId, (tokensOwed1 - amount1), amount)).toUint128()
        })
    );
}

function _unwrap(address to, uint256 tokenId, uint256 amount, bytes calldata extraData) internal override {
    PositionState memory positionState = _getPositionState(tokenId);

    (uint256 pendingFees0, uint256 pendingFees1) = _pendingFees(positionState);
    _accumulateFees(tokenId, pendingFees0, pendingFees1);

    uint128 liquidityToRemove = proportionalShare(tokenId, positionState.liquidity, amount).toUint128();
    (uint256 amount0, uint256 amount1) = _principal(positionState, liquidityToRemove);

    _decreaseLiquidity(tokenId, liquidityToRemove, ActionConstants.MSG_SENDER, extraData);

    poolKey.currency0.transfer(to, amount0 + proportionalShare(tokenId, tokensOwed[tokenId].fees0Owed, amount));
    poolKey.currency1.transfer(to, amount1 + proportionalShare(tokenId, tokensOwed[tokenId].fees1Owed, amount));
}
HIGH severity: Fees can become stuck in `UniswapV4Wrapper`. **Description:** When a modification is made to Uniswap V4 position liquidity, such as in the case of a partial `UniswapV4Wrapper` unwrap which decreases liquidity, any outstanding fees are also transferred and required to be completely settled. For multiple holders of a given ERC-6909 `tokenId`,...

Vulnerable code:
function test_finalLosesFeesPoC() public {
    int256 liquidityDelta = -19999;
    uint256 swapAmount = 100_000 * unit0;

    LiquidityParams memory params = LiquidityParams({
        tickLower: TickMath.MIN_TICK + 1,
        tickUpper: TickMath.MAX_TICK - 1,
        liquidityDelta: liquidityDelta
    });

    (uint256 tokenId1,,) = boundLiquidityParamsAndMint(params);

    startHoax(borrower);
    wrapper.underlying().approve(address(wrapper), tokenId1);
    wrapper.wrap(tokenId1, borrower);
    wrapper.enableTokenIdAsCollateral(tokenId1);
    address borrower2 = makeAddr("borrower2");
    wrapper.transfer(borrower2, tokenId1, wrapper.FULL_AMOUNT() * 5 / 10);

    //swap so that some fees are generated
    swapExactInput(borrower, address(token0), address(token1), swapAmount);

    (uint256 expectedFees0Position1, uint256 expectedFees1Position1) =
        MockUniswapV4Wrapper(payable(address(wrapper))).pendingFees(tokenId1);

    console.log("Expected Fees Position 1: %s, %s", expectedFees0Position1, expectedFees1Position1);

    startHoax(borrower);
    wrapper.unwrap(
        borrower,
        tokenId1,
        borrower,
        wrapper.balanceOf(borrower, tokenId1),
        bytes("")
    );

    console.log("Wrapper balance of currency0: %s", currency0.balanceOf(address(wrapper)));

    startHoax(borrower2);
    wrapper.unwrap(borrower2, tokenId1, borrower2);

    console.log("Wrapper balance of currency0: %s", currency0.balanceOf(address(wrapper)));
    if (currency0.balanceOf(address(wrapper)) > 0 && wrapper.totalSupply(tokenId1) == 0) {
        console.log("Fees stuck in wrapper!");
    }
}
HIGH severity: Users can enable `tokenIds` as collateral even if they do not own any of the ERsupply. **Description:** It is possible for users to enable `tokenIds` that that they do not own as collateral. Based on the current remediated logic, it does not appear possible for this to be further weaponized beyond the blocking of liquidations, as described in C-01, stemming from the incorrect...

Vulnerable code:
function test_enableCollateralPoC() public {
    LiquidityParams memory params = LiquidityParams({
        tickLower: TickMath.MIN_TICK + 1,
        tickUpper: TickMath.MAX_TICK - 1,
        liquidityDelta: -19999
    });

    (tokenId,,) = boundLiquidityParamsAndMint(params, borrower);

    startHoax(borrower);
    wrapper.underlying().approve(address(wrapper), tokenId);
    // avoid wrapping such that ERC-6909 total supply remains zero
    assertEq(wrapper.totalSupply(tokenId), 0);

    address borrower2 = makeAddr("borrower2");
    startHoax(borrower2);
    assertEq(wrapper.getEnabledTokenIds(borrower2).length, 0);
    wrapper.enableTokenIdAsCollateral(tokenId);
    assertEq(wrapper.getEnabledTokenIds(borrower2).length, 1);
}

+   error TokenIdNotOwnedByBorrower(uint256 tokenId, address sender);

    function enableTokenIdAsCollateral(uint256 tokenId) public returns (bool enabled) {
        address sender = _msgSender();
        enabled = _enabledTokenIds[sender].add(tokenId);
        if (totalTokenIdsEnabledBy(sender) > MAX_TOKENIDS_ALLOWED) revert MaximumAllowedTokenIdsReached();
+       if (balanceOf(sender, tokenId) == 0) revert TokenIdNotOwnedByBorrower(tokenId, sender);
        if (enabled) emit TokenIdEnabled(sender, tokenId, true);
    }
HIGH severity: Underlying liquidity position is not transferred when fully unwrapping through the partial unwrap overload. **Description:** While the `ERC721WrapperBase` partial unwrap overload is intended for use following partial liquidations, it is possible for the sole holder of an ERC-6909 token to use this function to perform a full unwrap by specifying the the total supply of the corresponding `tokenId`:

The...

Vulnerable code:
function unwrap(address from, uint256 tokenId, address to, uint256 amount, bytes calldata extraData)
    external
    callThroughEVC
{
    _unwrap(to, tokenId, amount, extraData);
    _burnFrom(from, tokenId, amount);
}

function unwrap(address from, uint256 tokenId, address to) external callThroughEVC {
    _burnFrom(from, tokenId, totalSupply(tokenId));
    underlying.transferFrom(address(this), to, tokenId);
}

// ERC721WrapperBase
function _burnFrom(address from, uint256 tokenId, uint256 amount) internal {
    address sender = _msgSender();
    if (from != sender && !isOperator(from, sender)) {
        _spendAllowance(from, sender, tokenId, amount);
    }
    _burn(from, tokenId, amount);
}

// ERC6909
function _burn(address from, uint256 id, uint256 amount) internal {
    if (from == address(0)) {
        revert ERC6909InvalidSender(address(0));
    }
    _update(from, address(0), id, amount);
}

// ERC721WrapperBase
function _update(address from, address to, uint256 id, uint256 amount) internal virtual override {
    super._update(from, to, id, amount);
    if (from != address(0)) evc.requireAccountStatusCheck(from);
}

// ERC6909
function _update(address from, address to, uint256 id, uint256 amount) internal virtual {
    address caller = _msgSender();

    if (from != address(0)) {
        uint256 fromBalance = _balances[from][id];
        if (fromBalance < amount) {
            revert ERC6909InsufficientBalance(from, fromBalance, amount, id);
        }
        unchecked {
            // Overflow not possible: amount <= fromBalance.
            _balances[from][id] = fromBalance - amount;
        }
    }
    if (to != address(0)) {
        _balances[to][id] += amount;
    }

    emit Transfer(caller, from, to, id, amount);
}
HIGH severity: Missing access control on critical `FeeController` setters. **Description:** `FeeController.setFunctionFeeConfig()`, `setTokenGetter()`, and `setGlobalTokenGetter()` are declared `external` but have **no modifier** restricting the caller. Any account can freely (re)configure fee percentages, token-getter addresses, or even swap in malicious...

Vulnerable code:
/// @inheritdoc IFeeController
function setFunctionFeeConfig(bytes4 _selector, FeeType _feeType, uint256 _feePercentage) external {
    if (_feePercentage > maxFeeLimits[_feeType]) {
        revert FeePercentageExceedLimit();
    }

    functionFeeConfigs[_selector] = FeeConfig(_feeType, _feePercentage);

    emit FeeConfigSet(_selector, _feeType, _feePercentage);
}

/// @inheritdoc IFeeController
function setTokenGetter(bytes4 _selector, address _tokenGetter, address _target) external {
    if (_target == address(0) || _tokenGetter == address(0)) {
        revert ZeroAddressNotValid();
    }

    tokenGetters[_target][_selector] = _tokenGetter;
    emit TokenGetterSet(_target, _selector, _tokenGetter);
}

/// @inheritdoc IFeeController
function setGlobalTokenGetter(bytes4 _selector, address _tokenGetter) external {
    if (_tokenGetter == address(0)) {
        revert ZeroAddressNotValid();
    }

    globalTokenGetters[_selector] = _tokenGetter;
    emit GlobalTokenGetterSet(_selector, _tokenGetter);
}
HIGH severity: Fee calculations break when token decimals are different from 18. **Description:** `FeeController.calculateFee()` and `calculateTokenAmount()` implicitly treat every ERC-20 as if it had **18 decimals**, but many popular assets (USDC/USDT = 6, etc.) do not.
Inside `calculateFee` the line

divides by `1e18` to cancel the **oracle’s** 18-dec scaling, but completely.....

Vulnerable code:
uint256 _feeInUSD = _feeAmount * _tokenPrice / 10**18;

total = 1e6  (USDT)  + 1e18 (DAI)  = 1000000000001000000  wei-USD

uint8 decimals = IERC20Metadata(token).decimals();   // OZ ERC20Metadata
   uint256 scale = 10 ** decimals;                      // token’s native unit

   // feeAmount is already in token-units
   uint256 _feeInUSD = _feeAmount * _tokenPrice / scale;  // always 18-dec USD
MEDIUM severity: Missing staleness check in `PriceOracle::getTokenPrice`. **Description:** `PriceOracle.getTokenPrice()` relays `pythOracle.getPriceUnsafe()` but never verifies the price **publish time** or confidence interval. A price that is hours old (or intentionally frozen) is accepted as fresh.

**Impact:** * Automation fees may be computed from obsolete data,...

Vulnerable code:
function getTokenPrice(address _token) external view returns (uint256) {
    bytes32 _oracleID = oracleIDs[_token];

    if (_oracleID == bytes32(0)) {
        revert OracleNotExist(_token);
    }

    PythStructs.Price memory price = pythOracle.getPriceUnsafe(_oracleID);

    return _scalePythPrice(price.price, price.expo);
}
HIGH severity: Automation DoS via blacklisted or reverting fee recipients. **Description:** `FeeHandler.handleFee()` uses `safeTransferFrom()` to forward ERC-20 tokens to `beneficiary`, `creator`, `vault`, and `burnerAddress`. If any of those addresses are **black-listed** by USDT/USDC (transfer returns false) the call reverts.
`handleFeeETH()` uses `transfer()` which...

Vulnerable code:
IERC20(token).safeTransferFrom(msg.sender, beneficiary, beneficiaryAmount);

if (creator != address(0)) {
    IERC20(token).safeTransferFrom(msg.sender, creator, creatorAmount);
    IERC20(token).safeTransferFrom(msg.sender, vault, vaultAmount);
} else {
    IERC20(token).safeTransferFrom(msg.sender, vault, vaultAmount + creatorAmount);
}

if (burnAmount > 0) {
    IERC20(token).safeTransferFrom(msg.sender, burnerAddress, burnAmount);
}

payable(beneficiary).transfer(beneficiaryAmount);

if (creator != address(0)) {
    payable(creator).transfer(creatorAmount);
    payable(vault).transfer(vaultAmount);
} else {
    payable(vault).transfer(vaultAmount + creatorAmount);
}

if (burnAmount > 0) {
    payable(burnerAddress).transfer(burnAmount);
}
MEDIUM severity: DoS of strategy execution due to array indices out of bounds. **Description:** `StrategyBuilderPlugin._validateStep()` intends to enforce that the condition results reference a new index that does not exceed the maximum step index:

However, given `maxStepIndex` is passed as the array length, this fails to account for zero indexing and as a it is possible to.....

Vulnerable code:
function _validateStep(StrategyStep memory step, uint256 maxStepIndex) internal pure {
    if (step.condition.result0 > maxStepIndex || step.condition.result1 > maxStepIndex) {
        revert InvalidNextStepIndex();
    }
}

function test_executeStrategy_OOB() external {
    uint256 numSteps = 2;
    IStrategyBuilderPlugin.StrategyStep[] memory steps = _createStrategySteps(numSteps);
    steps[0].condition.result0 = 2;
    steps[0].condition.result1 = 2;
    uint32 strategyID = 222;

    deal(address(account1), 100 ether);

    //Mocks
    vm.mockCall(
        feeController,
        abi.encodeWithSelector(IFeeController.getTokenForAction.selector),
        abi.encode(address(0), false)
    );

    vm.mockCall(
        feeController,
        abi.encodeWithSelector(IFeeController.functionFeeConfig.selector),
        abi.encode(IFeeController.FeeConfig({feeType: IFeeController.FeeType.Deposit, feePercentage: 0}))
    );
    vm.mockCall(feeController, abi.encodeWithSelector(IFeeController.minFeeInUSD.selector), abi.encode(0));

    //Act
    vm.startPrank(address(account1));
    strategyBuilderPlugin.createStrategy(strategyID, creator, steps);

    strategyBuilderPlugin.executeStrategy(strategyID);
    vm.stopPrank();

    //Assert
    assertEq(tokenReceiver.balance, numSteps * 2 * TOKEN_SEND_AMOUNT);
}

function _validateStep(StrategyStep memory step, uint256 maxStepIndex) internal pure {
--      if (step.condition.result0 > maxStepIndex || step.condition.result1 > maxStepIndex) {
++      if (step.condition.result0 >= maxStepIndex || step.condition.result1 >= maxStepIndex) {
            revert InvalidNextStepIndex();
        }
    }
MEDIUM severity: Volume-based fee can be bypassed with a wrapper contract. **Description:** `_executeAction()` levies a percentage fee only when `FeeController.getTokenForAction()` can map the `(target, selector)` pair to a tracked token. If the mapping is absent it falls back to `minFeeInUSD`.
An attacker can therefore wrap any high-value call inside a helper contract...

Vulnerable code:
User → StrategyBuilder → AAVEHandler.supplyFor(100 000 USDC) → Aave Pool.supply()

function _executeAction(address _wallet, Action memory _action) internal returns (uint256 feeInUSD) {
    (address tokenToTrack, bool exist) =
        feeController.getTokenForAction(_action.target, _action.selector, _action.parameter);
    // If the volume token exist track the volume before and after the execution, else get the min fee

    uint256 preExecBalance = exist ? IERC20(tokenToTrack).balanceOf(_wallet) : 0;

    _execute(_wallet, _action);

    IFeeController.FeeType feeType = feeController.functionFeeConfig(_action.selector).feeType;

    if (exist) {
        uint256 postExecBalance = IERC20(tokenToTrack).balanceOf(_wallet);
        uint256 volume = feeType == IFeeController.FeeType.Deposit
            ? preExecBalance - postExecBalance
            : postExecBalance - preExecBalance;

        feeInUSD = feeController.calculateFee(tokenToTrack, _action.selector, volume);
    } else {
        feeInUSD = feeController.minFeeInUSD(feeType);
    }

    emit ActionExecuted(_wallet, _action);
}
LOW severity: Accumulation of dust amounts due to rounding in `_tokenDistribution()`. **Description:** `_tokenDistribution()` computes each share independently with integer division:

If `beneficiaryPercentage + creatorPercentage + vaultPercentage == 10000` but any division truncates, `beneficiaryAmount + creatorAmount + vaultAmount < amount`, leaving 1–2 wei of...

Vulnerable code:
function _tokenDistribution(uint256 amount) internal view returns (uint256, uint256, uint256) {
    uint256 beneficiaryAmount = (amount * beneficiaryPercentage) / PERCENTAGE_DIVISOR;
    uint256 creatorAmount = (amount * creatorPercentage) / PERCENTAGE_DIVISOR;
    uint256 vaultAmount = (amount * vaultPercentage) / PERCENTAGE_DIVISOR;
    return (beneficiaryAmount, creatorAmount, vaultAmount);
}

vaultAmount = amount - beneficiaryAmount - creatorAmount;
LOW severity: `automationsToIndex` storage is not correctly reset when deleting automations. **Description:** When deleting an automation, the logic pops the final element from the `_usedInAutomations` array but fails to correctly reset the `automationsToIndex` mapping storage. Regardless of whether the if statement executes, `automationsToIndex[automationSID]` should be reset to zero...

Vulnerable code:
function _deleteAutomation(address wallet, uint32 id) internal {
        bytes32 automationSID = getStorageId(wallet, id);
        Automation memory _automation = automations[automationSID];

        uint32[] storage _usedInAutomations = strategiesUsed[getStorageId(wallet, _automation.strategyId)];

@>      uint32 _actualAutomationIndex = automationsToIndex[automationSID];
        uint256 _lastAutomationIndex = _usedInAutomations.length - 1;
        if (_actualAutomationIndex != _lastAutomationIndex) {
            uint32 _lastAutomation = _usedInAutomations[_lastAutomationIndex];
            _usedInAutomations[_actualAutomationIndex] = _lastAutomation;
@>          automationsToIndex[getStorageId(wallet, _lastAutomation)] = _actualAutomationIndex;
        }
        _usedInAutomations.pop();

        _changeAutomationInCondition(
            wallet, _automation.condition.conditionAddress, _automation.condition.id, id, false
        );

        delete automations[automationSID];

        emit AutomationDeleted(wallet, id);
    }
HIGH severity: Multiple `vault_gkhan_account` Can be Used Resulting in DoS on `complete_vest`. **Description:** The program validates `vault_gkhan_account` only by checking that

* `vault_gkhan_account.owner == state.key()` and
* `vault_gkhan_account.mint  == state.belo_mint`

It **does not** verify that the account is the canonical vault ATA created via the Associated Token Account (ATA)...

Vulnerable code:
let (vault_key, _bump) = Pubkey::find_program_address(
      &[b"vault", state.belo_mint.as_ref()],
      program_id,
  );
  require!(vault_gkhan_account.key() == vault_key, ErrorCode::InvalidTokenAccount);