Stellar AMM - Normal Finance

The sanitize_new_price() helper in the Oracle Registry computes the percentage change between a freshly reported price (new_price) and the stored time-weighted average price (last_twap) using unsigned subtraction.

Whenever the market dips (i.e. new_price < last_twap), this subtraction underflows, panics inside safe_sub(), and reverts every transaction that depends on a fresh price, effectively locking all swaps, deposits, withdrawals, rebalances, and insurance claims.

// contracts/oracle_registry/src/oracle.rs   (function sanitize_new_price)

pub fn sanitize_new_price(
    e: &Env,
    new_price: u128,
    last_twap: u128,
    clamp_denominator: i64
) -> u128 {
    /* … */
    let price_change = (new_price as u64)
        .safe_sub(e, last_twap as u64);   // ❌ under-flows when new_price < last_twap

    // further maths on price_change …
}

#[cfg(test)]
mod tests {
    use soroban_sdk::Env;
    use utils::math::pool::sanitize_new_price;

    #[test]
    #[should_panic(expected = "Error(Contract, #511)")]
    fn oracle_panics_on_price_down_tick() {
        let e = Env::default();

        let last_twap = 1_000_000_u128;  // 1.00
        let new_price = 980_000_u128;    // 0.98 (price went down)

        // sanitize_clamp_denominator = 100 (any non-zero value works)
        // This will call new_price.safe_sub(last_twap) which under-flows and panics
        let _ = sanitize_new_price(&e, new_price, last_twap, 100);
    }
} 

It is recommended that signed arithmetic be adopted to allow negative deltas without underflow. For example:

let price_change = (new_price as i128) - (last_twap as i128);

Alternatively, if unsigned arithmetic must be retained, it is advised that the operand order be reversed so that the minuend is always greater than or equal to the subtrahend:

let price_change = (last_twap as u64)
    .safe_sub(e, new_price as u64);

• It is suggested that unit and regression tests be added to cover both upward and downward price moves, ensuring that sanitize_new_price() never panics.

• It is further recommended that all other uses of safe_sub() on unsigned integers be audited to confirm that the first operand can never be smaller than the second.

• It is encouraged that explicit pre-condition checks or require! statements be introduced to validate expected price relationships before performing subtraction.

SOLVED: The Normal Finance team solved this issue by updating all contracts to handle spread direction calculations using unsigned values, removing potential sign-related logic errors.

Additionally, they implemented saturating subtraction across all contracts and related assertions to prevent underflow conditions when subtracting values.

These changes ensure more predictable math behavior, eliminate the risk of negative value propagation, and improve overall contract safety and consistency.

The pool rebalances using oracle prices and may mint synthetic Token A to re-peg. However, deposits always mint LP shares at a fixed 1:1 rate with Token B and withdrawals pay Token B 1:1. After a rebalance that increases Token A (NAV↑), NAV per share exceeds 1 B, yet new LPs still mint shares priced at 1 B per share. This lets entrants acquire an underpriced claim on the pool, diluting incumbents.

• Fixed 1:1 share minting on deposit() (ignores NAV/share):

// Now calculate how many new pool shares to mint
let total_shares = get_total_lp_tokens(&e);
let shares_to_mint = token_b_amount;

mint_lp_tokens(&e, &user, shares_to_mint as i128);

• B-only withdrawal at 1:1 per share:

// Transfer any remaining to the user
transfer_b(&e, &user, share_amount);
set_reserve_b(&e, &(reserve_b - share_amount));

• Rebalance can mint synthetic A (NAV increases):

if delta_a > 0 {
    mint_synthetic_tokens(&e, &e.current_contract_address(), delta_a);
    set_reserve_a(&e, &(reserve_a + (delta_a as u128)));
}

• Share pricing ignores NAV: Shares mint strictly 1:1 with Token B regardless of pool value, while rebalance can mint/burn A and change NAV/share.

• Withdrawal policy: B-only redemption at 1:1 per share preserves the underpricing gap for entrants and socializes the cost onto incumbents over time.

Exploit Scenario

1. An LP deposits B. The pool rebalances (pre-deposit) and may mint A over time to track the oracle peg, increasing NAV/share above 1 B.

2. An attacker waits for a rebalance that mints A (NAV/share > 1 B).

3. Attacker deposits B and receives shares at 1 B per share (under fair value).

4. The attacker now holds an over-sized claim on pool value (including A), diluting existing LPs. Withdrawals are B-only 1:1, but the unfair entry pricing transfers value over time via fee flows and subsequent pool dynamics.

Impact

• Enables economic extraction without any privileged access.

• Scales with the size/frequency of synthetic mints.

• Encourages timing attacks (MEV) around rebalances and oracle movements.

To mitigate this issue, the following improvements are recommended:

• Correct share pricing to NAV:

  • Mint shares based on deposit value at current NAV/share.

  • Example: shares_to_mint = deposit_value_in_B / share_price, where share_price = NAV_in_B / total_shares and NAV_in_B = reserve_b + reserve_a × peg.

• Withdrawals:

  • Pay pro‑rata in A and B, or convert A->B internally at current peg (or internal invariant) so the B redemption reflects NAV.

• Safeguards:

  • Add unit/integration tests around rebalance -> deposit sequencing to ensure NAV/share invariants hold.

  • Consider oracle TWAP/rate-limits for large deltas; add circuit-breakers for abnormal conditions.

SOLVED: The Normal Finance team solved this issue by adding validation logic to the share calculation process to ensure that after synthetic Token A minting, new depositors cannot receive a disproportionate number of shares compared to their actual contribution.

This prevents potential value extraction by verifying that minted shares accurately reflect the depositor’s real share of the pool’s total value, safeguarding against exploitation of synthetic minting events.

An attacker can front-run the two public, unauthenticated steps that configure a pool’s reward rate fill_liquidity(asset) and config_pool_rewards(asset) to lock any new pool at 0 tokens per second (TPS). Once processed, the pool’s liquidity and reward rate cannot be updated, permanently starving LPs of emissions.

The PoolRouter contract distributes rewards in a two-step sequence:

1. fill_liquidity(asset)

   • Reads the pool’s on-chain liquidity (initially zero for a fresh pool).

   • Sets processed = true and stores total_liquidity.

   
   

2. config_pool_rewards(asset)

   • Verifies processed == true.

   • Computes pool_tps based on the recorded total_liquidity.

   • Calls the pool’s set_incentives_config() with the calculated TPS.

   
   

Both functions are public and lack any access control:

// contracts/pool_router/src/contract.rs

fn fill_liquidity(e: Env, asset: Symbol) {
    // Public, no require_auth
    let mut data = tokens_with_liquidity.get(asset).unwrap();
    if data.processed {
        panic_with_error!(…LiquidityAlreadyFilled)
    }
    data.processed = true;                 // irreversible
    data.total_liquidity = total_liquidity; // zero if no deposits yet
    tokens_with_liquidity.set(asset, data);
}

fn config_pool_rewards(e: Env, asset: Symbol) -> u128 {
    // Public, no require_auth
    if !reward_info.processed {
        panic_with_error!(…LiquidityNotFilled)
    }
    let pool_tps = if reward_info.total_liquidity > 0 {
        /* compute based on liquidity */
    } else {
        0  // locked at zero
    };
    /* set incentives on the pool */
}

To mitigate this issue, following steps are recommended:

1- Access-control

• Gate fill_liquidity() and config_pool_rewards() with require_auth(rewards_admin) or

• Restrict them to the pool contract itself (called via a trusted callback).

2- Replay / sanity checks

• Reject fill_liquidity() when total_liquidity == 0.

• Allow an admin-only “refill” that overwrites liquidity and recalculates TPS.

SOLVED: The Normal Finance team solved this issue by restricting access to fill_liquidity() and config_pool_rewards() so they can only be executed by the rewards_admin, preventing unauthorized calls.

They also added a check to reject fill_liquidity() when total_liquidity == 0, ensuring the function cannot be called in an uninitialized or empty pool state, which mitigates potential misconfigurations or abuse.

The Pool contract allows the LP-token total supply to reach zero:

• deposit() mints LP tokens 1-for-1 with the Token-B amount;

• withdraw() burns the same amount, enabling the last LP to empty the pool.

• When the pool is empty, an attacker can:

1. Supply a dust-sized deposit (e.g., 1 Token B) and thereby own 100% of the LP supply.

2. Any swaps executed before other LPs join pay their entire fee to that attacker (fees are credited pro-rata to LP balances).

3. The attacker burns their single LP token, withdrawing the dust plus all accrued fees, resetting totalLP back to zero.

4. This maneuver can be repeated whenever the pool is emptied.

   
   Price manipulation is prevented by the contract’s oracle-pegged rebalance(), so the impact is limited to economic fee leakage and potential griefing, rather than direct theft of user funds.

To address this issue it is recommended to implement the industry-standard “minimum-liquidity burn” on the first deposit:

// inside deposit() just before minting to the user

const MIN_LIQUIDITY: u128 = 1_000;          // any small constant

let mut shares_to_mint = token_b_amount;

if get_total_lp_tokens(&e) == 0 {

    // lock MIN_LIQUIDITY LP tokens forever

    mint_lp_tokens(

        &e,

        &Address::from_contract_id(&e, &BytesN::from_array(&e, &[0; 32])), // burn address

        MIN_LIQUIDITY as i128,

    );

    shares_to_mint = shares_to_mint.saturating_sub(MIN_LIQUIDITY);

}

mint_lp_tokens(&e, &user, shares_to_mint as i128);

Optional hardening:

• Reject withdrawals that would push Token-B reserves below the value represented by MIN_LIQUIDITY.

• Emit an event logging the permanently locked liquidity for transparency.

SOLVED: The Normal Finance team successfuly remediated this issue with the following improvements:

• Automated minimum liquidity recovery: Modified set_status function to automatically trigger minimum liquidity
  recovery when a pool is set to Delisted status

• Streamlined workflow: Pool delisting now automatically recovers locked minimum liquidity tokens and transfers the
  underlying Token B amount to the admin who initiated the delisting

• Minimum balance validation - Added validation to always make sure the pool has atleast the funds to send back to the admin

When cached == true or oracle.frozen == true, get_price immediately returns:

OraclePriceData {
    price: historical.last_oracle_price_twap,
    delay: historical.last_oracle_delay,   // ← static snapshot
}

last_oracle_delay is updated only inside update_twap, which never runs in this branch. As soon as the oracle is frozen (or callers routinely request cached data) the stored delay stops changing.

Any downstream code that checks:

delay <= guard.seconds_before_stale_for_pool

will always pass, even if the underlying quote is hours or days old. Pools can therefore trade on stale prices, exposing LPs and traders to manipulation or bad-debt risk.

To remediate this issue we recommend the following corrective action:

if cached || oracle.frozen {
    let now   = e.ledger().timestamp();
    let delay = now - historical.last_oracle_price_twap_ts;  // recompute in real time

    return OraclePriceData { price: historical.last_oracle_price_twap, delay };
}

Alternatively, remove last_oracle_delay from storage entirely and always derive it from the stored timestamp.

SOLVED: The Normal Finance team successfuly remediated this issue by computing delay instead of using a stored one and temporal type (delay) implementation.

After the pool is initialized, deposit() accepts calls with token_b_amount = 0. The function still:

1. Executes a 0 amount transfer_token (cross-contract call).

2. Queries both oracles and runs rebalance().

3. Emits deposit_liquidity and rebalance events with zero values.

This wastes gas for the caller and lets anyone spam on-chain logs and oracle calls, potentially throttling genuine activity.

To mitigate this issue it is recommended to add a check at the top of deposit():

if token_b_amount == 0 {
    panic_with_error!(&e, PoolValidationError::ZeroAmount);
}

SOLVED: The Normal Finance team successfuly remediated this issue by adding a non-zero check.

InsuranceFund::deposit(user, amount) does not check that amount > 0.

When amount == 0 the function:

1. Skips the token transfer.

1. Mints 0 IF-shares and leaves total_shares unchanged.

2. Still executes all accounting code and emits an if_stake_record event.<

Impact

• No stake inflation: percentage ownership and future interest remain unchanged.

• Wastes gas and clutters event logs; a bot could spam zero-stakes at very low cost.

It is recommended to add a check to ensure amount > 0.

SOLVED: The Normal Finance team successfuly remediated this issue by implementing a non-zero check.

set_oracle_price attempts to rate-limit admin overrides:

if now - oracle.last_updated <= guard.seconds_before_stale_for_pool {
    panic!(PriceOverrideTooSoon);
}

However, the function never updates oracle.last_updated after a successful override. Once the first override has passed the threshold, every subsequent ledger tick also passes, allowing an authorized account to push a new price each block.

While only privileged roles can exploit this, it defeats the intended operational safety net.

To mitigate this issue we recommend the following changes

• After a successful override, set oracle.last_updated = now and persist the modified OracleInfo with put_oracle.

• Alternatively, introduce a dedicated last_override_ts field and compare now - last_override_ts against the cooldown.

SOLVED: The Normal Finance team successfuly remediated this issue by updating lst_updated to the current time in function.

commit_transfer_ownership does not forbid future_address == current_address.

Admin can inadvertently trigger the three-day delay but end up with the same owner after apply, causing needless downtime.

It is recommended to reject identical addresses or treat it as a no-op.

SOLVED: The Normal Finance team successfuly remediated this issue by adding a check.

Admins can supply any i64 value including 0 or negative numbers for sanitize_clamp_denominator. If update_twap divides by this field, a zero triggers a panic that bricks price updates; a negative value may yield signed arithmetic anomalies and corrupt TWAP.

To mitigate this issue we recommend the following hard-validation in both registration and update paths:

require!(sanitize_clamp_denominator > 0, OracleRegistryError::InvalidClampDenominator);

You can also consider capping it to a reasonable upper bound to avoid accidental overflow in downstream math.

SOLVED: The Normal Finance team successfuly remediated this issue by checking util for negative denominators - avoids twap clamps and validating util in sync with prev type changes.

When the pool’s status is set to ReduceOnly, the swap logic blocks direct Token B → Token A buys, but rebalance() can still mint any positive amount of Token A if deposits or Token A→Token B swaps create a peg deficit.

Because there is no quantitative limit, a sequence of large Token B deposits can expand total synthetic supply contradicting the intent of “reduce-only” (wind-down risk).

Impact

• No immediate loss of funds, but the pool’s net synthetic exposure can increase while users believe risk is being reduced.

• If synthetic supply grows significantly, settlement collateral requirements rise and downstream risk metrics become inaccurate.

• Classified as Informational because the behaviour is within spec but undermines policy assumptions.

It is recommended to keep minting enabled but cap it per ledger to a small, configurable fraction of current supply.

Example patch inside rebalance():

let delta_a = get_delta_a(&e, base_price, quote_price);

if pool.is_reduce_only() && delta_a > 0 {

    // allow minting up to 0.1 % of current supply per ledger

    let mint_cap = (get_total_synthetic_tokens(&e) / 1000) as i128;

    if delta_a > mint_cap {

        panic_with_error!(&e, PoolError::SwapReduceOnly);

    }

}

• Synthetic supply can still rise enough to keep trades flowing, but risk is bounded.

• Make mint_cap_bps a governance parameter so admins can tighten or loosen the limit as needed.

• Edge-case behaviour (pool reaches cap and price drifts) should be documented for integrators.

• Consider emitting an event when minting is blocked by the cap to alert risk dashboards.

SOLVED: The Normal Finance team successfuly remediated this issue by making the following improvements.

• Strict ReduceOnly for all the use cases

• Configurable rebalance mint cap and event