Farseen0/scar-pairs huggingface dataset:
HIGH severity: `getGlobalPnL` is wrongly accrued resulting on a critical computation.. **Severity**: Critical	

**Status**: Resolved

**Description**

The function `getGlobalPnL()` within the `VaultFacet.sol` smart contract is used to calculate the profit and losses of the whole protocol:

However, Profit short is wrongly accrued as the subtraction should be the other way...

Vulnerable code:
function getGlobalPnL(address _address, uint256 price) public view returns (int256) {
       int256 ProfitLong =
           ((int256(price) - int256(s.averagePriceLong[_address])) * (int256(s.reservedSizeLong[_address]))) / 1e6; 
       int256 ProfitShort =
           (int256(price) - int256(s.averagePriceShort[_address])) * (int256(s.reservedSizeShort[_address])) / 1e6;
       return ProfitLong + ProfitShort;
   }

int256 ProfitShort =
            ( int256(s.averagePriceShort[_address])) * (int256(s.reservedSizeShort[_address]) - int256(price)) / 1e6;
HIGH severity: Fees are wrongly accrued when reducing collateral.. **Severity**: Critical	

**Status**: Resolved

**Description**

The function `_reduceCollateral()` within the `TradeFacet.sol` smart contract is called when a user decreases a position, this function gets the `fundingFee` and if it needs to be paid:

The function differentiates several cases,...

Vulnerable code:
(uint256 fundingfee, bool tobePaid) = IFormula(s.formula).getFundingFee(
           position._indexToken, position.size, position.entryFundingRate, position.isLong
       );

       if (tobePaid) {
           fee = int256(borrowingInterest + fundingfee);
       } else {
           if (borrowingInterest > fundingfee) {
               fee = int256(borrowingInterest - fundingfee);
           } else {
               fee = toNegativeInt256(fundingfee + borrowingInterest);
           }
       }
HIGH severity: Referrer is not valid for earning for more time than expected.. **Severity**: Critical	

**Status**: Resolved

**Description**

The `isValidForEarning()` function in `Referrals.sol` is used for checking if a referrer is able to earn funds or not. This restriction is checked considering that a referrer is available for earning only if `maxDaysForEarning` have...

Vulnerable code:
if (regis.hasRegistered && regis.registrationTime + (maxDaysForEarning * 1 days * 86_400) >= block.timestamp) { 
           return true;
       }
HIGH severity: Collateral is reduced twice when a position is liquidated by a user different from `protocolLiquidator`.. **Severity**: Critical 	

**Status**: Resolved

**Description**

The function `liquidatePosition()` within the `Escrow.sol` smart contract is used to liquidate a position. This function differentiates if the caller is `protocolLiquidator` or any other user. However, there is an `else` statement...

Vulnerable code:
if (msg.sender != protocolLiquidator) {
           require(actualCollateralRequired <= _collateralAmount, "less collateral");
           uint256 positionSize = position.size;
           require(positionSize >= _collateralAmount, "position size should be greater than collateral");
           uint256 userleverage = positionSize / _collateralAmount;
           require(userleverage <= 20, "exceeds 20X leverage");
           position.collateral = positionSize / userleverage;

           IERC20(usdc).transferFrom(msg.sender, address(this), _collateralAmount);  
           IERC20(usdc).approve(diamond, _collateralAmount);

           ITrade(diamond).updateCollateralFromLiquidation(
               _decreasedCollateralValue, _collateralAmount, position, newKey
           );
       } 

       ITrade(diamond).updateCollateralFromLiquidation(_decreasedCollateralValue, 0, position, newKey);

function updateCollateralFromLiquidation(
       uint256 _decreaseCollateralValue,
       uint256 _collateralAmount,
       Position memory _position,
       bytes32 _key
   ) external onlyEscrow {
       if (_position.isLong) {
           s.longCollateral[_position._indexToken] -= _decreaseCollateralValue;
           s.longCollateral[_position._indexToken] += _collateralAmount;
       } else {
           s.shortCollateral[_position._indexToken] -= _decreaseCollateralValue;
           s.shortCollateral[_position._indexToken] += _collateralAmount;
       }
       s.positions[_key] = _position;
   }
HIGH severity: Users are paying an incorrect amount of fees when decreasing a position leading to loss of funds.. **Severity**: High	

**Status**: Resolved

**Description**

When a user decreases one of its positions by calling `decreasePosition()` within the `TradeFacet.sol` smart contract, `tradeFees` are calculated and decreased from position.collateral, these feeds vary depending if the traderType is...

Vulnerable code:
if (_decPosParams.traderType == TraderType.Maker) {
           (, uint256 makerincreaseFees,,,) = s.keeper.orderFeesByCompartment(position._indexToken);
           uint256 tradeFees = (makerincreaseFees * _decPosParams.indexDelta) / 10_000;
           s.keeper.distributeTradeFees(
               tradeFees, position._indexToken, _decPosParams.account, _decPosParams.traderType
           );
       } else if (_decPosParams.traderType == TraderType.Taker) {
           (,,, uint256 takerIncreaseFees,) = s.keeper.orderFeesByCompartment(position._indexToken);
           uint256 tradeFees = (takerIncreaseFees * _decPosParams.indexDelta) / 10_000;
           IERC20(s.USDC).approve(address(s.keeper), tradeFees);
           s.keeper.distributeTradeFees(
               tradeFees, position._indexToken, _decPosParams.account, _decPosParams.traderType
           );
           position.collateral -= tradeFees;
       }
HIGH severity: Interest Could Be Calculated As 0 Due To Rounding. **Severity** - High

**Status** - Resolved

**Description**

In the updateBalances function the formula to calculate interest (short or long) is as follows →

Consider the following scenario ->

Borrow rate for the index token = 3e3

Borrow amount from pool = 1e10 (10k USD)

Since the contracts...

Vulnerable code:
uint256 shortInterest = (borrowingRate * interval * s.borrowedAmountFromPool[_indexToken].short)
            / (INTERESTRATE_DECIMALS * 365 * 86_400);
HIGH severity: Returning position.collateral When pnlWithFees > 0 Might Be Problematic. **Severity** - High

**Status** - Invalid

**Description**

(Please double check this finding )

_validateLiquidation is a function which is used to check if a position is liquidatable or not , to check this it calculates the current collateral using the CurrentCollateral function , in the current.....

Vulnerable code:
int256 pnlWithFees = pnl - int256(collectFees(position)) - int256(_liquidationFee) + fundingFee;
        if (pnlWithFees > 0) {
            return (position.collateral);        }
HIGH severity: addNewAsset Does Not Mark The Previous Assets As False. **Severity** - High

**Status** - Resolved

**Description**

When adding new assets in the vault facet all the index tokens are removed first (the old ones) and new ones are added (deletion of allIndexTokens) and then new tokens are pushed in the allIndexTokens.

But , s.indexTokens[] is not being.....

Vulnerable code:
delete s.allIndexTokens;
        uint256 length = _address.length;
        for (uint256 i = 0; i < length; ++i) {
            s.allIndexTokens.push(_address[i]);
            s.IndexToken[_address[i]] = true;
            assignCompartment(_address[i], _percentage[i]);
            emit NewAssetAdded(_address[i], _percentage[i]);
        }
HIGH severity: When updateCompartmentsReduceLiq Is Called It Is Possible The Compartment Becomes Invalid. **Severity** - High

**Status** - Acknowledged

**Description**

When liquidity is being removed (removeLiquidity() in trade facet) , every compartment’s balance is adjusted depending on the assigned percentage in the updateCompartmentsReduceLiq  function . When liquidity is removed from a...

Vulnerable code:
for (uint256 i = 0; i < length; ++i) {
            address asset = s.allIndexTokens[i];
            Compartment memory compartment = s.compartments[asset];
            uint256 liquidityPerAsset = (amount * (compartment.assignedPercentage)) / (BASIS_POINTS_DIVISOR);
            compartment.balance -= liquidityPerAsset;
HIGH severity: updateBalances Should Be Called  Before collectFees(). **Severity** - High

**Status** - Resolved

**Description**

When increasePosition is invoked in the trade facet , the borrow interest is accumulated using the collectFees() function

uint256 fee = collectFees(position);
The collectFees() function calculates the interest which is dependent on the...

Vulnerable code:
function collectFees(Position memory _position) internal returns (uint256) {
        uint256 interestFactor = s.cumulativeInterestRate[_position._indexToken] - _position.entryCumulativeInterestRate;
        uint256 interest = (interestFactor * _position.reserveAmount) / (INTERESTRATE_DECIMALS * 365 * 86_400);
        emit BalanceOutDone(interest);
        return interest;
    }
HIGH severity: Anyone can set a malicious `diamond` address.. **Severity**: High	

**Status**: Resolved

**Description**

The `Referrals.sol` smart contract implements an `addDiamond` function which set a specific address to the `diamond` which is used by the `onlyDiamond` modifier. The function `addDiamond` does not implement any access restrictions so...

Vulnerable code:
function addDiamond(address _address) external { 
       diamond = _address;
 }
MEDIUM severity: Fees keep accumulating when the protocol is paused.. **Severity**: Medium	

**Status**: Acknowledged

**Description**

The function `updateBalances()` within the `TradeFacet.sol` smart contract is used to update balances and distribute borrowing fees. This function is public and has no access restrictions or any other modifier so that it can be...

Vulnerable code:
if (s.lastBalanceUpdateTime[_indexToken] == 0) {
           s.lastBalanceUpdateTime[_indexToken] = block.timestamp;
       }
 uint256 interval = (block.timestamp - s.lastBalanceUpdateTime[_indexToken]);

s.longCollateral[_indexToken] -= longInterest;
       s.shortCollateral[_indexToken] -= shortInterest;
       uint256 totalInterest = shortInterest + longInterest;

       IERC20(s.USDC).approve(address(s.keeper), totalInterest);
       s.keeper.distributeBorrowingFees(address(this), totalInterest, _indexToken);
MEDIUM severity: >= Instead Of >. **Severity** - Medium

**Status** - Resolved

**Description**

Auto deleveraging is a feature where if the compartment’s Pnl is higher than the ADL ratio then the balance is auto deleveraged  , ADLValue is calculated as follows →

According to the AppStorage definition of ADLPercentage , it is the.....

Vulnerable code:
uint256 ADLValue = (s.ADLPercentage[_address] * (compartment.balance)) / BASIS_POINTS_DIVISOR;

if (uint256(globalPnL) > ADLValue) { 
            return true;
        }

if (uint256(globalPnL) >= ADLValue) {
            return true;
MEDIUM severity: Multiple Functionalities Will Break If USDC Turns On Fee-On-Transfer. **Severity** - Medium

**Status** - Resolved

**Description**

In the deposit contract , when depositing we perform the following →

But if USDC (which is upgradeable) turns on its feature for fee-on-transfer , the amount which is transferred into the contract would be less than the amount in the...

Vulnerable code:
IERC20(usdc).safeTransferFrom(msg.sender, address(this), _amount);
        balances[msg.sender] += _amount;
        emit Deposited(msg.sender, _amount);
MEDIUM severity: orderType Does Not Account For Entire Order Struct. **Severity** - Medium

**Status** - Invalid

**Description**

In the router contract orders are processed and the orderType is defined as → 

And this orderType is then hashed to get the order hash (which is then validated) , the problem here is that the original order struct has many more...

Vulnerable code:
orderType =
            "struct Order { address indexToken; address sender; uint256 price; uint256 amount; uint256 collateral}";

struct Order {
        address indexToken;
        address sender;
        uint256 priceX18;
        uint256 amount; // ammount of indextoken
        uint256 collateral;
        bool reduceOnly;
        uint64 nonce;
        bool isLong;
        uint256 referalCode;
        TraderType traderType;
    }
MEDIUM severity: Transaction displacement can lead to the referrer not being able to earn.. **Severity**: Medium	

**Status**: Acknowledged

**Description**

The `registerReferral()` function in the Referrals.sol smart contract is used to assign a referral code to a new trader, or in other words, register a referred user by a referrer. The registration involves several parameters of...

Vulnerable code:
Registration memory registration =
           Registration({ registrationTime: block.timestamp, referralCode: referralCode, hasRegistered: true });
MEDIUM severity: It is not possible to add only 1 asset to the vault.. **Severity**: Medium	

**Status**: Resolved

**Description**

The `addNewAsset()` function within the `VaultFacet.sol` smart contract is used to set compartment percentages to assets. The total sums of percentages should always hold to be 100%, this is checked by the function....

Vulnerable code:
function assignCompartment(address _asset, uint256 _percentage) internal {
       require(_percentage < BASIS_POINTS_DIVISOR, "invalid value");        Compartment storage compartment = s.compartments[_asset];
       compartment.assignedPercentage = _percentage;
       s.compartments[_asset] = compartment;
       emit CompartmentAssigned(_asset, _percentage);
   }
MEDIUM severity: Vault and diamond assigned to the same address. **Severity**: Medium	

**Status**: Resolved

**Description**

`vault` and `diamondAddress` within `LpToken.sol` are being assigned to the same address using the `setDiamondAddress` function. These addresses are used for important logic like modifiers.

**Recommendation**:

Define each address...

Vulnerable code:
function setDiamondAddress(address _diamondAddress) public onlyGov {
       require(_diamondAddress != address(0), "Invalid address");
       vault = _diamondAddress;
       diamondAddress = _diamondAddress;
       emit SetVault(_diamondAddress);
   }
MEDIUM severity: `collateralizationRatio` can be set higher than 100% which would result in no positions getting liquidated.. **Severity**: Medium	

**Status**: Resolved

**Description**

The `updateCollateralizationRatio()` within the `VaultFacet.sol` function is used by the owner to change the collateralizationRatio of an asset. It is possible to set the value higher than 100% by error because there are no...

Vulnerable code:
if (currentCollateral <= (s.CollateralizationRatio[_indexToken] * position.collateral) / BASIS_POINTS_DIVISOR) {
           return (true, currentCollateral);
       }
MEDIUM severity: `allowUSDCtoVault` increases allowance by X instead of setting it to X.. **Severity**: Medium	

**Status**: Resolved

**Description**

The `allowUSDCtoVault` function withing the `MMTrade.sol` smart contract is used for setting MMTrade to Trade’s allowance to an specific amount:

However, the function is not setting the allowance to X as expected but increasing it by...

Vulnerable code:
/// @notice Allows the vault to spend a specified amount of USDC on behalf of the trade contract
   /// @param _amount The amount of USDC to allow the trade contract to spend
   function allowUSDCtoVault(uint256 _amount) external onlyOwner {
       usdc.safeIncreaseAllowance(trade, _amount);
   }
MEDIUM severity: `reservedSizeShort` can not reach its maximum allowed value. **Severity**: Medium	

**Status**: Resolved

**Description**

The `_increaseGlobalShortSize` function within the `TradeFacet.sol` smart contract is used to increase the `reservedSizeShort` of a specific token. It should fulfill that the new value for `reservedSizeShort` if within the allowed...

Vulnerable code:
function _increaseGlobalShortSize(address _token, uint256 _amount) private {
       s.reservedSizeShort[_token] = s.reservedSizeShort[_token] + _amount;
       uint256 MaxSize = s.MaxShortSize[_token];

       if (MaxSize != 0) {
           require(MaxSize > s.reservedSizeShort[_token], "Vault: max shorts exceeded");        }
       emit GlobalShortSizeIncreased(_token, _amount);
   }
MEDIUM severity: `updateBalancesForAll()` and `updateBalances()` functions are callable when the contract is paused.. **Severity**: Medium	

**Status**: Resolved

**Description**

The `updateBalancesForAll()` function within the `TradeFacet.sol` smart contract should only be callable when the contract is not paused as stated in the NatSpec comments: ‘This function is only callable when the contract is not...

Vulnerable code:
/// @notice Updates the balances for all index tokens in the contract.
   /// @dev Iterates through all index tokens stored in `s.allIndexTokens` and calls `updateBalances` for each.
   ///      This function is only callable when the contract is not paused.
   function updateBalancesForAll() public {
       uint256 len = s.allIndexTokens.length;
       for (uint256 i = 0; i < len; ++i) {
           updateBalances(s.allIndexTokens[i]);
       }
   }
LOW severity: Transaction displacement lead to force users to stake more time than expected. **Severity**: Low	

**Status**: Resolved

**Description**

The functions `addLiquidity` and `reduceLiquidity` within the `MMTrade.sol` smart contract serve for adding and reducing liquidity to the vault, respectively. When a user executes `addLiquidity`, the `lastAdded` mapping is updated:

This...

Vulnerable code:
function addLiquidity(uint256 _amount) public nonReentrant whenNotPaused {
       require(_amount > minUSDC, "MMTrade: Amount less than minimum USDC");

       lastAdded[msg.sender] = block.timestamp; 
       usdc.safeTransferFrom(msg.sender, address(this), _amount);

       _mint(msg.sender, _amount);

       emit BotLiquidityAdded(msg.sender, _amount);
   }
LOW severity: Optimal utilization percentage can be set to more than 100%. **Severity**: Low	

**Status**: Resolved

**Description**

The `addOptimalUtilization()` function within the `VaultFacet.sol` smart contract is used to set an optimal utilization percentage for a specific token but there are not checks to ensure that the new value is within a threshold, less or...

Vulnerable code:
/**
    * @notice Adds optimal utilization percentage for a specific index token.
    * @dev Can only be called by the contract owner (governance).
    * @param _percentage The optimal utilization percentage.
    * @param _indexToken The address of the index token.
    */
   function addOptimalUtilization(uint256 _percentage, address _indexToken) external onlyOwner {
       s.optmialUtilization[_indexToken] = _percentage;
   }

require(_percentage < BASIS_POINTS_DIVISOR, "invalid value");
LOW severity: Position leverage is hardcoded when transferring position as a result of a liquidation.. **Severity**: Low	

**Status**: Resolved

**Description**

When a position becomes liquidable it is transferred to the `Escrow.sol` contract to get it later liquidated by the protocol liquidator bot or an external liquidator by calling `liquidatePosition()`. When the position is transferred, it is.....

Vulnerable code:
Position memory newPosition = Position({
           size: _getliqPositionSize,
           collateral: _getliqPositionSize / 20, 
           averagePrice: collateralAtliquidationPrice / leverage,
           entryFundingRate: position.entryFundingRate,
           entryCumulativeInterestRate: position.entryCumulativeInterestRate,
           reserveAmount: position.reserveAmount,
           viaOrder: position.viaOrder,
           realisedPnl: 0,
           isLong: position.isLong,
           lastIncreasedTime: position.lastIncreasedTime,
           _indexToken: indexToken
       });

function _checkPositionForLiquidation(
       address _account,
       address _indexToken,
       bool _islong,
       uint256 _currentCollateral
   ) internal view returns (uint256, uint256, uint256, uint256, uint256, uint256) {
       bytes32 key = getPositionKey(_account, address(0), _indexToken, _islong);
       Position memory position = getPosition(key);
       uint256 decreasedCollateralValue = position.collateral - _currentCollateral;
       uint256 _getliqPositionSize = getLiquidationPositionSize(_account, _indexToken, _islong);
       uint256 leverage = (position.size / position.collateral);
       uint256 collateralAtliquidationPrice = _getliqPositionSize / leverage;
       uint256 liquidatorCollateralNeeded = (_getliqPositionSize / 20);
       uint256 collateralAfterFeesDeduction = _currentCollateral;

       return (
           _getliqPositionSize,
           collateralAtliquidationPrice,
           leverage,
           decreasedCollateralValue,
           collateralAfterFeesDeduction,
           liquidatorCollateralNeeded
       );
   }
HIGH severity: Users Might Be Unable To Make Their Position Healthy. **Severity** - High

**Status** - Acknowledged

**Description**

When new index tokens are added (refer finding 14) the isIndexToken[] mapping is toggled to false for all the older index tokens (VaultFacet.sol) . Users who had a position open previously with tokenA(say) and that position is coming.....

Vulnerable code:
require(s.isIndexToken[_incPosParams.indexToken], "index token is not valid");
HIGH severity: Anyone can drain the treasury.. **Severity**: Critical	

**Status**: Resolved

**Description**

The `Treasury.sol` smart contract implements a `calculateSetupRate()` function which should ‘Calculates setup reward rate and distributes fee for setup creator` as stated in the NatSpec comment. However, this function can be called by.....

Vulnerable code:
function calculateSetupRate(
       uint256 lostTeamTotal,
       uint256 wonTeamTotal,
       address initiator
   ) external returns (uint256, uint256) {
       lostTeamTotal *= 10 ** IERC20Mint(approvedToken).decimals();
       wonTeamTotal *= 10 ** IERC20Mint(approvedToken).decimals();
       uint256 withdrawnFee = (lostTeamTotal * fee) / FEE_DENOMINATOR;
       collectedFee += withdrawnFee;
       uint256 lostTeamFee = (lostTeamTotal * setupInitiatorFee) /
           FEE_DENOMINATOR;
       uint256 wonTeamFee = (wonTeamTotal * setupInitiatorFee) /
           FEE_DENOMINATOR;
       SafeERC20.safeTransfer(
           IERC20(approvedToken),
           initiator,
           lostTeamFee + wonTeamFee
       );
       //collect dust
       uint256 rate = ((lostTeamTotal - withdrawnFee - lostTeamFee) *
           FEE_DENOMINATOR) / (wonTeamTotal - wonTeamFee);
       return (rate, lostTeamFee + wonTeamFee);
   }

SafeERC20.safeTransfer(
           IERC20(approvedToken),
           initiator,
           lostTeamFee + wonTeamFee
       );
HIGH severity: Users can lose funds due to `safeTransferFrom` after approval.. **Severity**: Critical	

**Status**: Resolved

**Description**

The `deposit()` function within the `Treasury.sol` smart contract implements a `safeTransferFrom()` call:

As can be seen, the `from` is specified within the function's parameters, and the function is public and callable by...

Vulnerable code:
function deposit(uint256 amount, address from) public {
       SafeERC20.safeTransferFrom(
           IERC20(approvedToken),
           from,
           address(this),
           amount * 10 ** IERC20Mint(approvedToken).decimals()
       );
   }