ZealousSwap Kaspa AMM DEX Security Review Report

The ZEALInfinityPool.stake() function mints xZeal shares in exchange for Zeal tokens. The number of shares minted is calculated as follows:

1. currentRate = (totalStaked + totalRewards).mulDiv(PRECISION_FACTOR, xSupply);
 
2. xMint = _amount.mulDiv(PRECISION_FACTOR, currentRate);

Since the calculation uses rounding down, xMint will be zero if _amount * PRECISION_FACTOR < currentRate. An attacker can exploit this by inflating the exchange rate using the addRewards() function, effectively making the next user's stake lose half of value.

Exploit Scenario (Assuming zealPerBlock = 0)

1. Initial Stake:

• The attacker stakes minStakeAmount, receiving the same amount in xZeal.
• State:
  • totalStaked = minStakeAmount
  • xZeal.totalSupply() = minStakeAmount
  • getExchangeRate() = 1e18

2. Partial Unstake:

• The attacker unstakes minStakeAmount - 1 xZeal.
• State:
  • totalStaked = 1
  • xZeal.totalSupply() = 1
  • getExchangeRate = 1e18

3. Victim Prepares to Stake:

• Alice (a regular user) submits a transaction to stake X Zeal.

4. Front-Run with Rewards:

• The attacker front-runs Alice by calling addRewards(X / 2).
• This increases totalStaked without changing xZeal.totalSupply, causing the exchange rate to spike.
• State:
  • totalStaked = 1 + X / 2
  • xZeal.totalSupply() = 1
  • getExchangeRate() = (1 + X / 2) * 1e18

5. Alice's Stake Executes:

• Alice's minted shares: xMint = X * 1e18 / ((1 + X / 2) * 1e18) = X / (1 + X / 2)
  • Due to rounding, xMint = 1 Outcome

The attacker eventually calls unstake() to reclaim their Zeal tokens, they will receive:

    (X + 1 + X / 2) / 2
 
  = (X / 2 + 1) + (X / 4 - 0.5)

Here:

• (X / 2 + 1) represents the attacker's initial investment in the strategy.
• (X / 4 - 0.5) is the net profit gained from front-running and draining the next user's stake.

uint256 xMint = _amount.mulDiv(PRECISION_FACTOR, currentRate);
 
require(xMint > 0, "Stake amount too small");
 
totalRewards += _amount;
totalManualRewards += _amount;
 
uint256 xSupply = xZealToken.totalSupply();
uint256 newRate;
 
if (xSupply == 0) {
    newRate = currentRate;
} else {
    newRate = (totalStaked + totalRewards).mulDiv(
        PRECISION_FACTOR,
        xSupply
    );
}

Consider burning an initial amount of xZeal tokens (representing dead shares), similar to the approach used in the ZealousSwapPair contract.

The function ZealousSwapPair.swap() facilitates token swaps and takes an input parameter actualUser, representing the true initiator of the swap (typically the caller of the router contract). This actualUser is used to determine fee eligibility—specifically, whether the user qualifies for a discount—within lines 287 to 291. If eligible, the swap fee is reduced from regularFee to discountFee.

function swap(
    uint amount0Out,
    uint amount1Out,
    address to,
    bytes calldata data,
    address actualUser /// @audit There is no guarantee that actualUser is the actual caller
) external lock {
    ...
 
    {
        // Scope for reserve{0,1}Adjusted to avoid stack-too-deep errors
        if (discountManagerContract != address(0)) {
            isDiscountEligible = IZealousSwapDiscountManager(
                discountManagerContract
            ).isDiscountEligible(actualUser);
        }
        uint balance0Adjusted = balance0.mul(10000).sub(
            (
                balance0 > _reserve0 - amount0Out
                    ? balance0 - (_reserve0 - amount0Out)
                    : 0
            ).mul(isDiscountEligible ? discountFee : regularFee)
        );
        uint balance1Adjusted = balance1.mul(10000).sub(
            (
                balance1 > _reserve1 - amount1Out
                    ? balance1 - (_reserve1 - amount1Out)
                    : 0
            ).mul(isDiscountEligible ? discountFee : regularFee)
        );
        require(
            balance0Adjusted.mul(balance1Adjusted) >=
                uint(_reserve0).mul(_reserve1).mul(10000 ** 2),
            "ZealousSwap: K"
        );
    }
 
    ...

However, the function does not enforce that actualUser is the true caller of the swap. As a result, an attacker could bypass the router and call swap directly, supplying a discount-eligible address as actualUser—even if that address is not the one actually executing the transaction. This allows them to illegitimately benefit from a reduced swap fee.

Consider implementing a whitelist of approved router addresses. If the caller of the swap function is not in the whitelist, enforce that msg.sender must match actualUser. This ensures that only trusted routers can pass a different address as actualUser, preventing misuse of the discount mechanism.

++ mapping(address => bool) public isWhitelistedRouter;
 
function swap(
    uint amount0Out,
    uint amount1Out,
    address to,
    bytes calldata data,
    address actualUser
) external lock {
++  if (!isWhitelistedRouter[msg.sender]) {
++      require(
++          msg.sender == actualUser,
++          "Incorrect actual user"
++      );
++  }
 
    ...
}

In the ZEALInfinityPool contract, the exchange rate between staked Zeal tokens and minted xZeal tokens always increases, using the highestExchangeRate variable as a minimum bound.

function getExchangeRate() public view returns (uint256) {
    uint256 xSupply = xZealToken.totalSupply();
    if (xSupply == 0) {
        return highestExchangeRate; // Return highest rate if no supply
    }
    uint256 currentRate = (totalStaked + totalRewards).mulDiv(
        PRECISION_FACTOR,
        xSupply
    );
 
    return
        currentRate > highestExchangeRate
            ? currentRate
            : highestExchangeRate;
}

function stake(uint256 _amount) external nonReentrant {
    ...
    if (xSupply == 0) {
        currentRate = PRECISION_FACTOR;
    } else {
        currentRate = (totalStaked + totalRewards).mulDiv(
            PRECISION_FACTOR,
            xSupply
        );
    }
 
    currentRate = currentRate > highestExchangeRate
        ? currentRate
        : highestExchangeRate;
    ...

However, even when the current supply is zero, the contract still uses the previous highestExchangeRate as the minimum bound. This allows an attacker to repeatedly stake and unstake when the supply is zero, inflating the exchange rate, especially when rewards are added.

With no cost, the attacker can inflate the exchange rate to an extremely large or even unlimited value, effectively preventing others from staking into the contract (DoS).

Example:

1. Initially, currentRate = highestExchangeRate = PRECISION_FACTOR (1e18). The attacker stakes 1e18 Zeal tokens and receives 1e18 xZeal shares.
2. The attacker then adds 1e18 Zeal tokens as rewards. Now, highestExchangeRate = currentRate = 2e18. The attacker unstakes 1e18 xZeal shares and receives the full 2e18 Zeal tokens, because the exchange rate for unstaking is set to highestExchangeRate when the remaining xZeal supply is zero.
3. Next, the attacker stakes 1e18 Zeal tokens again but receives only 0.5e18 xZeal shares, since the highestExchangeRate is now 2e18. The attacker can then add 1e18 Zeal tokens as rewards and unstake the 0.5e18 xZeal shares, doubling the exchange rate to 4e18.
4. By repeating this process (with just 2e18 Zeal tokens), the attacker can increase the highestExchangeRate to a very large value, making it so that any new Zeal token stakes will receive zero shares — effectively causing a denial-of-service (DoS) on the pool.

Consider reseting highestExchangeRate to its default value whenever the total supply returns to zero.

The contract ZealousSwapPair imports the interface IZealousSwapDiscountManager. However, there is a version mismatch: ZealousSwapPair is written for Solidity 0.5.16, while IZealousSwapDiscountManager requires Solidity version >=0.8.20. This version discrepancy causes a compiler error when attempting to compile ZealousSwapPair.

pragma solidity =0.5.16;

pragma solidity ^0.8.20;

Consider changing the solidity version of the interface IZealousSwapDiscountManager to 0.5.16.

When emissions are paused and later resumed via the setEmissionsPaused(false) function in the ZEALInfinityPool contract, the lastRewardBlock should be updated to the current block number at the time of resumption. Failing to do so results in reward accrual during the paused period, which is incorrect.

Consider the following scenario:

1. At block 100, the contract is configured as follows:

• zealPerBlock = 10
• totalRewards = 1000
• lastRewardBlock = 100
• xZealToken.totalSupply() > 0
• emissionsPaused = false

2. At block 200, the owner calls setEmissionsPaused(true):

• _updateEmissions() is invoked in line 301:
  • blocksSince = 200 - 100 = 100
  • pending = 100 * 10 = 1000
  • totalRewards = 2000
  • lastRewardBlock = 200
• emissionsPaused is set to true in line 302.

3. At block 300, the owner calls setEmissionsPaused(false):

• _updateEmissions() exits early due to emissionsPaused being true, so lastRewardBlock remains at 200.
• emissionsPaused is now set to false.

4. Still at block 300, a user calls stake(), which triggers _updateEmissions():

• blocksSince = 300 - 200 = 100
• pending = 100 * 10 = 1000
• totalRewards = 3000
• lastRewardBlock = 300 The issue is the rewards were incorrectly accrued for the paused period (block 200 to 300), even though emissions were halted. This happens because lastRewardBlock wasn't updated when emissions resumed, causing _updateEmissions() to include the paused period in its reward calculations.

function setEmissionsPaused(bool _paused) external onlyOwner {
    _updateEmissions();
    emissionsPaused = _paused;
    emit EmissionsPaused(_paused);
}

When setEmissionsPaused(false) is called, lastRewardBlock should be explicitly set to the current block number to ensure that no rewards are emitted for the paused interval.

When a user deposits tokens, they can't withdraw them until the locking period has passed. This is enforced in the withdraw function with the following check:

function withdraw(uint256 _pid, uint256 _amount) public nonReentrant {
    -- snip --
    require(
        block.number >= user.lastInteraction + lockingPeriod,
        "ZealousSwapFarms: Tokens are still locked"
    );

However, the emergencyWithdraw function does not have this restriction. This means a user can bypass the locking period and immediately withdraw their tokens by calling emergencyWithdraw.

function emergencyWithdraw(uint256 _pid) public nonReentrant {
    require(_pid < poolInfo.length, "ZealousSwapFarms: Invalid pool ID");
 
    PoolInfo storage pool = poolInfo[_pid];
    UserInfo storage user = userInfo[_pid][msg.sender];
 
    uint256 amount = user.amount;
    require(amount > 0, "ZealousSwapFarms: No LP tokens staked");
 
    user.amount = 0;
    user.rewardDebt = 0;
    pool.totalDeposited = pool.totalDeposited - amount;
    pool.lpToken.safeTransfer(address(msg.sender), amount);
 
    emit EmergencyWithdraw(msg.sender, _pid, amount);
}

To prevent users from bypassing the locking mechanism using emergencyWithdraw, introduce a penalty mechanism. For example, deduct a percentage (5–10%) from the withdrawn amount when emergencyWithdraw is used.

In the ZealousSwapFarms::add() function, if it reactivates an old pool, the lastRewardBlock of that pool is set to the maximum of block.number and the global startBlock. However, if it adds a new pool, the lastRewardBlock is specified via input, and there is no validation to ensure that it is greater than block.number or startBlock.

function add(
    uint256 _allocPoint,
    IERC20 _lpToken,
    bool _withUpdate,
    uint256 _lastRewardBlock
) public onlyOwner {
    ...
 
    uint256 lastRewardBlock = _lastRewardBlock;
    if (_lastRewardBlock == 0) {
        lastRewardBlock = block.number > startBlock
            ? block.number
            : startBlock;
    }
 
    ...

Therefore, a small lastRewardBlock might be set for a new pool, causing rewards to be distributed unfairly, as early liquidity providers would receive most of the rewards. This happens because a large time lapse is counted the first time updatePool is triggered with this small lastRewardBlock value.

The lastRewardBlock of a new pool should be checked after it is set.

For example:

  uint256 lastRewardBlock = _lastRewardBlock;
  if (_lastRewardBlock == 0) {
      lastRewardBlock = block.number > startBlock
          ? block.number
          : startBlock;
  }
+ require(
+     lastRewardBlock >= block.number &&
+     lastRewardBlock >= startBlock,
+     "ZealousSwapFarms: Too small lastRewardBlock"
+ );

The setRewardToken() function in ZealousSwapFarms allows the farm owner to update the reward token. However, this change can be made without first distributing all pending rewards in the previous token. As a result, stakers may unintentionally receive rewards in the new token, potentially leading to incorrect or inflated payouts.

For example:

• Alice has 1,000 USDC in unclaimed rewards (pendingReward(some_pid, Alice) = 1000 USDC).
• The owner calls setRewardToken() to change the reward token from USDC to BTC.
• When Alice later calls claim(some_pid), she receives 1,000 BTC instead of 1,000 USDC, creating a massive unintended profit. This flaw could lead to significant financial discrepancies.

function setRewardToken(IERC20 _newRewardToken) public onlyOwner {
    require(
        address(_newRewardToken) != address(0),
        "ZealousSwapFarms: Invalid token address"
    );
 
    massUpdatePools();
 
    rewardToken = _newRewardToken;
    emit RewardTokenUpdated(address(_newRewardToken));
}

When updating the reward token for a pool, we recommend deploying a new farm contract and instructing users to migrate their stakes to the new contract to prevent reward inconsistencies.

In the ZealousSwapNFTStaking contract, this owner check of the NFT from line 310 to 313 in the stakeNFT() function is redundant:

require(
    IERC721(nftContract).ownerOf(nftID) == msg.sender,
    "ZealousSwapNFTStaking: Not token owner"
);

This is because the function already checks the NFT's owner before and after the transfer, as shown in line 325.

address beforeOwner = IERC721(nftContract).ownerOf(nftID);
IERC721(nftContract).transferFrom(msg.sender, address(this), nftID);
address afterOwner = IERC721(nftContract).ownerOf(nftID);
require(
    afterOwner == address(this) && beforeOwner == msg.sender,
    "ZealousSwapNFTStaking: NFT transfer failed"
);

Remove the redundant check.

Lines 49–53 of the setFeesForAllPairs() function in the ZealousSwapFactory contract perform boundary validation on _regularFee and _discountFee before calling IZealousSwapPair(allPairs[i]).setFees(_regularFee, _discountFee) at line 56. However, the setFees() function in the ZealousSwapPair contract already enforces the same boundary checks (lines 46–50). As a result, these validations are executed twice, leading to unnecessary gas consumption.

A similar redundancy exists in the ZealousSwapFactory.setFeesForSpecificPairs() function as well.

require(_regularFee <= 100, "ZealousSwap: FEE_TOO_HIGH");
require(
    _discountFee <= _regularFee,
    "ZealousSwap: DISCOUNT_HIGHER_THAN_REGULAR"
);

Consider removing the fee boundary check within the function setFeesForAllPairs() and setFeesForSpecificPairs().

function setFeesForAllPairs(uint _regularFee, uint _discountFee) external {
    require(msg.sender == feeToSetter, "ZealousSwap: FORBIDDEN");
--  require(_regularFee <= 100, "ZealousSwap: FEE_TOO_HIGH");
--  require(
--      _discountFee <= _regularFee,
--      "ZealousSwap: DISCOUNT_HIGHER_THAN_REGULAR"
--  );
 
    for (uint i = 0; i < allPairs.length; i++) {
        IZealousSwapPair(allPairs[i]).setFees(_regularFee, _discountFee);
    }
}
 
function setFeesForSpecificPairs(
    uint _regularFee,
    uint _discountFee,
    address[] calldata _pairsToUpdate
) external {
    require(msg.sender == feeToSetter, "ZealousSwap: FORBIDDEN");
--  require(_regularFee <= 100, "ZealousSwap: FEE_TOO_HIGH");
--  require(
--      _discountFee <= _regularFee,
--      "ZealousSwap: DISCOUNT_HIGHER_THAN_REGULAR"
--  );
 
    for (uint i = 0; i < _pairsToUpdate.length; i++) {
        IZealousSwapPair(_pairsToUpdate[i]).setFees(
            _regularFee,
            _discountFee
        );
    }
}