Farseen0/scar-pairs huggingface dataset:
LOW severity: OWNERSHIP CAN BE TRANSFERRED TO `ADDRESS(0)`. **Severity**: Low	

**Status**: Resolved

**Description**

The `TonBridge.sol` contract is `OwnableUpgradeable` which contains a `renounceOwnership()` function that transfers ownership to address(0), which cause the same effect as having the contract without owner:

If ownership is renounced then...

Vulnerable code:
/**
    * @dev Leaves the contract without owner. It will not be possible to call
    * `onlyOwner` functions anymore. Can only be called by the current owner.
    *
    * NOTE: Renouncing ownership will leave the contract without an owner,
    * thereby removing any functionality that is only available to the owner.
    */
   function renounceOwnership() public virtual onlyOwner {
       _transferOwnership(address(0));
   }
LOW severity: MISSING SANITY CHECKS FOR IMPORTANT VARIABLES. **Severity**: Low	

**Status**: Resolved

**Description**

There are some important variables in the `TonBridge.sol` contract that are not checked before being set in the `initialize()` function and also in the functions used for changing their values.

- Initialize():

- Functions used for...

Vulnerable code:
function initialize(address _tonBridge, address _symbiosisBridge, uint256 _bridgeChainId, address _broadcaster) public virtual initializer {       

       __Ownable_init(); 
       tonBridge = _tonBridge;
       symbiosisBridge = _symbiosisBridge;
       bridgeChainId = _bridgeChainId;
       broadcaster = _broadcaster;
}

function changeBridgeChainId(uint256 _newBridgeChainId) external onlyOwner { 
       bridgeChainId = _newBridgeChainId;
       emit ChangeBridgeChainId(_newBridgeChainId);
   }

   /**
    * @notice Changes TON Bridge by owner
    */
   function changeTonBridge(address _newTonBridge) external onlyOwner {
       tonBridge = _newTonBridge;
       emit ChangeTonBridge(_newTonBridge);
   }

   /**
    * @notice Changes Symbiosis Bridge by owner
    */
   function changeSymbiosisBridge(address _newSymbiosisBridge) external onlyOwner {
       symbiosisBridge = _newSymbiosisBridge;
       emit ChangeSymbiosisBridge(_newSymbiosisBridge);
   }

   /**
    * @notice Changes Broadcaster by owner
    */
   function changeBroadcaster(address _newBroadcaster) external onlyOwner {
       broadcaster = _newBroadcaster;
       emit ChangeBroadcaster(_newBroadcaster);
   }


Override `renounceOwnership()` function and revert the execution to do not allow ownership getting revoked:

function renounceOwnership() public override onlyOwner { 
revert ("NotAllowed”); 
}
LOW severity: USE A MULTISIG ACCOUNT FOR OWNER. **Severity**: Low	

**Status**: Acknowledged

**Description**

The functions `changeBridgeChainId()`, `changeTonBridge()`, `changeSymbiosisBridge()` and `changeBroadcaster` are onlyOwner callable.This means the owner has the authority to change key variable values that affect core functionality....

Vulnerable code:
function changeBridgeChainId(uint256 _newBridgeChainId) external onlyOwner { 
       bridgeChainId = _newBridgeChainId;
       emit ChangeBridgeChainId(_newBridgeChainId);
   }

   /**
    * @notice Changes TON Bridge by owner
    */
   function changeTonBridge(address _newTonBridge) external onlyOwner {
       tonBridge = _newTonBridge;
       emit ChangeTonBridge(_newTonBridge);
   }

   /**
    * @notice Changes Symbiosis Bridge by owner
    */
   function changeSymbiosisBridge(address _newSymbiosisBridge) external onlyOwner {
       symbiosisBridge = _newSymbiosisBridge;
       emit ChangeSymbiosisBridge(_newSymbiosisBridge);
   }

   /**
    * @notice Changes Broadcaster by owner
    */
   function changeBroadcaster(address _newBroadcaster) external onlyOwner {
       broadcaster = _newBroadcaster;
       emit ChangeBroadcaster(_newBroadcaster);
   }
HIGH severity: Signature can be reused unlimited number of times. **Severity**: High

**Status**: Resolved

**Location**: Airdrop.sol

**Description**

The signature can be reused by a user to claim the airdrop again and again for unlimited number of times every week. If users who do not claim their airdrop, it could result in other users claiming reward meant...

Vulnerable code:
// tokens meant for Aliceis claimed by Bob by reusing previous signature
       await airdropZap
       .connect(Bob)
       .claimRewards(
         1,
         chapterIds,
         erc20RewardsForMultipleChapter,
         ownerSignature,
       );


      expect(await erc20Token.balanceOf(addr7.address)).to.equal(10000);


await time.increase(3600*24*7);
     await airdropZap
     .connect(Bob)
     .claimRewards(
       1,
       chapterIds,
       erc20RewardsForMultipleChapter,
       ownerSignature,
     );


   expect(await erc20Token.balanceOf(addr7.address)).to.equal(20000);
MEDIUM severity: The owner of the `AirdropFactory` contract can withdraw all the funds from the locker. **Severity**: Medium

**Status**: Acknowledged

**Location**: AirdropFactory.sol

**Description**

The owner of the `AirdropFactory` contract can withdraw all the tokens from the lockers that are created using the `createAirdropProject()` function. This is because the ownership of all the lockers...

Vulnerable code:
describe('removeAllAirdropFundsByOwner', function () {
   it('Owner trying to take all aridrop funds by himself', async function () {
     const OwnerofLocker = await locker.owner();
     expect(OwnerofLocker).to.equal(owner);
     await locker.connect(owner).transferERC20(erc20Token.target, 40000, owner);
     console.log("Balance of Owner after Locker withdrawal:", await erc20Token.balanceOf(owner));
    
     }
   )}
 );
MEDIUM severity: Inadequate Handling of Fee-on-Transfer Tokens. **Severity**: Medium

**Status**: Resolved

**Description**: 

The `AirdropFactory` contract does not account for tokens that charge a fee on transfer. When transferring such tokens, the actual amount received by the recipient is less than the specified amount due to the transfer fee. This...

Vulnerable code:
- Due to the fee-on-transfer mechanism, only 950 tokens (assuming a 5% fee) are actually received by the Locker contract.
- The contract logic, however, assumes that 1000 tokens were transferred, leading to potential underfunding of the airdrop distribution.
MEDIUM severity: Missing length check between array parameters of `createAirdropProject()` and `createChapters()` functions. **Severity**: Medium

**Status**: Resolved

**Location**: AirdropFactory.sol

**Description**

In the function `createAirdropProject()`, there is missing length check between length of `erc20RewardAddresses` of chapters array and the length `erc20Rewards` array parameter. This can result in the...

Vulnerable code:
function createAirdropProject(
       uint256 parentAirdropId,
       ChapterData[] calldata chapters,
       ERC20Reward[] calldata erc20Rewards
   ) external payable nonReentrant {
LOW severity: Use memory instead of storage in `createChapters()` function. **Severity**: Low

**Status**: Resolved

**Location**: AirdropFactory.sol

**Description**

In `createChapters()`, storage has been used instead of memory for reading values from `AirdropList`. The usage of storage is not needed as there are no state changes that are being made in the function,...

Vulnerable code:
AirdropConfig storage airdrop = AirdropList[parentAirdropId];
LOW severity: Incorrect/inconsistent comment with implementation for `Locker.sol`. **Severity**: Low

**Status**: Resolved

**Location**: Locker.sol

**Description**

There is inconsistency in the comments and implementation in Locker.sol. The following code on line: 12 states that the contract implements IERC721Receiver for handling ERC721 tokens. But there is no such...

Vulnerable code:
line: 12 /// @dev Implements both IERC721Receiver for handling incoming ERC721 tokens and ReentrancyGuard for secure native token transfers.
HIGH severity: Facilitators can borrow assets and never repay it back. **Severity**: High

**Status**: Acknowledged

**Description**

It is mentioned in the documentation that a `Facilitator` has the capability to borrow assets from the `vault`. However, upon reviewing the `AdvancedOrderEngine` smart contract, no mechanism for verifying whether the borrowed assets...

Vulnerable code:
701:     function _processFacilitatorInteraction(
702:         bytes calldata facilitatorInteraction,
703:         OrderEngine.Order[] calldata orders,
704:         uint256[] calldata executedSellAmounts,
705:         uint256[] calldata executedBuyAmounts,
706:         ERC20[] calldata borrowedTokens,
707:         uint256[] calldata borrowedAmounts
708:     ) private {
..
724:             // Transfer funds to the 'interactionTarget' address.
725:             for (uint256 i; i < borrowedTokens.length; ) {
726:                 _sendAsset(
727:                     borrowedTokens[i],
728:                     borrowedAmounts[i],
729:                     interactionTarget
730:                 );
731:                 unchecked {
732:                     ++i;
733:                 }
734:             }
LOW severity: USE A MULTISIG ACCOUNT FOR THE OWNER. **Severity**: Low	

**Status**: Acknowledged

**Description**

The functions `updateUsdt()`, `updateWeth()`, `updateOracle()` are onlyOwner callable. It means that the owner has the power of changing important variable values for the core behavior so it is recommendable to use a multisig account...

Vulnerable code:
function updateUsdt(address _usdt) external onlyOwner {
       usdt = _usdt;
   } 

   function updateWeth(address _weth) external onlyOwner {
       weth = _weth;
   } 

   function updateOracle(address _oracle) external onlyOwner {
       oracle = IStaticOracle(_oracle);
   }
LOW severity: Owner can renounce ownership. **Severity**: Low

**Status**: Resolved

**Description**

The Ownable2Step and Ownable contracts includes a function named `renounceOwnership()` which can be used to remove the ownership of the contract. 
If this function is called on the `AdvancedOrderEngine` contract, it will result in the...

Vulnerable code:
function renounceOwnership() public override onlyOwner { 
revert ("renounceOwnership is disabled"); 
}
LOW severity: Improper Order of Modifiers in `fillOrders()`. **Severity**: Low

**Status**: Resolved

**Description**

The `fillOrders()` function of `AdvancedOrderEngine` contract currently places the nonReentrant modifier after the onlyOperator modifier. The order of modifiers can influence the behavior of a function, and it is generally recommended to...

Vulnerable code:
File: AdvancedOrderEngine.sol
311:     function fillOrders(
..
319:     ) external onlyOperator nonReentrant {
LOW severity: Misleading Documentation for recipient Address Handling. **Severity**: Low

**Status**: Resolved

**Description**

The documentation for the `AdvancedOrderEngine` contract here specifies that if a user wants the recipient address to be the same as the maker address, they should set the recipient address to the null address (address(0)). However, the...

Vulnerable code:
533:     function _validateOrder(
534:         OrderEngine.Order calldata order,
535:         uint256 executedSellAmount,
536:         uint256 executedBuyAmount,
537:         bytes32 orderHash
538:     ) private view {
..
569:         // Revert if any address in the order is zero.
570:         if (
571:             order.maker == address(0) ||
572:             order.buyToken == address(0) ||
573:             order.sellToken == address(0) ||
574:             order.recipient == address(0)  
575:         ) {
576:             revert ZeroAddress();
577:         }
LOW severity: Missing Sanity checks. **Severity**: Low

**Status**: Resolved

**Description**

It was found that the constructor of the Airdrops.sol smart contract does not include checks to validate the provided parameters are not zero. Although this is not critical, failing to perform this validation can lead to unexpected...

Vulnerable code:
30:     constructor(bytes32 _merkleRoot, address _rewardContract) { 
31:         merkleRoot = _merkleRoot;
32:         rewardContract = _rewardContract;
33:     }
LOW severity: Incorrect Event Emission in Constructor. **Severity**: Low

**Status**: Resolved

**Location**: AdvancedOrderEngine.sol

**Description**

The `feeCollectorAddr` parameter is incorrectly emitted as part of the `PredicatesChanged` event in the constructor of the `AdvancedOrderEngine` contract, where it should be `predicatesAddr`...

Vulnerable code:
emit PredicatesChanged(address(0), feeCollectorAddr);
HIGH severity: Method `revertEscrow()` doesn’t work as expected. **Severity**: High

**Status**: Resolved

**Description**

Method `revertEscrow()` has the following logic:

Here. this method is supposed to be called only by owner but later on it checked that `msg.sender` is either seller or buyer which is contradicting as `msg.sender` can not owner and...

Vulnerable code:
function revertEscrow(string calldata refId) external onlyOwner {
   Escrow storage escrow = escrows[refId];


   require(escrow.seller != address(0) || escrow.buyer != address(0), "Escrow does not exist.");


   bool isSeller = msg.sender == escrow.seller; 
   bool isBuyer = msg.sender == escrow.buyer;


   require(isSeller || isBuyer, "Only the escrow creator can cancel before funding.");
… }
LOW severity: CEI pattern is not followed. **Severity**: Low

**Status**: Resolved

**Description**

Most of the methods of the contract follow this pattern: (Check, Interaction, Effects)

Here, it is advised to update the state before making external calls such as transferring ERC20 tokens to avoid reentrancy attack.

In case, ERC777...

Vulnerable code:
function methodName(string calldata refId) external onlyOwner {
// Checks
   Escrow storage escrow = escrows[refId];
   require(escrow.sellerFundsLocked && escrow.buyerFundsLocked, "Funds not locked");


   uint256 feeAmount = (escrow.amount * feePercentage) / 10000;
// Interaction
   IERC20 token = IERC20(escrow.tokenAddress);


   _transferFromEscrow(token, escrow.walletAddress, escrow.seller, escrow.amount);
   token.safeTransferFrom(feeReceiver, escrow.seller, feeAmount);


   _transferFromEscrow(token, escrow.walletAddress, escrow.buyer, escrow.amount);
   token.safeTransferFrom(feeReceiver, escrow.buyer, feeAmount);
// Effects
   _removeEscrow(refId);


   emit EscrowCancelled(refId);
 }
HIGH severity: `SharePrice` can get manipulated. **Severity**: Critical	

**Status**: Acknowledged

**Description**

The `_withdrawFromTarget` function in the `CurveLiquidityWarehouse.sol` contract calculates the theoric price for a LP share relying on the spot price of the curve pool.

The `calc_withdraw_one_coin` function from the Curve’s...

Vulnerable code:
uint256 sharePrice = curveStableSwapPool.calc_withdraw_one_coin(10 ** (withdrawTargetDecimals), curvePoolCoinIdx);
MEDIUM severity: Lack of Fallback Mechanism for Stale Price Feeds in `_getAssetOraclePrice` Function. **Severity**: Medium

**Status**: Resolved

**Location**: GammaSwapLiquidityWarehouse.sol

**Description**

The `_getAssetOraclePrice` function in the `GammaLiquidityWarehouse` contract checks for stale price feeds but does not handle the case where the price feed is stale. If the price feed is...

Vulnerable code:
function _getAssetOraclePrice(address asset) internal view returns (uint256) {
    AggregatorV3Interface priceFeed = s_assetConfigs[asset].chainlinkPriceFeed;
    AggregatorV3Interface fallbackPriceFeed = s_assetConfigs[asset].fallbackPriceFeed; // Add a secondary price feed
    (, int256 oraclePrice,, uint256 updatedAt,) = priceFeed.latestRoundData();
    if (block.timestamp - updatedAt > STALENESS_THRESHOLD) {
        // Use fallback price feed if the primary price feed is stale
        (, int256 fallbackOraclePrice,, uint256 fallbackUpdatedAt,) = fallbackPriceFeed.latestRoundData();
        if (block.timestamp - fallbackUpdatedAt > STALENESS_THRESHOLD) {
            revert ChainlinkPriceFeedStale();
        }
        oraclePrice = fallbackOraclePrice;
    }
    (, int256 answer, uint256 startedAt,,) = i_sequencerUptimeFeed.latestRoundData();
    if (answer == 1) {
        revert SequencerDown();
    }
    if (block.timestamp - startedAt <= GRACE_PERIOD_TIME) {
        revert GracePeriodNotOver();
    }
    return oraclePrice.toUint256() * 10 ** (18 - priceFeed.decimals());
}
MEDIUM severity: Incorrect Staleness Threshold for Chainlink Price Feeds. **Severity**: Medium

**Status**: Resolved

**Location**: GammaSwapLiquidityWarehouse.sol

**Description**

The current implementation uses a single `STALENESS_THRESHOLD` constant (24 hours) for all Chainlink price feeds. This approach is problematic because different price feeds have vastly...

Vulnerable code:
if (block.timestamp - updatedAt > STALENESS_THRESHOLD) revert ChainlinkPriceFeedStale();

mapping(address => uint256) private s_stalenessTresholds;

function _getAssetOraclePrice(address asset) internal view returns (uint256) {
    AggregatorV3Interface priceFeed = s_assetConfigs[asset].chainlinkPriceFeed;
    (, int256 oraclePrice,, uint256 updatedAt,) = priceFeed.latestRoundData();
    if (block.timestamp - updatedAt > s_stalenessTresholds[asset]) revert ChainlinkPriceFeedStale();
    // ... rest of the function
}
LOW severity: Unused Constant `DELTASWAP_PROTOCOL_ID` in GammaLiquidityWarehouse Contract. **Severity**: Low

**Status**: Resolved

**Location**: GammaSwapLiquidityWarehouse.sol

**Description**

The `GammaLiquidityWarehouse` contract defines a constant `DELTASWAP_PROTOCOL_ID` with a value of 3. However, this constant is never used within the contract. Unused constants can lead to...

Vulnerable code:
/// @notice The deltaswap protocol id
uint16 internal constant DELTASWAP_PROTOCOL_ID = 3;
LOW severity: Return value ignored and unhandled. **Severity**: Low	

**Status**: Acknowledged

**Description**

In the `GammaSwapLiquidityWarehouse.sol` contract, in the `_swapToBaseAsset()` function, the value returned by the `exactInput()` is being ignored and unhandled.

**Recommendation**: 

It is advised that the returned value is handled...

Vulnerable code:
Line: 289	ISwapRouter(s_assetConfigs[asset].swapRouter).exactInput(
                ISwapRouter.ExactInputParams({
                    path: s_assetConfigs[asset].swapPath,
                    recipient: address(this),
                    deadline: block.timestamp + s_assetConfigs[asset].deadline,
                    amountIn: assetBalance,
                    amountOutMinimum: assetOutExpected * (WAD - slippage) / WAD
                })
            );
LOW severity: `_withdrawFromTarget` Would Revert If asset `Decimals > Withdraw` Target Decimals. **Severity**: Low

**Status**: Resolved

**Description**

The `minReceived` inside `withdrawFromTarget` is calculated as follows -> 

This normalises `burnAmt` to be in the asset's decimal precision , but it would revert if `(s_terms.asset)).decimals() > withdrawTargetDecimals`...

Vulnerable code:
uint256 minReceived =
            burnAmt / 10 ** (withdrawTargetDecimals - IERC20Metadata(address(s_terms.asset)).decimals()) - maxLossAmount;
LOW severity: Unhandled Chainlink Oracle Access Revocation Risk. **Severity**: Low

**Status**: Resolved

**Location**: GammaSwapLiquidityWarehouse.sol

**Description**

The `_getAssetOraclePrice` function in the `GammaLiquidityWarehouse` contract directly calls the `latestRoundData` function of Chainlink price feeds without proper error handling. This approach.....

Vulnerable code:
function _getAssetOraclePrice(address asset) internal view returns (uint256) {
    AggregatorV3Interface priceFeed = s_assetConfigs[asset].chainlinkPriceFeed;

    try priceFeed.latestRoundData() returns (
        uint80,
        int256 oraclePrice,
        uint256,
        uint256 updatedAt,
        uint80
    ) {
        if (block.timestamp - updatedAt > STALENESS_THRESHOLD) revert ChainlinkPriceFeedStale();

        try i_sequencerUptimeFeed.latestRoundData() returns (
            uint80,
            int256 answer,
            uint256 startedAt,
            uint256,
            uint80
        ) {
            if (answer == 1) {
                revert SequencerDown();
            }
            if (block.timestamp - startedAt <= GRACE_PERIOD_TIME) {
                revert GracePeriodNotOver();
            }
            return oraclePrice.toUint256() * 10 ** (18 - priceFeed.decimals());
        } catch {
            // Handle sequencer uptime feed failure
            // e.g., revert with a custom error, use a fallback mechanism, etc.
            revert SequencerUptimeFeedFailure();
        }
    } catch {
        // Handle price feed failure
        // e.g., revert with a custom error, use a fallback oracle, etc.
        revert PriceFeedAccessRevoked();
    }
}
HIGH severity: Incorrect Check For Early Stakers. **Severity** - High

**Status** - Resolved

**Description**

For the first 99 stakers the minimum staking amount is 8e6 ether tokens (compared to 16e6 ether tokens for non-early stakers) , the code which verifies if the staker is an early staker is →

It checks if the stakerIndex == 99 , which is...

Vulnerable code:
function canUseReducedStakingAmount(address stakerAddress) public view returns (bool eligible, uint256 requiredStakingAmount) {
    EarlyStaking earlyStaking = EarlyStaking(payable(getContractAddress("EarlyStaking")));
    int256 stakerIndex = earlyStaking.getIndexOf(stakerAddress);
    bool isEarlyStaker = stakerIndex >= 0; // True if stakerIndex is not -1, meaning the staker exists


    // Assuming the first early staker gets a special treatment and should use the reduced amount
    if (isEarlyStaker) {
        if (stakerIndex == 99) {
            // First 100 early stakers, eligible for reduced amount
            return (true, 8_000_000 ether);
        }

If (stakerIndex <= 99) return (true , 8_000_000 ether);
HIGH severity: Incorrect Check To Verify startTime. **Severity** - High

**Status** - Resolved

**Description**

The start time for staking is assigned in the function `recordStakingStart()` , to verify if the `startTime` is a valid timestamp the following check is performed →

But this would be incorrect in a general sense since startTime can be...

Vulnerable code:
if (startTime > block.timestamp) {
        revert("InvalidStartTime");
    }

if (startTime < block.timestamp) {
        revert("InvalidStartTime");
    }
HIGH severity: Incorrect check in the `createMinipool()` function. **Severity**: High

**Status**: Resolved

**Description**

MinipoolManager.sol L244
For the early stakers in the `createMinipool()` function, the function only accepts the combined contribution equal to the DAO’s minimum requirement and `plsAssignmentRequest` equal to the DAO’s max PLS...

Vulnerable code:
if (msg.value + plsAssignmentRequest < dao.getMinipoolMinPLSStakingAmt() || plsAssignmentRequest > dao.getMinipoolMaxPLSAssignment())
HIGH severity: Non-Standard ERtokens could be locked in the contract. **Severity**: High

**Status**: Resolved

**Description**

The ERC20 transfer function is used to transfer `rewardToken` tokens. However, some ERC20 tokens, such as USDT, BNB, and OMG, do not return a boolean. This can cause the `require(success, "Token transfer failed")` check to always fail,...

Vulnerable code:
bool success = IERC20(rewardToken).transfer(owner(), amount);

SafeERC20.safeTransfer(IERC20(rewardToken), owner(), amount);
HIGH severity: Out of range index in `getTokenBurnFee()` causes function reversion after 14 days. **Severity**: Critical

**Status**: Resolved

**Location**: PoolBase.sol

**Description**

The `getTokenBurnFee()` function in `PoolBase.sol` attempts to return `burnFees[feeCycle.length]` after 14 days of asset locking, which is out of range for the burnFees array. This out-of-bounds access...

Vulnerable code:
920 function getTokenBurnFee(address _staker) public view returns (uint256) {
921     User memory user = users[_staker];
922     for (uint256 i; i < feeCycle.length; i++) {
923     if (
924         (block.timestamp < user.lastUnstakedTimestamp + feeCycle[i]) ||
925         block.timestamp < user.lastInvestTimestamp + feeCycle[i]
926     ) {
927         return burnFees[i];
928     }
929     }
930     return burnFees[feeCycle.length];
931 }