Farseen0/scar-pairs huggingface dataset:
LOW severity: Calling `contains` before `insert`. **Severity**: Low 

**Status**: Acknowledged

**File**: token-locker/src/lib.rs

**Function**: Contract::record_proof

**Details**:

The `UnorderedSet::insert` function returns false if the element already exists in the set. While the `contains` call requires less gas than `insert` itself it is...

Vulnerable code:
assert!(
self.used_events.insert(&proof_key),
"Event cannot be reused for withdrawing."
);
HIGH severity: Methods returning price in the wrong units. **Severity**: High

**Status**: Resolved

**Description**

In TradingVaultV2.sol - Method getPricePerFullShare() starts by assuming 18 decimal places for price (i.e. at totalSupply() = 0) . Once totalSupply() takes non-zero value the method returns the decimals of USDT (i.e. which is 6 on...

Vulnerable code:
if (totalSupply() == 0) {
    shares = _amount;
} else {
    shares = (_amount.mul(totalSupply())).div(currentBalanceUSDT);
}

if (totalSupply() == 0) {
    shares =  1e6  // _amount refers to USDC decimals
} else {
    shares =  1e18    // 1e6 * 1e18 / 1e6 = TradingVaultV2 decimals
}

function decimals() public view virtual override returns (uint8) {
        return 18;
    }
MEDIUM severity: Insufficient check on reward token balance. **Severity**: Medium

**Status**: Resolved

**Description**

In contracts NarwhalPool.sol and TradingVaultV2, at line 127 inside the setRewardDuration function check if balance of esToken is enough to distribute the newly added rewards for their duration. Current check cannot be sufficient to make.....

Vulnerable code:
it("Insufficient check on reward token balance ", async () => { 
      await vault.setRewardToken(es.address, { from: deployer });
  
      const sevenDaysRewardsDuration = BN("604800"); //7 days in seconds
      vault.setRewardsDuration(sevenDaysRewardsDuration, { from: deployer });


      //Governance sends 100 esTokens 
      await es.transfer(vault.address, new BigNumber(100e18));


      //Governance calls notifyRewardAmount
      const reward = new BigNumber(100e18);
      await vault.notifyRewardAmount(reward, { from: deployer });


      //User stakes 100 usdt
      await vault.deposit(new BigNumber(100e18), user2, { from: user2, gas: 5000000, gasPrice: 500000000 }); 
      
      //7 days later
      await time.increase(time.duration.seconds(604800));
      let bal = await vault.earned(user2, { from: user2 })
      console.log("first 7 days: ", bal.toString())


      //Governance calls notifyRewardAmount
      await vault.notifyRewardAmount(reward, { from: deployer });


      //7 days later
      await time.increase(time.duration.seconds(604800));
      let bal2 = await vault.earned(user2, { from: user2 })
      console.log("second 7 days: ", bal2.toString())
      
      expectRevert(
        vault.harvest(user2, { from: user2 }), "BaseToken: transfer amount exceeds balance"
        );
    });
LOW severity: Redundant if branching in deposit. **Severity**: Low

**Status**: Resolved

**Description**

In TradingVaultV2.sol - Method deposit(uint,address): Unnecessary if branching

Since user already takes the value of msg.sender in the else branch and that is given by the if-statement  before it (i.e. which checks the same...

Vulnerable code:
if (allowed[msg.sender]) {
    console.log("Allowed");
    IERC20(USDT).safeTransferFrom(msg.sender, address(this), _amount);
    harvest(user);
} else {
    console.log("Not allowed");
    IERC20(USDT).safeTransferFrom(user, address(this), _amount);
    harvest(msg.sender);
}

require(storageT.USDT().transferFrom(msg.sender, address(this), _amount));
harvest(user);
LOW severity: Methods `firstEmptyTradeIndex(...)` and `firstEmptyOpenLimitIndex` will always return 0 if no empty index available. **Severity**: Low

**Status**: Resolved

**Description**

In TradingStorage contract, methods firstEmptyTradeIndex(...) and firstEmptyOpenLimitIndex(...) uses for loop for finding the next empty index. 
These methods will always return 0 if the loop does not break and no empty index is found....

Vulnerable code:
require(
           storageT.openTradesCount(t.trader, t.pairIndex) +
               storageT.pendingMarketOpenCount(t.trader, t.pairIndex) +
               storageT.openLimitOrdersCount(t.trader, t.pairIndex) <
               storageT.maxTradesPerPair(),
           "MAX_TRADES_PER_PAIR"
       );
LOW severity: Missing Check for Price returned from tokenPriceUSDT. **Severity** : Low

**Status**: Resolved

**Descritption** :

The tokenPriceUSDT function should have a check in place to ensure that the price returned from the queryPriceFeed function is not zero, as this could lead to a vulnerability. Without this check, users could potentially exploit the...

Vulnerable code:
require(priceFeed.price.price != 0, “PRICE_FEED_ERROR”); // add check for non-zero price
MEDIUM severity: Logic edge case if a chain re-organisation happens. **Severity**: Medium

**Status**: Unresolved

**Description**

In contract PaymentProcessor.sol, the flow is firstly checking if the enforcePricingConstrains is enable and if it is enable is checking if the price is set between floor price and ceiling price, if it is not enabled then the execution.....

Vulnerable code:
Scenario #1 ( normal flow ):
1. Bob setup his collection, creating a security policy where he enables the price constrain enforce  in tx #1 block #1
2. Bob setup his pricing bound by calling the function “setTokenPricingBoundaries” in tx#2 block #1
3. Alice, Carol & David execute different nft  transactions using PaymentProcessor over Bob collection, creating tx #3, #4 & #5 that will also get indexed in block #1.

1. Bob setup his collection, creating a security policy where he enables   the price constrain enforce  in tx #1 block #1
2. Alice, Carol & David execute different nft  transactions using PaymentProcessor over Bob collection, creating tx #2, #3 & #4 that will also get indexed in block #1.
3. Bob setup his pricing bound by calling the function “setTokenPricingBoundaries” in tx#5 block #1.
HIGH severity: Execution flow in _onlyWhiteListed modifier leaves modified function unexecuted. **Severity**: High

**Status**: Resolved

**Description**

The MasterPenPIe._onlyWhiteListed() modifier checks whether the sender is part of the AllocationManagers, PoolManagers, or is the contract owner. If any of these conditions are satisfied, the modifier executes a return statement and...

Vulnerable code:
it("Invalid modifier execution flow", async () => {
           let stakingToken1: StandardTokenMock;
           stakingToken1 = await setup.newToken(20, 50)
           let allocPoint1 = ether(200)


           let totalAllocBefore = await masterPenPie.totalAllocPoint();


           await deposit(player1);
           await masterPenPie.updatePoolsAlloc([stakingToken1.address],[allocPoint1]);


           let totalAllocAfter = await masterPenPie.totalAllocPoint()
          
           expect((await masterPenPie.tokenToPoolInfo(stakingToken1.address)).allocPoint).to.eq(0)
           expect(totalAllocBefore).to.eq(totalAllocAfter);
       });

modifier _onlyWhiteListed() {
       require(
           AllocationManagers[msg.sender] ||
           PoolManagers[msg.sender] ||
           msg.sender == owner(),
           "OnlyWhiteListedAllocaUpdator"
       );
       _;
}
MEDIUM severity: Misuse of ecrecover. **Severity**: Medium

**Status**: Acknowledged

**Description**

VotingEscrow.sol - Vulnerability exists in the signature logic of delegateBySig() function. According to the following:

The contract retrieves the address of the signer from the signature via ecrecover() then the retrieved address...

Vulnerable code:
address signatory = ecrecover(digest, v, r, s);
require(signatory != address(0), "VotingEscrow::delegateBySig: invalid signature");
MEDIUM severity: Missing VERSION in DOMAIN_TYPEHASH. **Severity**: Medium

**Status**: Acknowledged

**Description**

In contract VotingEscrow, the DOMAIN_TYPEHASH  is build as following

While the EIP712 states it has one more parameter  `string version` which is missing in this contract.
In the same contract, the delegateBySig method id uses...

Vulnerable code:
bytes32 public constant DOMAIN_TYPEHASH =
       keccak256(
           "EIP712Domain(string name,uint256 chainId,address verifyingContract)"
       );

bytes32 public constant DOMAIN_TYPEHASH = keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)");
HIGH severity: Undeclared identifier. **Severity**: Critical

**Status**: Resolved

**Description**

Variable `_decimals` is not defined but is set in the constructor method

**Recommendation**: 

declare the variable as `uint8 immutable private _decimals` in the contract and override the `decimals`

function to return it's value:

Vulnerable code:
function decimals() public view virtual override returns (uint8) {
        return _decimals;
    }
MEDIUM severity: Unsafe Uint64 Casting in PendleVoteManager. **Severity**: Medium

**Status**: Resolved

**Description**

 In _getVoteInPercentage() function:
 
_getExactPercentage() function returns uint256 and it is being casted to uint64 without validating that the input number is less than or equal to type(uint64).max. The input number needs to be as...

Vulnerable code:
169  else {
170    votePerc = exactVoteCount * 100 / totalVotes();
171    pendleVote = uint64(_getExactPercentage(votePerc));
172  }

And
161     exactVoteCount = uint256(_vote) + _currentVote;
162     votePerc = exactVoteCount * 100 / totalVotes();
163     pendleVote = uint64(_getExactPercentage(votePerc));
LOW severity: Discarded Votes due to unsuitable datatype. **Severity**: Low

**Status**:  Acknowledged

**Description**

In _updateVoteAndCheck() function of the PendleVoteManagerBaseUpg contract, the weights of votes add up as follows:

This is given weight < 0 , while we have pool.totalVoteInVlPenPie of type uint256 and userVotedForPoolInVlPenPie maps...

Vulnerable code:
263  uint256 absVal = uint256(-weight);
264  pool.totalVoteInVlPenPie -= absVal;
265  userVotedForPoolInVlPenPie[_user][pool.market] -= absVal;
LOW severity: Returned UserPoolData from external call is not validated. **Severity**: Low

**Status**:  Acknowledged

**Description**

PendleVoteManagerMainChain.sol - In getVoteForMarket() function, the userPoolData return is not validated to be actually existent. An invalid address of market might be entered which returns a zero value by default. The result of the...

Vulnerable code:
75  function getVoteForMarket(address market) public view returns (uint256) {
76      IPVoteController.UserPoolData memory userPoolData = voter
77          .getUserPoolVote(address(pendleStaking), market);
78      uint256 poolVote = userPoolData.weight * totalVotes() / 1e18 ;
79      return poolVote;
80  }
LOW severity: Misleading Event Emitted From a State Changing Action. **Severity**: Low

**Status**: Resolved 

**Description**

StakingPoolConfigurator.sol -

This mixes things up with
 function setFee(uint256 fee) external 

as they emit the same event.

**Recommendation** 

setLiquidationFactor requires a new event created for it to be emitted on calling the...

Vulnerable code:
function setLiquidationFactor(uint256 liquidationFactor) external onlyPoolAdmin {
    DataTypes.PoolConfigurationMap memory currentConfig = pool.getReserveData().configuration;
    currentConfig.setLiquidationFactor(liquidationFactor);
    pool.setPoolConfiguration(currentConfig.data);

    emit PoolFeeChanged(liquidationFactor);
  }
MEDIUM severity: Users Might Receive Undesired Tokens In fullfilledRequestSwap. **Severity** : Medium

**Status** : Resolved 

**Description**

Say a user closed his position (closePosition() in SakeVaultV2.sol) and does not want to withdraw in USDC. 
`closePositionAmount[_user][_positionID] = amountAfterFee;` and `closePositionRequest[_user][_positionID] = true;` would be...

Vulnerable code:
UserInfo storage _userInfo = userInfo[_user][_positionID];
require(msg.sender == _userInfo.user)
HIGH severity: Missing `onlyGelatoRelay` modifier. **Severity**: High

**Status**: Acknowledged

**Description**

In Contract `TradableSideVault`, there are several methods that are “ForUser” meaning those methods will be called on behalf of a user and using _msgSender() for getting the address for the user.

Although these methods are using...

Vulnerable code:
function stakingAccountDepositForUser(
address token,
uint256 amount
) external {
 
function stakeFromMarginAccountForUser(
address token,
uint256 amount
) external {

function withdrawalRequestForUser(
WithdrawalType withdrawalType,
address token,
uint256 amount
) external {
HIGH severity: Methods related to Staking on SideVault Contract encode incorrect staking/withdrawal methods for Base chain. **Severity**: High

**Status**: Resolved

**Description**

In Contract TradableSideVault, method stakingAccountDepositForUser(...) and method stakingAccountDeposit encodes the keyword, destination base chain contract address, and the destination method along with the parameters. The destination...

Vulnerable code:
// message being sent
bytes memory payload = abi.encode(
"md",
dstBaseStakingVault,
abi.encodeWithSignature(
"marginAccountDeposit(address,(address,uint8,bool),uint256)",
msg.sender,
getAcceptedTokenInfo(token),
amount
)
);

// creates messages using user data and send to base chain
address destinationVault = withdrawalType == WithdrawalType.Stake
? dstBaseStakingVault
: dstBaseMarginVault;
bytes memory payload = abi.encode(
"mw",
destinationVault,
abi.encodeWithSignature(
"marginAccountWithdrawal(address,(address,uint8,bool),uint256,address)", 
msg.sender,
getAcceptedTokenInfo(token),
amount,
address(this)
)
);
LOW severity: Admin user can `unstake` without fulfilling the staking period. **Severity**: Low

**Status**: Resolved

**Description**

In contract TradableStaking, the method _unstake(...) implemented the following logic

Here it means that the admin user can unstake without fulfilling the staking period.

**Recommendation**: 

Use a multi-sig account for the admin user.

Vulnerable code:
if (user != ITradableSettings(settingsProvider).getAdminUser()) {
uint256 timeDiff = block.timestamp - (uint256(_stakes.timestamp));
require(
timeDiff > uint256(stakingSettings.stakingPeriod),
"period"
);
// console.log("timeDiff: %d", timeDiff);
}
MEDIUM severity: APR Unintentionally Set to High Value. **Severity**: Medium

**Status**: Resolved

**Description**

In InterestRateModel.sol It is noted from `setAPR()` that the value of APR should be less than 8e5. Despite that the initial value of APR is being set to 42e5. It is pointed out in the comments that it is meant to be 4.2% which is...

Vulnerable code:
// Assuming the maximum is 4.2%
uint256 private APR = 42 * 1e5;

function setAPR(uint256 newAPR) external onlyRole(DEFAULT_ADMIN_ROLE) {
	require(newAPR <= 8 * 1e5, "apr should be less then 8%");
	...
}
MEDIUM severity: Mixed up shares/rUSTP conversion. **Severity**: Medium

**Stratus**: Resolved

**Description**

rUSTP.sol - Function _burnShares() in lines: 272, 276 _sharesAmount is supposed to be converted to rUSTP but the method getSharesByrUSTPAmount is used which is expected from its formula to convert rUSTP amount to shares...

Vulnerable code:
272     uint256 preRebaseTokenAmount = getSharesByrUSTPAmount(_sharesAmount);
273     newTotalShares = _getTotalShares().sub(_sharesAmount);
274     totalShares = newTotalShares;
275     shares[_account] = accountShares.sub(_sharesAmount);
276     uint256 postRebaseTokenAmount = getSharesByrUSTPAmount(_sharesAmount);
MEDIUM severity: Possibility of Slippage Attacks. **Severity**: Medium 

**Status** : Acknowledged 

**Description**: 

No slippage has been set during swapping of tokens in the function `swapTokensForEth()`. This could lead to the user being vulnerable to slippage attacks in which an attacker can manipulate the prices and frontrun and backrun...

Vulnerable code:
uniswapV2Router.swapExactTokensForETHSupportingFeeOnTransferTokens(
            tokenAmount,
            0, // accept any amount of ETH
            path,
            address(this),
            block.timestamp
        );
LOW severity: Missing Zero Address Check. **Severity**: Low

**Status**: Resolved

In the contact GenericERC20Token.sol, the following functions are missing zero address checks that could cause undesired effects:

**Recommendation**:

Add a custom error or require statement to ensure the zero address cannot be passed.

Comment: The client.....

Vulnerable code:
setRouter() 
setTaxReceiver() 
setExcludedFromLimits() 
setExcludedFromFees() 
setLP()
LOW severity: The stake method allows staking 0 tokens. **Severity**: Low

**Status**: Resolved

**Description**

In contract cloudX_staker.sol, the method stake(...) allows users to stake 0 tokens because of the following logic:

If lastStakedValue == amount then currentValue will be 0. 

**Recommendation**: 

Update the require statement as following

Vulnerable code:
require(
lastStakedValue <= _amount,
"amount should be greater than previous stake "
);
uint256 currentValue = _amount - lastStakedValue; // @audit 0 can be allowed
lastStakedValue = _amount;

require(
lastStakedValue < _amount,
"amount should be greater than previous stake "
);
HIGH severity: Unrestricted position fulfillment due to lack of initiator validation. **Severity**: Critical

**Status**:  Resolved

**Description**

The VodkaVaultV2 contract holds functions designed to initiate specific operations on the GMX system, essentially starting the whole relevant process. For instance, the user can call the requestOpenPosition function specifying the...

Vulnerable code:
IExchangeRouter.CreateDepositParams memory params = IExchangeRouter.CreateDepositParams({
           receiver: address(this),
           callbackContract: strategyAddresses.VodkaHandler,
           uiFeeReceiver: msg.sender,
           market: gmp.marketToken,
           initialLongToken: gmp.longToken,
           initialShortToken: gmp.shortToken,
           longTokenSwapPath: new address[](0),
           shortTokenSwapPath: new address[](0),
           minMarketTokens: 0, 
           shouldUnwrapNativeToken: false,
           executionFee: gmxOpenCloseFees,
           callbackGasLimit: 2000000
       });


       bytes32 key = IExchangeRouter(gmxAddresses.exchangeRouter).createDeposit(params);

function createDeposit(
       DepositUtils.CreateDepositParams calldata params
   ) external payable nonReentrant returns (bytes32) {
       address account = msg.sender;


       return depositHandler.createDeposit(
           account,
           params
       );
   }

Deposit.Props memory deposit = Deposit.Props(
           Deposit.Addresses(
               account,
               params.receiver,
               params.callbackContract,
               params.uiFeeReceiver,
               market.marketToken,
               params.initialLongToken,
               params.initialShortToken,
               params.longTokenSwapPath,
               params.shortTokenSwapPath
           ),
HIGH severity: Loss of funds by griefing debt repayment. **Severity**: High

**Status**:  Resolved

**Description**

When a deposit request is denied on the GMX side (privileged account triggers cancelDeposit from DepositHandler contract), the afterDepositCancellation callback from VodkaV2GMXHandler is invoked as part of the routine. Within this...

Vulnerable code:
IWater(strategyAddresses.WaterContract).repayDebt(            dr.leverageAmount,
           dr.leverageAmount
       );


       payable(dr.user).transfer(eventData.uintItems.items[0].value);
MEDIUM severity: User is granted less amount than they should. **Severity**: Medium

**Status**: ِ Acknowledged

**Description**

VodkaVaultV2.sol - In function fulffillClosePosition(),extraData.returnedValue is being compared to wr.fullDebtValue + waterProfits, after that a bigger amount wr.fullDebtValue + extraData.profits is subtracted from...

Vulnerable code:
829 if (extraData.returnedValue < (wr.fullDebtValue + waterProfits)) {
830     _positionInfo.liquidator = msg.sender;
831     _positionInfo.liquidated = true;
832     waterRepayment = extraData.returnedValue;
833 } else {
834     extraData.toLeverageUser = (extraData.returnedValue - wr.fullDebtValue - extraData.profits) + leverageUserProfits;
835     waterRepayment = extraData.returnedValue - extraData.toLeverageUser - waterProfits;
MEDIUM severity: Flags can be passed by in case of `requestClosePosition`. **Severity**: Medium

**Status**: Resolved

**Description**

VodkaVaultV2.sol - Function requestClosePosition is called with the purpose to close an open position. After functionfulfillClosePosition is invoked, position is either liquidated or closed. Function requestClosePosition should not pass...

Vulnerable code:
function requestClosePosition(
        uint256 _positionID,
        address _user
    ) external payable InvalidID(_positionID, _user) nonReentrant {
        PositionInfo storage _positionInfo = positionInfo[_user][_positionID];
718     require(
            !_positionInfo.liquidated || !_positionInfo.closed,
            "VodkaV2: position is closed or liquidated"
        );
LOW severity: Upgradeability is malfunctioning due to false gap placement. **Severity**: Low

**Status**: Acknowledged

**Description**

VodkaV2GMXHandler.sol - In that contract __gaps is not placed as the last variable of the declared variables which leads to having the storage mixed up with the upgraded versions of the contract.

**Recommendation** 

Place _gaps at the.....

Vulnerable code:
uint256[50] private __gaps;
    address public WETH;
    uint24 public univ3Fee;
    address public RoleStore;
LOW severity: No slippage protection on token swap. **Severity**: Low

**Status**: Acknowledged

**Description**

VodkaVaultV2.sol - In body of function requestClosePosition() there is a procedure that aims at closing an opened position while the amount of tokens to be received out via gmx is not protected against sudden price change. This is...

Vulnerable code:
760     IExchangeRouter.CreateWithdrawalParams memory params = IExchangeRouter
            .CreateWithdrawalParams({
                receiver: strategyAddresses.VodkaHandler,
                callbackContract: strategyAddresses.VodkaHandler,
                uiFeeReceiver: _user,
                market: gmp.marketToken,
                longTokenSwapPath: new address[](0),
                shortTokenSwapPath: new address[](0),
                minLongTokenAmount: 0,
                minShortTokenAmount: 0,
                shouldUnwrapNativeToken: false,
                executionFee: gmxOpenCloseFees,
                callbackGasLimit: 2000000
            });

uint256 amountOut;
        if (wr.longToken != WETH) {
            ISwapRouter.ExactInputParams memory params = ISwapRouter
            .ExactInputParams({
                path: abi.encodePacked(wr.longToken, univ3Fee, WETH, univ3Fee, strategyAddresses.USDC),
                recipient: address(this),
                deadline: block.timestamp,
                amountIn: longAmountFromGMX,
                amountOutMinimum: 0
            });

            amountOut = ISwapRouter(strategyAddresses.univ3Router).exactInput(params);
        } else {
            ISwapRouter.ExactInputSingleParams memory params = ISwapRouter
            .ExactInputSingleParams({
                tokenIn: wr.longToken,
                tokenOut: strategyAddresses.USDC,
                fee: univ3Fee,
                recipient: address(this),
                deadline: block.timestamp,
                amountIn: longAmountFromGMX,
                amountOutMinimum: 0,
                sqrtPriceLimitX96: 0
            });

            amountOut = ISwapRouter(strategyAddresses.univ3Router).exactInputSingle(params);
        }
LOW severity: Mapping inCloseProcess not updated. **Severity**: Low

**Status**: Acknowledged

**Description**

VodkaVaultV2.sol - In function fulfillClosePosition, we have mapping inCloseProcess that allows or denies execution of the function. As fulfillClosePosition for a given key is intended to be executed successfully once. It is expected...

Vulnerable code:
require(
    inCloseProcess[wr.user][wr.positionID],
    "VodkaV2: close position request not ongoing"
);