Farseen0/scar-pairs huggingface dataset:
MEDIUM severity: No Price Staleness Check. **Severity** - Medium

**Status** - Resolved

**Description**

The function `latestRoundData()` has been used inside ERC20PriceOracle_V2 (getChainlinkPrice) to fetch the price of an asset , but there are no price staleness checks.

**Recommendation**:

Introduce price staleness checks as follows →

Vulnerable code:
(uint80 roundID, int256 answer, , uint256 timestamp, uint80 answeredInRound) = tokenInfos[token].chainlinkOracle.latestRoundData();
require(answeredInRound >= roundID, "Stale price");
require(timestamp != 0,"Round not complete");
LOW severity: Typing error in the `RemoveContract_toStableToken()` function. **Severity**: Low

**Status**: Resolved

**Description**

In the MultiSig_V2 contract, the line: 145 in the RemoveContract_toStableToken() function is as follows:

The code “(_target);” has been repeated twice in the function as an error and is not necessary. This could render confusion and render.....

Vulnerable code:
IStalbeToken(StableToken).RemoveVaultManager(_target);(_target);
LOW severity: Missing zero value check for gap in `RiskManagerEscrow_V2`. **Severity**: Low

**Status**: Resolved

**Description**

There is no check in the `setPara()` function to check that the `_ratio` parameter assigned to the gap variable should not be zero. A zero value assigned to gap can lead to division by zero panic on line: 172. Once the gap amiable is...

Vulnerable code:
return riskManagerEscrowAmt[riskManager][escrowToken] * (10 ** deltaDecimal) * unitRatio / gap;
LOW severity: Unsafe Downcasting in `TokenStableV6_V2`. **Severity**: Low

**Status**: Resolved

**Description**

There is unsafe downcasting on line: 483 and 492. 

This can result in silent overflows or truncation of the number.

**Recommendation**: 

It is advised to use a safecast library such as that of Openzeppelin’s Safecast library.

Vulnerable code:
Line: 483             uint16 currentEpoch = uint16((block.number - epochStart) / epochTerm + 1);

Line: 492 	     return uint16((block.number - epochStart) / epochTerm + 1);
HIGH severity: The method `multiTransfer(...)` fails to transfer tokens to recipients. **Severity**: High

**Status**: Resolved

**Description**

In Contract DevvE.sol, the method multiTransfer(...) fails to transfer tokens to recipients due to the following line:

Here even if `msg.sender` has approved the DevvE contract to transfer tokens on behalf of the `msg.sender` using the...

Vulnerable code:
transferFrom(msg.sender, recipients[i], amounts[i]);

this.transferFrom(msg.sender, recipients[i], amounts[i]);
HIGH severity: The undercollateralized positions may avoid being liquidated. **Severity**: Critical

**Status**: Resolved

**Description**

The stopTrade function in the Trading contract is designed to be called for the liquidation of undercollateralized positions. This function verifies various conditions to ensure a trade can be legitimately closed. These include...

Vulnerable code:
require(ot.lastUpdateTime + minAcceptanceDelay <= block.timestamp,"wait");
MEDIUM severity: Pairs `feeIndex` and `groupIndex` can not be updated 0. **Severity**: Medium

**Status**: Resolved

In Contract PairInfos.sol, the method updatePair(...) has the following check:

Here if owner wants to set feeIndex and/or groupIndex to 0, it will not be set without reverting leading to owner believing values are set properly.

**Recommendation**:...

Vulnerable code:
if (_pair.feeIndex > 0) {
           p.feeIndex = _pair.feeIndex;
       }


       if (_pair.groupIndex > 0) {
           p.groupIndex = _pair.groupIndex;
       }
HIGH severity: Insufficent ERtransfer validation. **Severity**: Medium

**Status**: Unresolved

**location**: multiTransfer

**Description**

Function `[multiTransfer]` requires the msg.sender to possess the total amount of the asset being transferred as shown in the following:

This validation though does not verify that msg.sender actually...

Vulnerable code:
require(token.balanceOf(msg.sender) >= totalAmount, "Insufficient balance");
HIGH severity: Certain ERtokens do not return bool from approve which breaks the core logic. **Severity**: High

**Status**:  Resolved

**Description**

The BrightPoolLenger and UniswapExchange contracts attempt to approve the ask or bid token for other contracts and require the approve function to return true. However, some ERC20 implementations (e.g., mainnet USDT) do not return a...

Vulnerable code:
if (!order.bid.token.approve(address(exchangeable), order.bid.amount)) revert InsufficientFunds();
LOW severity: Insufficient Signature Verification. **Severity**: Low

**Status** : Resolved

**Description**: 

In hexagate-callback.ts the endpoint conditionally checks the IS_HEXAGATE_SIGNATURE_DISABLED flag to determine whether to verify the x-hexagate-signature header. If this flag is disabled, the endpoint processes requests without...

Vulnerable code:
if (!IS_HEXAGATE_SIGNATURE_DISABLED) {
    const hexagateSignature = req.headers['x-hexagate-signature'];
    // ... signature verification logic ...
}
LOW severity: Insufficient Validation Checks in react/src/hooks/useSwap. **Severity**: Low

**Status** : Resolved 

**Code Snippet**:

**Scenario / Proof of Concept (POC)**:

The current validation in the swap function checks if the first element of amountChanges is zero but does not validate other elements in the array. There's a potential risk that invalid or...

Vulnerable code:
const swap = useCallback(
  async (erc20Addresses: string[], amountChanges: bigint[], externalActionId: ExternalActionId, data: string) => {
    ...
    if (amountChanges[0] === 0n) {
      throw new Error(transactionErrorCodes.AMOUNT_EMPTY);
    }
    // TODO: Add other validators
    ...
  },
  [hinkal, shieldedAddress, setTransactionStatus, requireAccessToken, runTransaction, onSuccess, onError],
);
HIGH severity: Reentrance vulnerability allowing unlimited badges minting. **Severity**: Critical

**Status**:  Resolved

**Description**

The claim function is intended to manage the process of claiming rewards by users. It performs several checks to ensure that the rewards have not already been claimed, that the claim is being made before a specified deadline, and if...

Vulnerable code:
function _mintBadge(address user) internal {
       _safeMint(user, ++lastId);
       _claimed[user] = true;
       emit Claimed(user, lastId);
   }
HIGH severity: Denial of service via front-run gas limit manipulation. **Severity**: High

**Status**:  Resolved

**Description**

The execute function of the Forwarder contract is designed to forward transactions on behalf of users, allowing for operations such as token transfers or contract interactions. However, it does not properly validate or enforce minimum gas.....

Vulnerable code:
uint gasForTransfer = 0;
       if (req.value != 0) {
           gasForTransfer = 40000; // Buffer for ETH transfers
       }
       require(gasleft() * 63 / 64 >= req.gas + gasForTransfer, "Insufficient gas");
HIGH severity: Arbitrary amount can be withdrawn (stolen). **Severity**: Critical

**Status**: Resolved

GatewayManagerFacet.sol - A vulnerability is identified in the `releaseRewardForRelayer` function. The issue allows for arbitrary amount inputs without proper validation, potentially leading to unauthorized fund drainage from the Gateway.

The...

Vulnerable code:
function releaseRewardForRelayer(uint256 amount) external nonReentrant {
        if (amount == 0) {
            revert CannotReleaseZero();
        }

        (bool registered, Subnet storage subnet) = LibGateway.getSubnet(msg.sender);
        if (!registered) {
            revert NotRegisteredSubnet();
        }

        payable(subnet.id.getActor()).sendValue(amount);
    }
HIGH severity: Validators with dust collateral can join the network. **Severity**: High

**Status**: Acknowledged

SubnetActorManagerFacet.sol - In join function , validators are allowed to join the network to be added to `genesisValidators` in exchange for dust values as low as 1 native coin. This in turn can flood the `s.genesisValidators` array. There is a...

Vulnerable code:
function depositWithConfirm(address validator, uint256 amount) internal {
        ...
        if (!s.bootstrapped) {
            // add to initial validators avoiding duplicates if it
            // is a genesis validator.
            bool alreadyValidator;
            uint256 length = s.genesisValidators.length;
            for (uint256 i; i < length; ) {
                if (s.genesisValidators[i].addr == validator) {
                    alreadyValidator = true;
                    break;
                }
                unchecked {
                    ++i;
                }
            }
            if (!alreadyValidator) {
                uint256 collateral = s.validatorSet.validators[validator].confirmedCollateral;
                Validator memory val = Validator({
                    addr: validator,
                    weight: collateral,
                    metadata: s.validatorSet.validators[validator].metadata
                });
                s.genesisValidators.push(val);
            }
        }
    }

function stake() external payable whenNotPaused notKilled {
        ...
        // AUDIT:  dust stakers are allowed
        if (msg.value == 0) {
            revert CollateralIsZero();
        }
        ...
        // AUDIT: genesisValidators are added up here
        if (!s.bootstrapped) {
            LibStaking.depositWithConfirm(msg.sender, msg.value);
            return;
        }
        LibStaking.deposit(msg.sender, msg.value);
    }
MEDIUM severity: External call is fed the wrong `QuorumObjKind` Parameter. **Severity**: Medium

**Status**: Resolved

**Description**

GatewayRouterFacet.sol - In `execBottomUpMsgBatch` function, the kind argument for `distributeRewardToRelayers` (i.e. in the encodeCall) should be `QuoromObjKind.BottomUpMsgBatch` instead of `QuoromObjKind.Checkpoint`.

The issue is...

Vulnerable code:
Address.functionCallWithValue({
    target: msg.sender,
    data: abi.encodeCall(
        ISubnetActor.distributeRewardToRelayers,
        (block.number, totalFee, QuorumObjKind.Checkpoint)
    ),
    value: totalFee
});

} else if (kind == QuorumObjKind.BottomUpMsgBatch) {
            // FIXME: The distribution of rewards for batches can't be done
            // as for checkpoints (due to how they are submitted). As
            // we are running out of time, we'll defer this for the future.
            revert MethodNotAllowed("rewards not defined for batches");
        }
MEDIUM severity: Marked active despite not having enough collateral. **Severity**: Medium

**Status**: Resolved

**Description**

GatewayManagerFacet.sol - In `register()` function, collateral is not being validated to be greater than `s.minStake`. Consequently we end up having a subnet status marked as Status.Active as shown in the following:

Noting that this...

Vulnerable code:
subnet.id = subnetId;
        subnet.stake = collateral;
        subnet.status = Status.Active;
        subnet.genesisEpoch = block.number;
        subnet.circSupply = genesisCircSupply;

if (subnet.status == Status.Inactive) {
            if (subnet.stake >= s.minStake) {
                subnet.status = Status.Active;
            }
        }
MEDIUM severity: Use < Instead of <=. **Severity** - Medium

**Status** - Resolved

**Description**

Case 1:

In the function createBottomUpMsgBatch (L353 in GatewayRouterFacet.sol) we check that if we are trying to create a batch from the future and the check for that is ->

This check would make the code revert even if `block.number.....

Vulnerable code:
if (batch.blockHeight % s.bottomUpMsgBatchPeriod != 0 || block.number <= batch.blockHeight) {
            revert InvalidBatchEpoch();        }

if (s.networkName.route.length + 1 >= s.maxTreeDepth) {
            revert MethodNotAllowed(ERR_CHILD_SUBNET_NOT_ALLOWED);
        }

if (batch.blockHeight % s.bottomUpMsgBatchPeriod != 0 || block.number < batch.blockHeight) {
            revert InvalidBatchEpoch();
        }
LOW severity: Input lack validation to be non-zero. **Severity**: Low

**Status**: Acknowledged

**Description**

LibGateway.sol - In `storeBottomUpCheckpoint` function, the input checkpoint is not being validated. It accepts a zero value of checkpoint.blockHeight. Note that if blockHeight is zero it implies the non-existence of a checkpoint to...

Vulnerable code:
function storeBottomUpCheckpoint(
        BottomUpCheckpoint memory checkpoint
    ) internal {
        GatewayActorStorage storage s = LibGatewayActorStorage.appStorage();
        s.bottomUpCheckpoints[checkpoint.blockHeight] = checkpoint;
    }

function createBottomUpCheckpoint(
        BottomUpCheckpoint calldata checkpoint,
        bytes32 membershipRootHash,
        uint256 membershipWeight
    ) external systemActorOnly {
        if (checkpoint.blockHeight % s.bottomUpCheckPeriod != 0) {
            revert InvalidCheckpointEpoch();
        }
        if (LibGateway.bottomUpCheckpointExists(checkpoint.blockHeight)) {
            revert CheckpointAlreadyExists();
        }

        LibQuorum.createQuorumInfo({
            self: s.checkpointQuorumMap,
            objHeight: checkpoint.blockHeight,
            objHash: keccak256(abi.encode(checkpoint)),
            membershipRootHash: membershipRootHash,
            membershipWeight: membershipWeight,
            majorityPercentage: s.majorityPercentage
        });
        LibGateway.storeBottomUpCheckpoint(checkpoint);
    }
HIGH severity: CryptoPunks can be stolen via mint frontrunning. **Severity**: Critical

**Status**:  Resolved

**Description**

Since the CryptoPunks NFT collection not implementing the ERC721 standard, depositing of CryptoPunks NFTs is handled via a separate flow:
Token owner needs to call `offerPunkForSaleToAddress` and set the `toAddress` value to the...

Vulnerable code:
it("CryptoPunk can be stolen via mint frontrunning", async () => {
   // user 1 want to supply CryptoPunk NFT
   await app.cryptopunks
     .connect(app.user1)
     .offerPunkForSaleToAddress(tokenId, 0, app.pcryptopunks.address);


   //user 2 frontrun the mint call
   await app.pcryptopunks.connect(app.user2).mint(tokenId);
   await app.unitroller
     .connect(app.user2)
     .enterNFTMarkets([app.pcryptopunks.address]);


   expect(await app.pcryptopunks.ownerOf(tokenId)).to.equal(app.user2.address);
 });
HIGH severity: Borrowers can get Interest-free loans. **Severity**: High

**Status**: Acknowledged

**Description**

In Contract PTokens.sol, the borrowing balance of a borrower is calculated as follows:

The above formula in Solidity is as following, 

Here, when the principal and ratio of borrow indices are both small the result can equal the...

Vulnerable code:
/* Calculate new borrow balance using the interest index:
* recentBorrowBalance = borrower.borrowBalance * market.borrowIndex / borrower.borrowIndex
*/

uint principalTimesIndex = mul_(borrowSnapshot.principal, borrowIndex);
return div_(principalTimesIndex, borrowSnapshot.interestIndex);
MEDIUM severity: Asset is not approved to the zero first. **Severity**: Medium

**Status**:  Resolved

**Description**

Some ERC20 tokens (like USDT) do not work when changing the allowance from an existing non-zero allowance value. For example, Tether (USDT)'s `approve()` function will revert if the current approval is not zero, to protect against...

Vulnerable code:
require(EIP20Interface(assetToSell).approve(address(SWAP_ROUTER), amountInMaximum), "Liquidator: exchange tokens approve failed");
LOW severity: Possible overflow in error codes. **Severity**: Low

**Status**: Acknowledged

**Description**

In Contract PEther.sol, the method `requireNoError` is using the calculation as follows to calculate the full message.

Here, there is a typecast to uint8 which will keep only the least 8 significant bits, and any errorCode greater than.....

Vulnerable code:
fullMessage[i+2] = byte(uint8(48 + ( uint(err) / 10 )));
MEDIUM severity: Strict equality in `migrate()` method. **Severity** : Medium

**Status** : Resolved

**Description** :

In Contract TokenMigration.sol, the method migrate() allows users to migrate their TWSDM tokens to WSDM tokens. 

This method contains the following logic:

If the balance is not equal to the investor shares, it will fail to migrate...

Vulnerable code:
uint256 angelInvestorShares = angelInvestors[msg.sender];
       uint256 strategicInvestorShares = strategicInvestors[msg.sender];
       require(
           balance == angelInvestorShares + strategicInvestorShares,            "TokenMigration: twsdm balance does not match with investor shares"
       );
MEDIUM severity: Emergency Exit Logic Undermines Pausable Mechanism. **Severity** : Medium

**Status** : Resolved

**Description** :

The contract Locking implements an emergency exit mechanism alongside the Pausable functionality inherited from OpenZeppelin's Pausable contract. However, the logic used to determine whether actions can proceed during an emergency or.....

Vulnerable code:
contract Locking is Pausable {
    bool private emergencyExit = false;

    modifier whenNotPausedOrEmergencyExitActive() {
        require(!paused() || emergencyExit, "Contract is not paused and not in emergency");
        _;
    }

    function activateEmergencyExit() public onlyOwner {
        emergencyExit = true;
    }

    function emergencyFunction() public whenNotPausedOrEmergencyExitActive {
        // Emergency function logic here
    }

    function normalFunction() public whenNotPaused {
        // Normal function logic here
    }
}
MEDIUM severity: Use of `SafePermit` is removed Based on recommendations from @trust1995. **Severity** : Medium

**Status** : Resolved

**Description** :

In the latest openzeppelin-contracts safePermit has been removed

https://github.com/OpenZeppelin/openzeppelin-contracts/pull/4582 

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/192e873fcb5b1f6b4b9efc177be231926e2280d2/C...

Vulnerable code:
try token.permit(msg.sender, spender, value, deadline, v, r, s) {} catch {}
LOW severity: Precision Loss due to truncated decimals. **Severity** : Low

**Status** : Resolved

**Description** :

This line will cause precision loss, however it mainly depends on the constructor arguments. This bug is due to the lack of support for floating-point arithmetic, which means Solidity can only handle integer...

Vulnerable code:
uint256 tge_round_release = (totalAllocation * tgeReleasePercentage) / 100;
LOW severity: Penalty fee can be higher than the user balance. **Severity** : Low

**Status** : Resolved

**Description** :

In Contract locking.sol, the method unlock allows users to unlock their locked tokens but a penalty fee is incurred on them.

The amount fee is calculated as follows:

Here if the method calculatePenaltyFee returns a value > 10^6, then...

Vulnerable code:
function calculatePenalty(address locker, uint256 balance, uint256 unlockTimestamp) public view returns (uint256) {
       if (emergencyExit) {
           return 0;
       }
       return (balance * calculatePenaltyFee(locker, unlockTimestamp)) / 10 ** 6;
   }
LOW severity: Records can be overridden without checking. **Severity**: Low

**Status**: Resolved

**Location**: TokenMigration.sol

**Description** :

In function `_addInvestor()`, the values inside mappings angelInvestors and strategicInvestors can be overridden. Suppose a scenario in which the owner (i.e. procedure can only be triggered by owner)...

Vulnerable code:
function _addInvestor(address investor, uint256 balance, InvestorType investorType) private {
        require(investor != address(0), "TokenMigration: investor should not be address zero");

        if (investorType == InvestorType.ANGEL) {
            angelInvestors[investor] = balance;
        } else if (investorType == InvestorType.STRATEGIC) {
            strategicInvestors[investor] = balance;
        }

        emit InvestorInserted(investor, balance, investorType);
    }

require(
    balance == angelInvestorShares + strategicInvestorShares,
        "TokenMigration: twsdm balance does not match with investor shares"
);
HIGH severity: Lack of Return Value Validation in ERTransfer. **Severity**: Low

**Status**: Acknowledged

**Location**: AgentImplementation.sol

**Description**

The smart contract initiates ERC20 token transfers without validating the return value of the transfer operation. Failing to check the return value may result in unhandled failures, exposing the...

Vulnerable code:
if (_pFILBalance > 0) {
    IPFIL(pFIL).transfer(owner, _pFILBalance);
}

IPFIL(pFIL).transferFrom(msg.sender, address(this), _receive);
HIGH severity: The Keeper Management fee is being calculated wrong. **Severity**: High

**Status**: Acknowledged

**Description**

In Contract GlmVault.sol, the method `keeperManagementFeePreview()` calculates the fee for the keeper in gmTokens.

This method checks if the set interval has passed and calculates the fee as follows:

Here, the final `usdcAmount` only.....

Vulnerable code:
require(timeElapsed >= info.managementFeeInterval, "Not enough time elapsed");
       uint256 usdcAmountFor1Year = (totalAssets() * info.managementFeeBpsFor1Year) / DECIMAL_PRECISION;


       uint256 usdcAmount = (usdcAmountFor1Year * info.managementFeeInterval) / (365 days);
HIGH severity: GM tokens were deposited but no shares were minted. **Severity**: High

**Status**: Resolved

**Description**

In Contract GlmManger.sol, the method getGmTokenValueInUSDC() has the following logic:

Here if `_amount` is less than 1e12 (let’s say 1e11), the return will be (1e30*1e11*1e6)/(1e48) = 0 but the `_amount` can be of much lower value and...

Vulnerable code:
uint256 gmTokenPrice = IGlmPriceReader(glm.priceReader).getMarketTokenPrice(_gmToken);
       uint256 usdcPrice = IGlmPriceReader(glm.priceReader).getTokenPrice(USDC) / 1e18; 
       return ((gmTokenPrice * _amount) * usdcPrice) / 1e30 / 1e18; //return 1e6
MEDIUM severity: Method `requestWithdrawal` can fail if GMX reverts the txn. **Severity**: Medium

**Status**: Resolved

**Description**

In Contract GlmRouter.sol, the method `requestWithdrawal(...)` checks if the `glmAmount` being withdrawn is > `allPoolSplitingBps`. If that is the case, then tokens are withdrawn from all GM pools based on the current weight...

Vulnerable code:
for (uint i = 0; i < poolLength; i++) {
               uint256 amount = ((manager.getCurrentWeight(i, totalAssets) * _glmAmount)) / DECIMAL_PRECISION; //if it is the last pool, withdraw the remaining glm amount
               if (i == poolLength - 1) {
                   keys[i] = _processWithdrawal(remainingGlmAmount, manager.getGmTokenAddress(i), glmPrice);                } else {
                   keys[i] = _processWithdrawal(amount, manager.getGmTokenAddress(i), glmPrice);
                   remainingGlmAmount -= amount;
               }
           }
LOW severity: Rebalance will fail if either ETH or BTC pools’ current weight is 0. **Severity**: Low

**Status**: Resolved

**Description**

In Contract GlmRebalance, the method `startRebalance()` calls `getLatestRebalInfo()` which further calls `isCorePoolInRatioInRange()` which checks if the BTC/ETH pools ratio is higher than the upperRation/lowerRation or not.

The method...

Vulnerable code:
uint256 ratio = btcPoolWeight > ethPoolWeight
           ? (btcPoolWeight * DECIMAL_PRECISION) / ethPoolWeight            : (ethPoolWeight * DECIMAL_PRECISION) / btcPoolWeight;
LOW severity: Hardcoded index values for BTC and ETH can be problematic. **Severity**: Low

**Status**: Resolved

**Description**

Across the protocol, several methods use 0 as the index for the ETH pool and 1 as the index for the BTC pool.

GlmManager.sol contract allows to deletion/removal of a pool using the following logic:

Here, the deleted pool index is replaced.....

Vulnerable code:
address marketToken = gmPools[_index].marketToken;
       gmPools[_index] = gmPools[gmPools.length - 1];
       gmPools.pop();
       delete marketTokenToGmPoolInfo[marketToken];