Farseen0/scar-pairs huggingface dataset:
LOW severity: TRST-Insufficient dust checks. **Description:**
In `hedgeDelta()`, there is a dust check in the case of sell wETH order:

However, the actual used amount is _delta

The check should be applied on deltaToUse rather than wethBalance because it will be the 
minimum of wethBalance and _delta.
Additionally, there is no corresponding.....

Vulnerable code:
// sell wETH
             uint256 wethBalance = inversed ? amount1Current : amount0Current;
        if (wethBalance < minAmount) return 0;

uint256 deltaToUse = _delta > int256(wethBalance) ? wethBalance : 
           uint256(_delta);
        _createUniswapRangeOrder(rangeOrder, deltaToUse, inversed);
LOW severity: TRST-Lack of logging in important functions. **Description:**
For the sake of transparency, it is recommended to emit events during maintenance transfer 
of funds into and out of contracts. Make sure to add events for `withdraw()`, `recoverETH()` and 
`recoverERC20()`.

**Recommended Mitigation:**
Add the events listed above.

**Team...

Vulnerable code:
if (_amount <= balance) {
             SafeTransferLib.safeTransfer(ERC20(collateralAsset), msg.sender, _amount);
        emit Withdraw(_amount);
        // return in collateral format
             return _amount;
                 } else {
        SafeTransferLib.safeTransfer(ERC20(collateralAsset), msg.sender, balance);
        emit Withdraw(_amount);
        // return in collateral format
                return balance;
             }
LOW severity: TRST-_getUnderlyingBalances() does unnecessary computation when not in active position. **Description:**
If the RangeOrderReactor contract is not currently active, it should simply return the current 
token balances. However, it does a lot of expensive logic to calculate position value.

**Recommended mitigation:**
Perform early exit in case position is not active.

**Team...

Vulnerable code:
(uint128 liquidity, uint256 feeGrowthInside0Last, uint256 feeGrowthInside1Last, uint128 tokensOwed0,
        uint128 tokensOwed1) = pool.positions(_getPositionID());
        // compute current holdings from liquidity
             (amount0Current, amount1Current) = 
        LiquidityAmounts.getAmountsForLiquidity(
        sqrtRatioX96,
        currentPosition.activeLowerTick.getSqrtRatioAtTick(),
             currentPosition.activeUpperTick.getSqrtRatioAtTick(),
        liquidity
        );
        // compute current fees earned
                    uint256 fee0 =
                _computeFeesEarned(true, feeGrowthInside0Last, tick, liquidity) +
            uint256(tokensOwed0);
        uint256 fee1 =
        _computeFeesEarned(false, feeGrowthInside1Last, tick, liquidity) 
        +
        uint256(tokensOwed1);
HIGH severity: TRST-Attacker can freeze profit withdrawals from V3 vaults. **Description:**
Users of Ninja can use Vault's `withdrawProfit()` to withdraw profits. It starts with the 
following check:
 

If attacker can front-run user's `withdrawProfit()` TX and set **lastProfitTime** to 
block.timestamp, they would effectively freeze the user's yield. That is indeed...

Vulnerable code:
if (block.timestamp <= lastProfitTime) {
      revert NYProfitTakingVault__ProfitTimeOutOfBounds();
      }

function harvest() external override whenNotPaused returns (uint256 callerFee) {
            require(lastHarvestTimestamp != block.timestamp);
                uint256 harvestSeconds = lastHarvestTimestamp > 0 ? block.timestamp 
            - lastHarvestTimestamp : 0;
      lastHarvestTimestamp = block.timestamp;
                uint256 sentToVault;
          uint256 underlyingTokenCount;
     (callerFee, underlyingTokenCount, sentToVault) = _harvestCore();
            emit StrategyHarvest(msg.sender, underlyingTokenCount, 
                   harvestSeconds, sentToVault);
                }

function _harvestCore() internal override returns (uint256 callerFee, uint256 underlyingTokenCount,      uint256 sentToVault)
            {
        IMasterChef(SPOOKY_SWAP_FARM_V2).deposit(POOL_ID, 0);
            _swapFarmEmissionTokens();
                callerFee = _chargeFees();
                    underlyingTokenCount = balanceOf();
                       sentToVault = _sendYieldToVault();
            }
HIGH severity: TRST-Lack of child rewarder reserves could lead to freeze of funds. **Description:**
In ComplexRewarder.sol, `onReward()` is used to distribute rewards for previous time period, 
using the complex rewarder and any child rewarders. If the complex rewarder does not have 
enough tokens to hand out the reward, it correctly stores the rewards owed in storage. 
However,.....

Vulnerable code:
function onReward(uint _pid, address _user, address _to, uint, uint _amt) external override onlyParent nonReentrant {
      PoolInfo memory pool = updatePool(_pid);
         if (pool.lastRewardTime == 0) return;
            UserInfo storage user = userInfo[_pid][_user];
            uint pending;
         if (user.amount > 0) {
              pending = ((user.amount * pool.accRewardPerShare) / ACC_TOKEN_PRECISION) - user.rewardDebt;
      rewardToken.safeTransfer(_to, pending);
         }
         user.amount = _amt;
         user.rewardDebt = (_amt * pool.accRewardPerShare) / 
            ACC_TOKEN_PRECISION;
      emit LogOnReward(_user, _pid, pending, _to);
      }

uint len = childrenRewarders.length();
         for (uint i = 0; i < len; ) {
      IRewarder(childrenRewarders.at(i)).onReward(_pid, _user, _to, 0, 
         _amt);
      unchecked {
         ++i;
         }
      }
HIGH severity: TRST-Wrong accounting of user's holdings allows theft of reward. **Description:**
In `deposit()`, `withdraw()` and `withdrawProfit()`, `rewarder.onReward()` is called for reward 
bookkeeping. It will transfer previous eligible rewards and update the current amount user 
has:

In `withdraw()`, there is a critical issue where `onReward()` is called too...

Vulnerable code:
user.amount = _amt;
      user.rewardDebt = (_amt * pool.accRewardPerShare) / ACC_TOKEN_PRECISION;
      user.rewardsOwed = rewardsOwed;

// Update rewarder for this user
          if (address(rewarder) != address(0)) {
      rewarder.onReward(0, msg.sender, msg.sender, pending, user.amount);
      }
      // Burn baby burn
            _burn(msg.sender, _shares);
      // User accounting
                uint256 userAmount = balanceOf(msg.sender);
      // - Underlying (Frontend ONLY)
            if (userAmount == 0) {
               user.amount = 0;
         } else {
            user.amount -= r;
         }
HIGH severity: TRST-Unsafe transferFrom breaks compatibility with 100s of ERtokens. **Description:**
In Ninja vaults, the delegated strategy sends profit tokens to the vault using 
`depositProfitTokenForUsers()`. The vault transfers the tokens in using:

The issue is that the code doesn't use the `safeTransferFrom()` utility from SafeERC20. 
Therefore, profitTokens that don't...

Vulnerable code:
// Now pull in the tokens (Should have permission)
          // We only want to pull the tokens with accounting
                profitToken.transferFrom(strategy, address(this), _amount);
          emit ProfitReceivedFromStrategy(_amount);
MEDIUM severity: TRST-Attacker can force partial withdrawals to fail. **Description:**
In Ninja vaults, users call `withdraw()` to take back their deposited tokens. There is 
bookkeeping on remaining amount:

If the withdraw is partial (some tokens are left), user.amount is decremented by r.

Above, r is calculated as the relative share of the user's _shares of the...

Vulnerable code:
uint256 userAmount = balanceOf(msg.sender);
         // - Underlying (Frontend ONLY)
            if (userAmount == 0) {
            user.amount = 0;
         } else {
         user.amount -= r;
      }

uint256 r = (balance() * _shares) / totalSupply();

function deposit(uint256 _amount) public nonReentrant {
      …
            user.amount += _amount;
      …
         }
MEDIUM severity: TRST-Rewards may be stuck due to unchangeable slippage parameter. **Description:**
In NyPtvFantomWftmBooSpookyV2StrategyToUsdc.sol, MAX_SLIPPAGE is used to limit 
slippage in trades of BOO tokens to USDC, for yield:

If slippage is not satisfied the entire transaction reverts. Since **MAX_SLIPPAGE** is constant, it 
is possible that harvesting of the strategy...

Vulnerable code:
function _swapFarmEmissionTokens() internal { IERC20Upgradeable boo = IERC20Upgradeable(BOO);
            uint256 booBalance = boo.balanceOf(address(this));
      if (booToUsdcPath.length < 2 || booBalance == 0) {
         return;
      }
         boo.safeIncreaseAllowance(SPOOKY_ROUTER, booBalance);
             uint256[] memory amounts = 
      IUniswapV2Router02(SPOOKY_ROUTER).getAmountsOut(booBalance, booToUsdcPath);
          uint256 amountOutMin = (amounts[amounts.length - 1] * MAX_SLIPPAGE) / PERCENT_DIVISOR;
            IUniswapV2Router02(SPOOKY_ROUTER).swapExactTokensForTokensSupportingFeeOnTransferTokens( booBalance, amountOutMin, booToUsdcPath, address(this), block.timestamp );
                }
MEDIUM severity: TRST-truncation in reward calculation could cause leakage of rewards. **Description:**
In ComplexRewarder.sol, `updatePool()` call updates values in the specified pool.

**reward** could be a fairly large number. The decimals of **reward * ACC_TOKEN_PRECISION** is 
10**30, because of how ACC_TOKEN_PRECISION is defined:

**lpSupply** is given in LP's decimals, but...

Vulnerable code:
uint lpSupply = IVault(VAULT).balance();
          if (lpSupply > 0) {
      uint time = block.timestamp - pool.lastRewardTime;
                uint reward = totalAllocPoint == 0 ? 0 : ((time * rewardPerSecond * 
                  pool.allocPoint) / totalAllocPoint);
               pool.accRewardPerShare = pool.accRewardPerShare + uint128((reward * 
          ACC_TOKEN_PRECISION) / lpSupply);
      }

ACC_TOKEN_PRECISION = 10 ** (30 - decimalsRewardToken);
MEDIUM severity: TRST-potential overflow in reward accumulator may freeze functionality. **Description:** 
Note the above description of `updatePool()` functionality. We can see that **accRewardPerShare** is only allocated 128 bits in **PoolInfo:**

Therefore, even if truncation issues do not occur, it is likely that continuous incrementation 
of the counter would cause...

Vulnerable code:
struct PoolInfo {
          uint128 accRewardPerShare;
            uint64 lastRewardTime;
               uint64 allocPoint;
LOW severity: TRST-when using fee-on-transfer tokens in VaultV3, capacity is limited below underlyingCap. **Description:**
Vault V3 documentation states it accounts properly for fee-on-transfer tokens. It calculates 
actual transferred amount as below:

A small issue is that underlyingCap is compared to the _amount before correction for actual 
transferred amount. Therefore, it cannot actually be...

Vulnerable code:
uint256 _pool = balance();
           if (_pool + _amount > underlyingCap) {
      revert NYProfitTakingVault__UnderlyingCapReached(underlyingCap);
            }
      uint256 _before = underlying.balanceOf(address(this));
            underlying.safeTransferFrom(msg.sender, address(this), _amount);
               uint256 _after = underlying.balanceOf(address(this));
                   _amount = _after - _before;
LOW severity: TRST-Redundant checks in Vault V3. **Description:**
`depositProfitTokenForUsers()` and `withdrawProfit()` contain the following check:

However, lastProfitTime is only ever set to block.timestamp. Therefore, it can never be 
larger than block.timestamp.

**Recommended Mitigation:**
It would be best in terms of gas costs and logical.....

Vulnerable code:
if (block.timestamp <= lastProfitTime) {
       revert NYProfitTakingVault__ProfitTimeOutOfBounds();
          }
LOW severity: TRST-Hard-coding Uniswap path assumptions. **Description:**
In NyPtvFantomWftmBooSpookyV2StrategyToUsdc.sol, estimateHarvest() is used to check if 
harvesting is profitable.

Note that the code assumes `getAmountsOut()` will return USDC amount at index [1]. That is 
indeed the case right now, as **booToUsdcPath = [BOO, USDC];** However, it.....

Vulnerable code:
function estimateHarvest() external view override returns (uint256 profit, uint256 callFeeToUser) {
         uint256 pendingReward = 
         IMasterChef(SPOOKY_SWAP_FARM_V2).pendingBOO(POOL_ID, address(this));
            uint256 totalRewards = pendingReward + 
      IERC20Upgradeable(BOO).balanceOf(address(this));
      if (totalRewards != 0) {
         profit += 
      IUniswapV2Router02(SPOOKY_ROUTER).getAmountsOut(totalRewards, booToUsdcPath)[1];
      }
               profit += IERC20Upgradeable(USDC).balanceOf(address(this));
            uint256 usdcFee = (profit * totalFee) / PERCENT_DIVISOR;
         callFeeToUser = (usdcFee * callFee) / PERCENT_DIVISOR;
      profit -= usdcFee;
      }

uint256[] memory amounts = 
         IUniswapV2Router02(SPOOKY_ROUTER).getAmountsOut(booBalance, booToUsdcPath);
      uint256 amountOutMin = (amounts[amounts.length - 1] * MAX_SLIPPAGE) / PERCENT_DIVISOR;
LOW severity: TRST-Strategy deployer has privileges intended only for multisig addresses.. **Description:** 
When V3 Strategy is initialized, different roles are given to privileged addresses:

As the previous @audit note says, it is really important to not permit msg.sender to be 
**DEFAULT_ADMIN_ROLE**. In fact, this role gives arbitrary power to the EOA, to remove the 
multisig, to...

Vulnerable code:
for (uint256 i = 0; i < _strategists.length; i++) {
            _grantRole(STRATEGIST, _strategists[i]);
               }
               _grantRole(DEFAULT_ADMIN_ROLE, msg.sender); // @audit - Is this a security risk? Default admin roles is REALLY powerful! Consider removing
            _grantRole(DEFAULT_ADMIN_ROLE, _multisigRoles[0]);
         _grantRole(ADMIN, _multisigRoles[1]);
         _grantRole(GUARDIAN, _multisigRoles[2]);
LOW severity: TRST-Strategy upgrade cooldown is set too low. **Description:** 
**UPGRADE_TIMELOCK** defines how long admin must wait before upgrading the contract. 

It is currently set to 1 hour, which is far too low in relation to its importance and funds at 
risk. We recommend a value of 24 hours at minimum to give the community a chance to 
respond to a.....

Vulnerable code:
function _authorizeUpgrade(address) internal override {
            _atLeastRole(DEFAULT_ADMIN_ROLE);
      require(upgradeProposalTime + UPGRADE_TIMELOCK < block.timestamp);
          clearUpgradeCooldown();
             }
HIGH severity: TRST-disordered fee calculated causes collateral changes to be inaccurate. **Description:**
`_increasePosition()` changes the Hedger’s GMX position by **sizeDelta** amount and 
**collateralDelta** collateral. There are two **collateralDelta** corrections - one for swap fees and 
one for position fees. Since the swap fee depends on up-to-date **collateralDelta**, it’s...

Vulnerable code:
if (isLong) {
          uint swapFeeBP = getSwapFeeBP(isLong, true, collateralDelta);
           collateralDelta = (collateralDelta * (BASIS_POINTS_DIVISOR + swapFeeBP)) / BASIS_POINTS_DIVISOR;
      }
      // add margin fee
      // when we increase position, fee always got deducted from collateral
          collateralDelta += _getPositionFee(currentPos.size, sizeDelta, currentPos.entryFundingRate);
MEDIUM severity: TRST-small LP providers may be unable to withdraw their deposits. **Description:**
In LiquidityPool’s initiateWithdraw(), it’s required that withdrawn value is above a minimum 
parameter, or that withdrawn tokens is above the minimum parameter.

The issue is that **minDepositWithdraw** is measured in dollars while **amountLiquidityToken** is 
LP tokens. The...

Vulnerable code:
if (withdrawalValue < lpParams.minDepositWithdraw && 
          amountLiquidityToken < lpParams.minDepositWithdraw) {
      revert MinimumWithdrawNotMet(address(this), withdrawalValue, lpParams.minDepositWithdraw);
      }
MEDIUM severity: TRST-base to quote swaps trust GMX-provided minPrice and maxPrice to be correct, which may be manipulated. **Description:**
exchangeFromExactBase() in GMXAdapter converts an amount of base to quote. It 
implements slippage protection by using the GMX vault’s getMinPrice() and getMaxPrice() 
utilities. However, such protection is insufficient because GMX prices may be manipulated. 
Indeed, GMX supports...

Vulnerable code:
uint tokenInPrice = _getMinPrice(address(baseAsset));
        uint tokenOutPrice = _getMaxPrice(address(quoteAsset));
    ...
    uint minOut = tokenInPrice
      .multiplyDecimal(marketPricingParams[_optionMarket].minReturnPercent)
        .multiplyDecimal(_amountBase)
          .divideDecimal(tokenOutPrice);
HIGH severity: TRST-sendAllFundsToLP() does not handle popular ERtokens like BNB. **Description:**
sendAllFundsToLP() is used to transfer quote and base tokens to the LP after interaction with 
GMX. It uses an unsafe transfer call:

There are a great many tokens such as BNB and USDT that for historical reasons, don’t return 
a value in transfer(). Since Lyra aims to support...

Vulnerable code:
if (baseBal > 0) {
       if (!baseAsset.transfer(address(liquidityPool), baseBal)) {
    revert AssetTransferFailed(address(this), baseAsset, baseBal, 
         address(liquidityPool));
     }
    emit BaseReturnedToLP(baseBal);
HIGH severity: TRST-recoverFunds() does not handle popular ERtokens like BNB. **Description:** 
recoverFunds() is used for recovery in case of mistakenly-sent tokens. However, it uses unsafe 
transfer to send tokens back, which will not support 100s of non-compatible ERC20 tokens. 
Therefore it is likely unsupported tokens will be unrecoverable.

**Recommended...

Vulnerable code:
if (token == quoteAsset || token == baseAsset || token == weth) {
      revert CannotRecoverRestrictedToken(address(this));
    }
        token.transfer(recipient, token.balanceOf(address(this)));
MEDIUM severity: TRST-protectedQuote can be manipulated by calling processDepositQueue when large price moves in base asset occur. **Description:**
The protectedQuote storage variable ensures that a portion of the liquidity pool is preserved 
even in the case of a “contract adjustment event” (i.e. when the pool has become insolvent). 
However, the protectedQuote is updated on deposits and withdraws using the following...

Vulnerable code:
protectedQuote = (liquidity.NAV - withdrawalValue).multiplyDecimal(
       DecimalMath.UNIT - lpParams.adjustmentNetScalingFactor
      );
MEDIUM severity: TRST-canHedge may return true when there is insufficient GMX liquidity to facilitate hedging, causing insolvency risks. **Description:**
Lyra’s security model relies on being able to hedge and achieve delta-neutrality when opening 
a user position. The check is done in canHedge() in GMXFuturesPoolHedger. After expected 
hedge delta and current hedge delta are fetched, remainingDeltas is assigned the amount of...

Vulnerable code:
uint absHedgeDiff = (Math.abs(expectedHedge) - Math.abs(currentHedge));
      if (remainingDeltas < 
           absHedgeDiff.multiplyDecimal(futuresPoolHedgerParams.marketDepthBuffer)) {
        return false;
      }
MEDIUM severity: TRST-canHedge will return true when trade moves delta in same direction as expectedHedge even when that leads to more risk. **Description:**
Lyra’s security model relies on being able to hedge and achieve delta-neutrality when opening 
a user position. The check is done in canHedge() in GMXFuturesPoolHedger. The function will 
return true when the delta of the trade has the same sign as the expectedHedge. For...

Vulnerable code:
// expected hedge is positive, and trade increases delta of the pool - risk is reduced, so accept trade
    if (increasesPoolDelta && expectedHedge >= 0) {
        return true;
          }
MEDIUM severity: TRST-Decreasing a losing hedge position could make it overly-leveraged. **Description:**
It may be necessary to decrease a position in order to be delta neutral. When 
GMXFuturesPoolHedger does that, it also decreases the collateral so that the leverage ratio 
would equal the set **targetLeverage**. In GMX, fees are deducted from collateral when losses 
are realized....

Vulnerable code:
if (currentPos.unrealisedPnl < 0) {
          uint adjustedDelta = Math.abs(currentPos.unrealisedPnl).multiplyDecimal(sizeDelta)divideDecimal      (currentPos.size);
      if (adjustedDelta > collateralDelta) {
          collateralDelta = 0;
      } else {
            collateralDelta -= adjustedDelta;
           }
       }
LOW severity: TRST-Attacker can take over GMXAdapter implementation contract. **Description:**
GMXAdapter inherits from BaseExchangeAdapter. It is an implementation contract for a 
transparent proxy and has the following initializer:
 
Therefore, an attacker can call initialize() on the implementation contract and become the 
owner. At this point they can do just about...

Vulnerable code:
function initialize() external initializer {
         __Ownable_init();
       }
LOW severity: TRST-setPositionRouter leaks approval to previous positionRouter. **Description:**
positionRouter is used to change GMX positions in GMXFuturesPoolHedger. It can be replaced 
by a new router if GMX redeploys, for example if a bug is found or the previous one is hacked. 
The new positionRouter receives approval from the contract. However, approval to the...

Vulnerable code:
function setPositionRouter(IPositionRouter _positionRouter) external onlyOwner {
      positionRouter = _positionRouter;
        router.approvePlugin(address(positionRouter));
    emit PositionRouterSet(_positionRouter);
       }
LOW severity: TRST-GMXAdapter minReturnPercent can be over 100%, requiring positive slippage for exchange to execute. **Description:**
In GMXAdapter `exchangeFromExactBase()`, minReturnPercent is used to set the minimum 
output tokens for the trade:

Since the rest of the calculation is done precisely the same as in positionRouter, except 
imposed fees, if minReturnPercent is above 100% transactions will never...

Vulnerable code:
uint minOut = tokenInPrice.multiplyDecimal(marketPricingParams[_optionMarket].minReturnPercent)
      .multiplyDecimal(_amountBase)
       .divideDecimal(tokenOutPrice);
LOW severity: TRST-LiquidityToken owner can freeze all token interactions and suspend the platform. **Description:** 
Owner can freely set the **liquidityTracker** variable in LiquidityToken. It is used in 
`_afterTokenTransfer()`:

If owner is compromised, the platform can be immediately shut down by passing an invalid 
address as **liquidityTracker**.

 ### TRST-L-7 Owner can manipulate...

Vulnerable code:
function _afterTokenTransfer(address from, address to, uint amount) internal override {
      if (address(liquidityTracker) != address(0)) {
        if (from != address(0)) {
    liquidityTracker.removeTokens(from, amount);
    }
    if (to != address(0)) {
        liquidityTracker.addTokens(to, amount);
           }
         }
       }