Jump DeFiSmart Router and Farm Security Review

Description

The smart router collects user tokens into the contract before performing a swap or allowing withdrawals. After a swap is performed, check_and_finalize verifies if the user has sufficient balance through:

let input_amount: u128 = amount.parse().unwrap();let user_balances = self    .deposits    .entry(AccountId::from_str(&account_id).unwrap())    .or_insert_with(HashMap::new);let input_balance = *user_balances.entry(input_token_id.clone()).or_insert(0); if input_balance < input_amount {    env::panic_str("Insufficient input balance for the transaction.");}

A critical vulnerability exists where even if a user lacks sufficient input amount, but the contract has accumulated tokens from other users (since the swap uses the current contract account ID), the panic at this point won't revert the DEX-side swap. This allows a malicious actor to swap out any token amount currently held in the contract.

This vulnerability enables:

• Griefing (other users will lose tokens)
• Manipulating token price (swapping large amount of tokens stored in contract to another one)

PoC

Test case to run: contract_token_draining

• Construct SwapAction with any token amount up to the total amount held in the contract
• Call ft_on_transfer with a minimal amount (1)
• Note that while our deposits don't increase due to env::panic_str("Insufficient input balance for the transaction."), the tokens are still swapped on the DEX side
• Check ft_balance of current contract to confirm the balance is reduced or zero

Recommendation

Verify that the user has enough funds inside of ft_on transfer per sent number of tokens and SwapAction. Additionally, consider following the pattern of:

• Do all state updates before cross-contract calls
• Inside of the callback, depending on promise result, revert the changes on DEX side

Remediation

The Jump Defi team has fixed the issue by creating a reentrancy lock, verifying that provided amount matches the swap action amount, and doing the state updates before cross-contract-calls

Description

The smart router collects user tokens into the contract before performing a swap or allowing withdrawals. After a swap is performed, check_and_finalize verifies if the user has sufficient balance through:

let input_amount: u128 = amount.parse().unwrap();let user_balances = self    .deposits    .entry(AccountId::from_str(&account_id).unwrap())    .or_insert_with(HashMap::new);let input_balance = *user_balances.entry(input_token_id.clone()).or_insert(0); if input_balance < input_amount {    env::panic_str("Insufficient input balance for the transaction.");}

Additionally, before a user calls swap, the contract verifies sufficient funds through:

let amount: u128 = input_amount.parse().unwrap();             if *current_balance < amount {                env::panic_str("Not enough amount deposited");            }

However, this verification can be bypassed using a batch call. The contract decreases the balance in the callback, which executes after several blocks, allowing the same amount to be swapped multiple times. Even when a user lacks sufficient input amount, if the contract has accumulated tokens from other users (since the swap uses the current contract account ID), the panic won't revert the DEX-side swap.

This vulnerability enables:

• Griefing (other users will lose tokens)
• Manipulating token price (swapping large amount of tokens stored in contract to another one)

PoC

Test case to run: contract_race_condition_token_draining

• Deposit certain number of tokens through ft_on_transfer
• Construct SwapAction which matches the token amount you deposited
• Construct batch call to the swap function with the same SwapAction
• Note that while our deposits don't increase due to env::panic_str("Insufficient input balance for the transaction."), the tokens are still swapped on the DEX side
• Check ft_balance of current contract to confirm the balance is reduced or zero

Recommendation

Consider following the pattern of:

• Do all state updates before cross-contract calls
• Inside of the callback, depending on promise result, revert the changes on DEX side

Remediation

The Jump Defi team has fixed the issue by creating a reentrancy lock, and doing the state updates before cross-contract-calls.

Description

The smart router allows specifying a withdraw_token inside the swap function. This token is used to check contract balance before and after the swap to verify fund receipt. It then increases the user's balance per token out in the check_and_finalize method:

let final_balance: u128 = match final_balance_result {    Ok(value) => value.parse().unwrap_or(0), // Default to 0 if parsing fails    Err(_) => 0,}; let initial_balance: u128 = initial_balance.parse().unwrap(); if final_balance < initial_balance {    env::panic_str("Final balance is less than initial balance, swap might have failed.");} let tokens_received: u128 = final_balance - initial_balance; if tokens_received == 0 {    env::panic_str("Swap transaction failed, did not receive any funds after swap.");} match env::promise_result(0) {    PromiseResult::Successful(_) => {        let new_balance = user_balances.entry(output_token_id.clone()).or_insert(0);        *new_balance += tokens_received;        ...}

A vulnerability exists because the withdraw_token is not required to match the output token. This allows a malicious actor to use a cheap token to mint deposits of a more valuable token.

This vulnerability enables:

• Draining of funds from the contract of any token. For example, a malicious actor can use a cheap token during a swap of another token to WNEAR, receiving a large WNEAR deposit that they can then withdraw.

PoC

Test case to run: contract_race_condition_token_minting

• Construct SwapAction with any token amount and out token being wrap.near
• Swap with withdraw_token being some other cheap token you hold
• Wait 1kms & transfer cheap token to the router contract
• Note that our wrap.near deposit increase to the number of cheap tokens we sent to the contract
• Now we can call withdraw() to withdraw large number of wrap.near

Recommendation

Verify that token out equals to withdraw_token. Also, make sure to increase deposit balance by receiving the returned value from DEX instead of relying on ft_balance_of.

Remediation

The Jump Defi team has fixed the issue by verifying the withdraw token and relying on the swap response instead of ft_balance_of.

Description

The farm creation process in ft_on_transfer lacks storage fee charges, creating a vulnerability to contract bloating. When creating a farm with a long account ID (64 characters) and a seed ID of u16::MAX - 63953_u16 (staying under log limits), each call consumes approximately 13,206 bytes of storage, costing 0.0129N. A malicious actor could execute 100,000 iterations, forcing the contract to spend/lock 1,290N (approximately $9,030) and potentially trigger a denial of service (DoS).

let terms: HRSimpleFarmTerms = serde_json::from_str(&msg).expect("Invalid msg format for farm creation"); // Ensure the reward token matches the token calling ft_on_transferassert_eq!(&env::predecessor_account_id(),terms.reward_token.as_ref(),    "The reward token must be the same as the token calling ft_on_transfer"); // Use the provided amount as the initial rewardlet min_deposit = MIN_SEED_DEPOSIT; // Create the farmlet farm_id = self.internal_add_farm(&terms, min_deposit);

PoC

Test case to run: storage_bloating_measure

• Call ft_on_transfer with a long reward account (64 characters) and long seed (u16::MAX - 63953_u16)
• Observe logs to get the number of bytes written to the storage

Recommendation

Implement a storage deposit requirement as in create_simple_farmmethod.

Remediation

The Jump Defi team has fixed the issue by asserting the storage inside of the ft_on_transfer.

Description

Users are not charged storage fees in ft_on_transfer, which could lead to contract bloating. Each ft_on_transfer call with a long account ID (64 characters) adds approximately 125 bytes of storage, costing 0.00122N. A malicious actor could execute 100,000 iterations, forcing the contract to spend/lock 122N (approximately $780) and potentially trigger a denial of service (DoS).

This vulnerability also exists in the swap function, where or_insert is used to create default values for users:

let user_balances = self    .deposits    .entry(env::signer_account_id())    .or_insert_with(HashMap::new);let current_balance = user_balances.entry(input_token_id.to_string()).or_insert(0);

PoC

Test case to run: storage_dos_measure

• Call ft_on_transfer with a long account (64 characters)
• Measure storage usage and convert it to NEAR

Recommendation

Implement a storage deposit requirement equal to the storage cost of inserting the longest possible account ID.

Remediation

The Jump Defi team has fixed the issue by implementing the storage deposit function and asserting the storage inside of the ft_on_transfer.

Description

The extend_auto_whitelisted_postfix and remove_auto_whitelisted_postfix functions lack proper access control assertions. The is_owner_or_guardians method returns a boolean but does not prevent execution when the caller is unauthorized.

pub fn extend_auto_whitelisted_postfix(&mut self, postfixes: Vec<String>) {    assert_one_yocto();    self.is_owner_or_guardians();    for postfix in postfixes {        self.auto_whitelisted_postfix.insert(postfix.clone());    }}

pub fn remove_auto_whitelisted_postfix(&mut self, postfixes: Vec<String>) {    assert_one_yocto();    self.is_owner_or_guardians();    for postfix in postfixes {        let exist = self.auto_whitelisted_postfix.remove(&postfix);        assert!(exist, "{}", ERR105_WHITELISTED_POSTFIX_NOT_IN_LIST);    }}

pub(crate) fn is_owner_or_guardians(&self) -> bool {                env::predecessor_account_id() == self.owner_id                    || self.guardians.contains(&env::predecessor_account_id())            }

These methods control whitelisted_postfixes, which creates critical security risks since an attacker can:

• Add malicious tokens to whitelist
• Remove valid tokens from whitelist

Recommendation

Assert the self.is_owner_or_guardians() via:

assert!(self.is_owner_or_guardians(), "{}", ERR100_NOT_ALLOWED);

Remediation

The Jump Defi team has fixed the issue by asserting the check.

Description

Throughout the smart contract, env::signer_account_id is used both in functions and as arguments for cross-contract calls:

#[payable]pub fn swap(&mut self, actions: Vec<SwapAction>, withdraw_token: String) -> Promise {    let input_token_id =        AccountId::from_str(&actions[0].swap_action[0].tokens.first().unwrap()).unwrap();     let input_amount = actions[0].swap_action[0].amounts.first().unwrap();    let user_balances = self        .deposits        .entry(env::signer_account_id())        .or_insert_with(HashMap::new);

    .then(Promise::new(env::current_account_id()).function_call(    "check_and_finalize".to_string(),    json!({        "initial_balance": initial_balance,        "amount": amount,        "input_token_id": input_token_id,        "output_token_id": output_token_id,        "account_id": env::signer_account_id().to_string()    })    .to_string()    .into_bytes(),    NearToken::from_near(0)

While the signer account ID matches the original signer in cross-contract call chains, this creates a potential phishing vulnerability. A malicious actor could trick users into signing a chain of cross-contract calls targeting the smart contract, potentially stealing user funds or accessing privileged functions if the user has admin rights.

Recommendation

Use env::predecessor_account_id and pass the original caller within the arguments for callbacks or cross-chain calls instead of relying on env::signer_account_id.

Example:

#[private]pub fn execute_swap(    &mut self,    #[callback_result] initial_balance_result: Result<String, near_sdk::PromiseError>,    msg: String,    original_caller: AccountId,  ...) -> Promise {...    .then(        Promise::new(env::current_account_id()).function_call(            "check_and_finalize".to_string(),            json!({                "initial_balance": initial_balance,                "amount": amount,                "input_token_id": input_token_id,                "output_token_id": output_token_id,                "account_id": original_caller.to_string()            })            .to_string()            .into_bytes(),            NearToken::from_near(0)

Remediation

The Jump Defi team has fixed the issue by utilizing predecessor account id and passing the caller account inside of the argument.

Description

The get_deposits function does not use JSON-compatible types for its return value. It uses a plain u128 as the value in the HashMap, when it should instead use U128. Also in InnerSwapAction, the pool_id uses u64 instead of U64.

To preserve interoperability, JSON has a double floating point precision limit. Since double floating-point precision has only 52 bits to represent the mantissa (significant digits during conversion), values must be in the range of [-(2^53)+1, (2^53)-1], with a maximum safe integer of 9007199254740991. Using values larger than 9007199254740991 can cause issues such as:

• 9007199254740991 + 1 == 9007199254740991 + 2
• 90071992547409911111 => 90071992547409900000** (after parsing)
• 77777777777777777777 => 77777777777777770000 (after parsing)

These issues can lead to severe logical and calculation bugs.

Recommendation

To handle numbers larger than 2^53-1, input and output data should use string format instead. The NEAR rust SDK provides custom JSON types like U64 and U128: near_sdk::json_types - Rust. Alternatively, you can reduce the type to u32 or below, which would work for pool_id.

Remediation

The Jump Defi team has fixed the issue by utilizing the correct types.

Description

The smart router and farm contracts lack comprehensive test coverage. While some test scripts exist, they don't cover the entire codebase. The farm smart contract, which was forked and modified, is missing its original test suite.

Recommendation

The smart router requires both unit tests and integration tests using near-workspaces. For the farm contract, the original test suite should be forked. When modifying the forked farm contract, verification must include:

• Running the complete forked test suite
• Confirming that any test failures are directly related to the modifications and checking for unintended effects on the system
• Addressing test failures related to the modifications, and if systemic issues are discovered, conducting additional verification and implementing necessary fixes

Remediation

The Jump Defi team has acknowledged the issue and will fix it later.

Description

The smart router contract lacks prepaid gas validation before making cross-contract calls. This creates a risk where there may be sufficient gas to execute the initial cross-contract call but not enough for its callback.

Recommendation

Calculate and verify available gas against the worst-case scenario before each cross-contract call by subtracting used gas from prepaid gas.

Remediation

The Jump Defi team has fixed the issue by asserting the gas usage.

Description

The smart router contract is missing a callback after the ft_transfer in the withdraw function. The user's balance is decreased before the cross-contract call, which means if the transfer fails, the user cannot withdraw their funds again.

if *current_balance < amount.0 {    env::panic_str("Not enough amount deposited for withdraw");} Promise::new(AccountId::from_str(token_id.as_str()).unwrap()).function_call(    "ft_transfer".to_string(),    json!({"receiver_id": env::signer_account_id(), "amount": amount.to_string(), "msg": ""}).to_string().into_bytes(),    NearToken::from_yoctonear(1),    Gas::from_tgas(35),);

Recommendation

Implement a callback for the ft_transfer call to revert the balance decrease if the transfer fails.

Remediation

The Jump Defi team has fixed the issue by adding the callback.

Description

The contract uses a nested HashMap rather than utilizing LookupMap or IterableMap from the NEAR SDK store collections.

    pub struct DepositTracking {    deposits: HashMap<AccountId, HashMap<String, Balance>>,    fee: u32,    admin: AccountId,}

Native collections are not optimized for handling large amounts of data. As a native collection grows, deserializing it from memory becomes increasingly gas-intensive. If the collection becomes too large, the contract may consume all available gas while attempting to read its state, causing all function calls to fail.

Recommendation

Consider utilizing collections from the near_sdk::store.

Remediation

The Jump Defi team has fixed the issue by utilizing the collections from the near_sdk::store.

Description

The reset_tokens function is considered as test-only, but lacks the#[cfg(test)] directive. Without this directive, the function will be included in the final WASM binary, unnecessarily increasing the binary size and expanding the attack surface.

#[private]pub fn reset_tokens(&mut self, account_id: String) {    // Ensure the caller is the admin    assert_eq!(        env::signer_account_id(),        self.admin,        "Only the admin can reset tokens."    );     let account = AccountId::from_str(&account_id).expect("Invalid account ID format");    ...

Recommendation

Consider either removing the test function or marking it with#[cfg(test)].

Remediation

The Jump Defi team has fixed the issue by marking the function with #[cfg(test)] .

Description

We noticed that the initializer in the smart contract uses a redundant state check - assert! (!env::state_exists(),"The contract has already been initialized");

#[init]#[payable]pub fn new() -> Self {    assert!(        !env::state_exists(),        "The contract has already been initialized"    );    ...}

Recommendation

Consider removing the state check, as the #[init] macro already handles it.

Remediation

The Jump Defi team has fixed the issue by removing the redundant state check.

Description

The CombinedAction struct uses optional (Option) types for its actions and withdraw fields. However, these fields are required and are directly unwrapped throughout the codebase.

pub struct CombinedAction {    withdraw: Option<WithdrawAction>,    swap_actions: Option<Vec<SwapAction>>,}

Recommendation

Consider removing the optional types to make the code's intent more explicit.

Remediation

The Jump Defi team has fixed the issue by removing redundant optional types.

Description

We noticed that the crate type is set as both cdylib and rlib. However, since NEAR smart contracts are compiled to WASM, rlib is unnecessary. Eliminating rlib can significantly reduce the size of the generated WASM binary.

[package]name = "jump_smart_router"version = "0.1.0"edition = "2018" [dependencies]near-sdk = "5.1.0"borsh = "0.9.1"serde = { version = "1.0", features = ["derive"] } [lib]crate-type = ["cdylib", "rlib"] [profile.release]codegen-units = 1# s = optimize for binary size ("z" would additionally turn off loop vectorization)opt-level = "z"...

Recommendation

Consider removing rlib from Cargo.toml.

Remediation

The Jump Defi team has fixed the issue by removing the rlib.

Description

We noticed that in withdraw_tokens if current balance is zero contract still does the cross-contract call. It is redundant and wastes resources

if *current_balance > 0 {    *current_balance -= amount;} Promise::new(AccountId::from_str(token_id.as_str()).unwrap()).function_call(    "ft_transfer".to_string(),    json!({"receiver_id": env::signer_account_id(), "amount": amount.to_string(), "msg": ""}).to_string().into_bytes(),    NearToken::from_yoctonear(1),    Gas::from_tgas(35),);

Recommendation

Consider rewriting the logic to be something like:

#[payable]pub fn withdraw_tokens(    &mut self,    token_id: String,    amount: Option<U128>,) -> PromiseOrValue<U128> {    let user_balances = self        .deposits        .entry(env::predecessor_account_id())        .or_insert_with(HashMap::new);     let current_balance = user_balances.entry(token_id.to_string()).or_insert(0);     if *current_balance == 0 {        env::panic_str("nothing to withdraw");    }     let amount = amount.unwrap_or(U128(*current_balance));     if *current_balance < amount.0 {        env::panic_str("Not enough amount deposited for withdraw");    }     Promise::new(AccountId::from_str(token_id.as_str()).unwrap()).function_call(        "ft_transfer".to_string(),        json!({            "receiver_id": env::predecessor_account_id(),            "amount": amount.to_string(),            "msg": ""        }).to_string().into_bytes(),        NearToken::from_yoctonear(1),        Gas::from_tgas(35),    );...

Remediation

The Jump Defi team has fixed the issue by rewriting the logic.

Description

The contract uses hardcoded contract accounts for external contract interactions. This creates maintenance issues since any change to these contract addresses would require redeploying the entire contract.

pub static REF_EXCHANGE: &str = "v2.ref-finance.near"; pub static REF_EXCHANGE_TEST: &str = "ref-finance-101.testnet"; pub static JUMP_EXCHANGE: &str = "pools.jumpfinance.near"; pub static JUMP_EXCHANGE_TEST: &str = "jump_amm.testnet"; pub static VEAX_EXCHANGE: &str = "veax.near"; pub static VEAX_EXCHANGE_TEST: &str = "veax-dex15.testnet";

Recommendation

Contract accounts should be set through the initializer during deployment, with setter functions implemented to allow updates when needed.

Remediation

The Jump Defi team has fixed the issue by removing the constants file