EVM Stellar zkCrossDex - ZKCross

The swap and lock functions use payable(...).transfer(amount) to send native assets (ETH). The .transfer() method forwards a fixed gas stipend of 2300, which is often insufficient for smart contract recipients that have a receive() or fallback() function with logic that consumes more gas. If a user or the bridgeAdmin is a smart contract (e.g., a multi-sig wallet), transactions to it could consistently fail, effectively locking funds for those recipients.

The release function correctly uses the recommended .call{value: amount}("") pattern, highlighting an inconsistency in the codebase.

Use call pattern with success check:

(bool ok, ) = payable(recipient).call{value: amount}("");
require(ok, "ETH send failed");

SOLVED: The zkCross team solved this issue in the specified commit ID.

The primary swap function lacks a slippage protection mechanism. Unlike the lock and release functions, which accept a minAmountOut parameter, swap will execute regardless of the final output amount. This exposes users to significant financial risk from MEV attacks (like sandwich attacks) or high price volatility, where they could receive far fewer tokens than expected.

Add a minAmountOut parameter to swap and enforce swapOutput >= minAmountOut.

SOLVED: The zkCross team solved this issue in the specified commit ID.

The contract's logic assumes that transferring X tokens results in the contract receiving exactly X tokens. This assumption does not hold for fee-on-transfer tokens, which deduct a fee from the transfer amount. The contract transfers the nominal amount from the user but then approves the allowanceHolder for that same amount. If the contract received less due to a fee, the subsequent swap call from the allowanceHolder may fail because the contract's actual token balance is lower than the amount it attempts to spend.

Instead of approving the nominal amount, the contract should measure its balance of the source token before and after the transfer, and then approve the allowanceHolder for the actual amount received.

SOLVED: The zkCross team solved this issue in the specified commit ID.

The contract inherits OwnableUpgradeable, which uses a single-step transferOwnership(newOwner) flow. Single-step transfers can permanently cede control if the wrong address is provided (e.g., a mistyped EOA, an EOA that cannot accept ownership due to operational constraints, or an address that is later proven compromised). A two-step pattern reduces this risk by requiring the new owner to explicitly accept ownership before the transfer completes.

Migrate to Ownable2StepUpgradeable (OpenZeppelin) to require explicit acceptance: transferOwnership(pendingOwner) then acceptOwnership().

SOLVED: The zkCross team solved this issue in the specified commit ID.

After lock, the full swapOutput is transmitted to the bridge admin immediately. If the cross-chain operation fails or stalls, users lack an on-chain mechanism to recover their funds. The current model depends entirely on off-chain processes and administrator discretion for refunds.

This situation can leave user funds permanently inaccessible if the relayer or destination chain encounters an issue, potentially undermining trust and leading to increased support and liability challenges for operators.

Introduce a pending state with an associated expiry: record lockHash -> pending(amount, user, expiry). Require the bridge administrator to release the pending transaction before expiry. If not released in time, the user may invoke refund(lockHash) to recover the funds from the contract balance. Alternatively, implement an explicit refund function for bridge administrators, accompanied by auditable events.

SOLVED: The zkCross team solved the issue in the following commit IDs:

• 1aaaf2c

• 0815688

A new claims layer has been introduced, and it incorporates configurable deadlines, explicit administrator acknowledgements, and administrator-initiated ERC20 refunds. Corresponding events and comprehensive error handling have been added. The setBridgeLiquidityToken function has been modified to ensure that the bridge liquidity token ( intended to be USDC) can never be set equal to the native token.

The team provided the following statements:

We have designed our approach to avoid locking funds in the contract. We've implemented a proper token release mechanism where, in case of failures, users can submit support tickets for refunds. These refunds will be processed from a separate liquidity address maintained by the platform's owner. The owner who has deposited liquidity will periodically withdraw corresponding funds from TEE administrators based on the total volume of processed tickets, ensuring proper fund reconciliation.

On the EVM side, we use the 0x swapper to always convert any input token to the official USDC address for that particular EVM chain during the lock function. The funds are then transferred to the TEE liquidity address.

In cases where the release fails and the user submits a support ticket, we will provide refunds exclusively in USDC for the equivalent amount that was initially locked.

Because of the final statement in the provided response, it was highlighted that the previous implementation did not swap native tokens locked by users to USDC, which would have broken the refund mechanism. An appropriate fix was therefore added.

The contract is UUPS-upgradeable and inherits from several OpenZeppelin upgradeable base contracts (OwnableUpgradeable, PausableUpgradeable, ReentrancyGuardUpgradeable, UUPSUpgradeable). These base contracts already include internal __gap variables to safeguard their own future state modifications. The Swapper implementation itself does not declare a __gap, which does not constitute a vulnerability by itself. However, the primary risks stem from reordering or removing existing state variables, altering inheritance order, or introducing new parent contracts with storage layouts that precede existing storage—any of which can corrupt the contract's state during upgrades if not properly validated.

Add a storage gap at the end of Swapper to reserve slots for future variables, e.g., uint256[50] private __gap;. If new variables are added later, append them before the gap and reduce the gap size accordingly.

SOLVED: The zkCross team solved this issue in the specified commit ID.

The contract contains several typographical errors within comments and function names. While these do not impact the contract's logic, they may hinder readability and long-term maintainability.

The identified issues are listed below:

• Line 93: warpped should be wrapped in the function comment.

• Line 111: trasfered should be transferred in the function comment.

• Line 149: briding should be bridging in the function comment.

• Line 167: dstination should be destination in the require statement.

• Line 341: The function setallowanceHolder should be named setAllowanceHolder to follow Solidity's camelCase naming conventions.

Correct the identified typos and inconsistencies to improve code quality and clarity.

SOLVED: The zkCross team solved this issue in the specified commit ID.

The lock and release flows can reach the low‑level _swap without first verifying that allowanceHolder is set to a valid non‑zero address. The issue is that a zero or misconfigured address will lead to opaque low‑level call failures. The impact is preventable configuration‑time failures and harder debugging during deployment and operations; a simple non‑zero check improves robustness.

Ensure that the allowanceHolder is no the zero address before invoking _swap within the lock and release functions.

SOLVED: The zkCross team solved this issue in the specified commit ID.

The setallowanceHolder function enables the owner to designate the address of the exchange or aggregator contract responsible for performing token swaps. However, it does not validate whether the provided address is a smart contract. If the owner mistakenly sets allowanceHolder to an Externally Owned Account (EOA), the core swapping functionality will be disrupted. The external call within the _swap function (allowanceHolder.call{...}) will still return success = true, but no swap will be executed. As a result, the swapOutput will be zero, causing a revert with the message "Swap output was zero". Subsequently, all further swap and lock operations will fail.

Validation of allowanceHolder should be enforced by requiring that the provided address contains contract code (e.g., require(_addr.code.length > 0)) within setAllowanceHolder, ensuring that only contract addresses can be assigned and preventing externally owned accounts (EOAs) from disabling swap functionality.

SOLVED: The zkCross team solved this issue by emitting an AllowanceHolderSetToEOA warning event instead of reverting, allowing the transaction to proceed while signaling a potentially unintended configuration.

The withdrawTokens function allows the contract owner to withdraw assets; however, it transfers the withdrawn tokens to the swapAdmin address. Importantly, it fails to verify that the swapAdmin address is non-zero before executing the transfer. If the swapAdmin address is mistakenly set during contract initialization as address(0), any attempt to withdraw the native asset (ETH) will result in the funds being irretrievably burned, as transfers directed to the zero address cannot be recovered.

Add a require statement at the beginning of the withdrawTokens function to verify that swapAdmin is not the zero address (address(0)). Additionally, it is recommended to validate during initialization (within the initialize function) that the provided swap admin address is not the zero address. This ensures that the admin address is properly set and prevents potential security issues.

SOLVED: The zkCross team solved this issue in the specified commit ID.

Functions like setBridgeLiquidityToken and setBridgeAdmin lack events. This reduces transparency for off-chain monitoring.

It is recommended to implement the missing event emissions within the corresponding functions to ensure comprehensive event tracking and system transparency.

SOLVED: The zkCross team solved this issue in the specified commit ID.

When transferring ERC20 tokens to the bridge administrator after a lock, the code employs the raw IERC20.transfer function. This approach is problematic because non-standard tokens may revert or return false inconsistently, leading to unreliable transfer behavior. Relying solely on transfer can cause failed or unsafe transfers with such tokens. Using SafeERC20.safeTransfer consistently ensures uniform handling and guarantees a revert on failure, enhancing robustness across different token implementations.

It is recommended to use SafeERC20.safeTransfer consistently.

SOLVED: The zkCross team solved this issue in the specified commit ID.

The _approve helper sets approval only when the current allowance is insufficient and does not revoke allowances after a swap. This pattern is commonly employed for gas efficiency when the allowanceHolder is trusted. However, it results in residual approvals that can persist over time and through changes in spender authorization.

If tokens are later transferred to the contract—whether through accidental transfers, airdrops, misrouts, partial integrations, or failed transactions—the approved spender can potentially withdraw up to the remaining allowance. When the allowanceHolder is updated via setAllowanceHolder, existing approvals granted to the previous spender remain valid and are not automatically revoked by the contract.

Additionally, the documentation states that the allowance is "automatically reset after the swap," which does not reflect the actual implementation behavior.

Code Location

Code of the _approve helper function:

function _approve(
        address _token,
        address _spender,
        uint256 _amount
    ) private {
        uint256 allowance = IERC20(_token).allowance(address(this), _spender);
        if (allowance < _amount) {
            IERC20(_token).forceApprove(_spender, _amount);
        }
    }

It is recommended to approve per‑swap exact amount based on the actual tokens received, then forceApprove(spender, 0) after the swap completes.

SOLVED: The zkCross team solved this issue by approving the exact amount based on the actual tokens received per-swap and by introducing a revokeApproval function. Access to this function is restricted to the contract owner, and it revokes all approvals for a specified token and spender.

Replay protection for releases is identified by a string. However, using strings is gas‑intensive and susceptible to encoding variations for the same logical identifier, which makes equality checks unreliable and more costly. This results in unnecessary gas overhead and introduces a subtle risk that duplicate logical lock identifiers may bypass the replay guard if serialized differently; employing bytes32 eliminates these issues.

Use bytes32 for lock hashes (e.g., mapping(bytes32 => bool)) and accept a bytes32 _lockHash parameter in the release function.

SOLVED: The zkCross team solved this issue in the specified commit ID.

Some declared elements are unused, such as the InsufficientSwapOutput error and SwapFeeUpdated event. This represents dead code that can mislead integrators or suggest missing logic. The impact includes informational noise and potential confusion during audits and integrations. Unused declarations should be removed or properly implemented.

Remove or implement the associated logic.

SOLVED: The zkCross team solved this issue in the following commit IDs:

• 268595f

• 0815688

The Swapper contract currently specifies a floating pragma of ^0.8.28, which allows it to be compiled with any compiler version greater than or equal to 0.8.28 and less than 0.9.0.

It is recommended that contracts be deployed using the same compiler version and flags as during development and testing. Locking the pragma helps ensure that contracts are not accidentally compiled with an incompatible compiler version. For example, an outdated pragma may introduce bugs that negatively impact the security and functionality of the contract system.

It is recommended to lock the pragma version to match the version used during development and testing.

SOLVED: The zkCross team solved this issue through pinning the Solidity compiler to 0.8.28.

The contract frequently employs require(condition, "message"), which results in higher gas costs compared to custom errors and offers less structure for downstream tooling. Although SlippageExceeded is defined and utilized, most other checks still rely on string messages. Converting these repeated guards to custom errors reduces bytecode size and runtime gas consumption, while also enhancing the ability to decode failure reasons programmatically.

Define custom error types for recurring checks (e.g., error NotAdmin();). Replace string-based require statements with conditional checks that revert using the defined custom errors (e.g., if (!condition) revert CustomError();). Keep parameterized errors (such., SlippageExceeded) where they provide valuable debugging information. This approach results in lower gas costs and more structured revert reasons, while maintaining the original semantics.

SOLVED: The zkCross team solved this issue in the specified commit ID.

The _decode function already enforces input validity, including non-zero token addresses, positive amounts, and non-empty swap data. However, several other functions perform identical checks, which results in increased bytecode size and higher runtime gas costs without enhancing safety. Business-rule validations that vary between functions—such as srcToken != dstToken in swap or permitting equality in lock—are appropriate and should be retained.

Remove duplicate validations that are already handled by _decode—such as amount being greater than zero, token addresses being non-zero, and data being non-empty—in the functions swap, lock, release, and _swap. Maintain the specific business rules for each function: for instance, swap should ensure that srcToken != dstToken, whereas lock may intentionally allow equality to bypass swapping.

SOLVED: The zkCross team solved this issue in the specified commit ID.

Executing the repository’s documented command npx hardhat test resulted in test failures locally. While these do not constitute a production vulnerability, failing tests indicate discrepancies between the code and specifications, fragile assumptions within the tests (such as external integrations or fork state), or incomplete local configuration. This situation diminishes confidence in the expected behavior of the deployed software and hampers future refactoring or security reviews.

Below is a screenshot illustrating the test failures:

It is recommended to address the identified failing tests to ensure system integrity and reliability.

SOLVED: The zkCross team solved this issue in the specified commit ID.