Move Smart Contract Arithmetic: How Precision Fails and Economics Break

// Context: this `ifixed` library encodes signed values in sign-magnitude,
// using the top bit of `u256` as the sign. The overflow check confuses
// "top bit set" (a valid negative) with "magnitude overflow", so every
// legitimately-negative product is rejected with an OVERFLOW_ERROR.
//
// Bit layout:
//   z < GREATEST_BIT   => magnitude OK, top bit clear
//   z >= GREATEST_BIT  => magnitude exceeds 2^(width-1), i.e., real overflow
//   z | GREATEST_BIT   => valid negative encoding (top bit set on purpose)

// VULNERABLE: aborts on every legitimately-negative product, since the
// negative encoding *requires* the top bit to be set.
public fun from_u256balance(balance: u256, scaling_factor: u256): u256 {
    let z = balance * scaling_factor;
    if (balance ^ scaling_factor < GREATEST_BIT) {
        // Same sign => positive result => top bit must be clear. OK.
        assert!(z < GREATEST_BIT, OVERFLOW_ERROR);
        return z
    };
    // Opposite signs => negative result => top bit SHOULD be set.
    // BUG: this treats "top bit set" as overflow and DoSes every valid input.
    assert!(z <= GREATEST_BIT, OVERFLOW_ERROR);
    (GREATEST_BIT - z) ^ GREATEST_BIT
}

// SAFE: separate magnitude check from sign encoding.
public fun from_u256balance(balance: u256, scaling_factor: u256): u256 {
    let z = balance * scaling_factor;
    assert!(z < GREATEST_BIT, OVERFLOW_ERROR); // pure magnitude check, sign-agnostic
    if (balance ^ scaling_factor < GREATEST_BIT) z else (z | GREATEST_BIT)
}
// Same class as Cetus: hand-rolled signed types over `u256` are bug magnets because 
// Move has no native `i256`.

// Context: a hand-rolled i192 decoder for Chainlink prices guards against
// out-of-range values by comparing against `1 << 192` instead of `1 << 191`.
// For a 192-bit signed integer, the sign bit lives at position 191 (anything
// with bit 191 set is already negative), so the off-by-one lets negatives
// in [2^191, 2^192) slip past the guard and be read as positive prices.
//
// Layout: width = 192, sign bit at position 191 (the high bit of 192).

// VULNERABLE:
fun parse_chainlink_price(price: u256): (Status, FixedPoint64) {
    // BUG: off-by-one. (1 << 192) is one bit too high.
    // Negatives in [2^191, 2^192) slip past this guard and are read as positive.
    if (price >= (1 << 192)) { return (status::broken_status(), fixed_point64::zero()) };
    // ... downstream code treats `price` as a positive magnitude ...
}

// SAFE:
fun parse_chainlink_price(price: u256): (Status, FixedPoint64) {
    if (price >= (1 << 191)) { return (status::broken_status(), fixed_point64::zero()) };
    // ...
}

// Same root cause as the previous finding: no native `iN` in Move/Aptos, so teams encode 
// signedness on `u256` by hand and get the bit position wrong.

// Context: liquidity math wants `delta_l * num / denom`, where `num/denom`
// is a fractional ratio that is often < 1 in fixed-point terms. Doing the
// divide first truncates the ratio to 0, and the whole required-token
// amount collapses to 0, so downstream margin/liquidation logic then reads
// "this position needs 0 tokens".

public fun x_by_liquidity_x64(self: &PositionModel, sqrt_p_x64: u128, delta_l: u128): u128 {
    [...]
    // Target math:  delta_l * num / denom
    // Where num/denom is often < 1 (a fractional ratio in fixed-point terms).
    let num = ((self.sqrt_pb_x64 - sqrt_p_x64) as u256) << 128;
    let denom = (sqrt_p_x64 as u256) * (self.sqrt_pb_x64 as u256);

    // VULNERABLE:
    // 1) num / denom truncates toward 0.
    // 2) If num < denom, num/denom == 0.
    // 3) Then delta_l * 0 == 0, regardless of how large delta_l is.
    // Result: downstream margin/liquidation logic sees “0 tokens required”.
    let x_x64 = (delta_l as u256) * (num / denom);

    [...]
}

// SAFE: multiply first (keep precision in the numerator), divide last.
public fun x_by_liquidity_x64_safe(self: &PositionModel, sqrt_p_x64: u128, delta_l: u128): u128 {
    [...]
    let num = ((self.sqrt_pb_x64 - sqrt_p_x64) as u256) << 128;
    let denom = (sqrt_p_x64 as u256) * (self.sqrt_pb_x64 as u256);

    // Multiplication inflates the numerator before truncation happens.
    let x_x64 = ((delta_l as u256) * num) / denom;
    [...]
}

// Context: small orders cause the intended DEEP-token fee to truncate to 0.
// The `else if` branch isn't a graceful "fee-free" path; it routes into a
// quote-token penalty scaled by a multiplier. Users who expect a tiny DEEP
// fee silently get charged a much larger quote fee in a different unit.

public(package) fun fee_quantity([...]): Balances {
    [...]
    // deep_quantity is computed via integer math.
    // For small orders, the intended DEEP fee can truncate to 0.

    if (deep_quantity > 0) {
        // Intended path: charge DEEP fee.
        balances::new(0, 0, deep_quantity)

    } else if (is_bid) {
        // VULNERABLE FALLBACK:
        // deep_quantity == 0 does NOT mean “fee-free”.
        // It routes into a *different fee regime* (quote-token penalty).
        // User expects “small DEEP fee”, gets “quote fee * multiplier”.
        balances::new(
            0,
            math::mul(quote_quantity, constants::fee_penalty_multiplier()),
            0,
        )

    } [...]
}

// SAFE (conceptual): treat "rounds to zero" as an error or enforce a min size.
public(package) fun fee_quantity_safe([...]): Balances {
    [...]
    // Option A: enforce minimum order size so intended fee never truncates to 0.
    assert!(deep_quantity > 0, E_FEE_ROUNDS_TO_ZERO);
    balances::new(0, 0, deep_quantity)

    // Option B: if you truly want a fallback, it must be equivalent in meaning
    // (same unit/denomination and same economic intent), not a different penalty path.
}

// Context: LP-share math has TWO bugs stacked. (1) The fraction is inverted
// (pool_total / deposit instead of deposit / pool_total). (2) Even with the
// operands fixed, quantizing the share to basis points (1 part in 10,000)
// loses any deposit smaller than 1 bp of the pool, so small depositors
// mint 0 LP tokens for real money.
//
// Correct proportion: deposit / pool_total.

// VULNERABLE:
// Bug 1: INVERTED RATIO (pool_total / deposit).
// Example: deposit=1,000; pool_total=100,000.
// Correct: 1,000/100,000 = 1%.
// Actual: 100,000/1,000 = 100x (as a "share").
let share_proportion_bps = math64::mul_div(
    *vector::borrow(&asset_amounts, 0),      // pool_total
    BPS_BASE,                                 // 10,000
    *vector::borrow(&deposit_amounts, 0)      // deposit
);

// Bug 2: even if the operands were swapped, using BPS loses sub-1bp deposits.
// Anything with true_share < 1 bp truncates to 0 => depositor gets 0 LP tokens.
let depositor_lp_token_share = math64::mul_div(
    lp_token_amount,
    share_proportion_bps,
    BPS_BASE,
);

// SAFE:
// - Fix ratio direction.
// - Use higher precision than BPS for shares (e.g., 1e18), or keep fraction form.
let share_proportion_hi = math64::mul_div(deposit, SHARE_PRECISION, pool_total);
assert!(share_proportion_hi > 0, E_DEPOSIT_TOO_SMALL); // or enforce min deposit
let lp_out = math64::mul_div(lp_token_amount, share_proportion_hi, SHARE_PRECISION);

// Context: both `claimable` (token base units) and `DURATION` (seconds) are
// `u64`, so the type checker doesn't notice the dimensional nonsense. The
// guard `claimable > DURATION` rejects every reward smaller than 604,800
// base units, a number that has nothing to do with token economics.

// DURATION is time (seconds).
const DURATION: u64 = 7 * 86400; // 604,800 seconds

fun extract_claimable_for<T>(self: &mut Voter<T>, gauger_id: ID): Balance<T> {
    let gauger_id = into_gauge_id(gauger_id);
    self.update_for_internal<T>(gauger_id);
    let claimable = *self.claimable.borrow(gauger_id);

    // VULNERABLE:
    // claimable is a token amount (base units), not time.
    // Comparing token units to seconds is dimensionally meaningless.
    // Effect: if claimable <= 604,800 base units, user can never claim (DoS by dust).
    assert!(claimable > DURATION);

    // SAFE (conceptual): assert against a token-denominated threshold, or remove.
    // Example: enforce a minimum claim size in token base units.
    assert!(claimable > MIN_CLAIMABLE_TOKENS, E_TOO_SMALL_TO_CLAIM);

    // [...]
}

// VULNERABLE: First depositor can inflate share price
public entry fun deposit(user: &signer, pool: &mut Pool, amount: u64) {
    let shares = if (pool.total_shares == 0) {
        amount // First depositor: 1:1 ratio
    } else {
        amount * pool.total_shares / balance::value(&pool.assets)
        // After donation: 1000 * 1 / 1000001 = 0 shares for victim
    };
    pool.total_shares = pool.total_shares + shares;
    balance::join(&mut pool.assets, take_payment(user, amount));
}

// SAFE: Dead shares + minimum deposit
public entry fun deposit(user: &signer, pool: &mut Pool, amount: u64) {
    assert!(amount >= MIN_DEPOSIT, E_TOO_SMALL);
    let shares = if (pool.total_shares == 0) {
        // Burn dead shares to prevent empty-pool manipulation
        let dead_shares = DEAD_SHARES; // e.g., 1000
        pool.total_shares = pool.total_shares + dead_shares;
        amount - dead_shares
    } else {
        amount * pool.total_shares / balance::value(&pool.assets)
    };
    assert!(shares > 0, E_ZERO_SHARES);
    pool.total_shares = pool.total_shares + shares;
    balance::join(&mut pool.assets, take_payment(user, amount));
}

// Context: same first-depositor inflation shape as a vault, but on a Walrus
// staking pool's exchange rate. If `share_amount` is ever drained to 0,
// `(amount * wal_amount) / share_amount` divides by zero and aborts every
// call. Because the staking pool participates in epoch advancement, that
// single dust attack halts chain progression (liveness, not theft).

public(package) fun convert_to_wal_amount(exchange_rate: &PoolExchangeRate, amount: u64): u64 {
    match (exchange_rate) {
        PoolExchangeRate::Flat => amount,
        PoolExchangeRate::Variable { wal_amount, share_amount } => {
            let amount = (amount as u128);
            // VULNERABLE: if `share_amount` ever reaches 0, this divides by zero
            // and aborts. In a staking pool that participates in epoch advancement,
            // a single dust attack that empties shares bricks the chain's progression.
            let res = (amount * *wal_amount) / *share_amount;
            res as u64
        },
    }
}

// SAFE (conceptual): permanent dust floor so `share_amount > 0` is invariant.
//   - Mint dead shares at pool creation that can never be burned, OR
//   - Reject the operation that would zero out share_amount.

// Context: `(amount * ratio) / PRECISION` is the standard integer-math way
// to convert an underlying `amount` into shares; `ratio` is the share
// price pre-multiplied by `PRECISION` so the divide brings the result back
// to share units. A "fairness" patch tries to protect users whose deposit
// truncates to 0 shares by handing them 1 share. The hole: `amount` has no
// minimum, and when 1 share is worth far more than 1 base unit of
// underlying, that 1 share is essentially free.

// Walkthrough (pool: 1,000,000 underlying / 1,000 shares, so 1 share ≈ 1,000
// underlying; ratio ≈ PRECISION / 1,000):
//   1. Attacker calls from_shares(ratio, 1).
//   2. shares = 1 * (PRECISION / 1,000) / PRECISION  -->  truncates to 0.
//   3. Floor-to-1 fires: shares = 1. Cost: 1 base unit. Redeem value: ~1,000.
//   4. Repeat. One ~free share per call. Drains the pool.

// VULNERABLE:
public fun from_shares(ratio: u256, amount: u64): u64 {
    let shares = (amount as u256) * ratio / PRECISION;
    assert!(shares <= (U64_MAX as u256), E_U64_OVERFLOW);
    // BUG: unbounded floor-to-1. Adversarial dust mints 1 share for ~free.
    if (amount > 0 && shares == 0) { shares = 1 };
    (shares as u64)
}

// SAFE: reject the round-to-zero case instead of promoting it. Users with
// legitimate dust must batch off-chain into a single larger deposit.
public fun from_shares(ratio: u256, amount: u64): u64 {
    let shares = (amount as u256) * ratio / PRECISION;
    assert!(shares <= (U64_MAX as u256), E_U64_OVERFLOW);
    assert!(shares > 0, E_AMOUNT_TOO_SMALL); // no silent kicker
    (shares as u64)
}

// Context: an LP wants to deposit `max_a` of token A into a pool with
// current reserves (reserve_a, reserve_b). To preserve the pool's price,
// the LP must also deposit token B at the exact current ratio:
//
//     b_star = max_a * reserve_b / reserve_a
//
// Because Move integer division truncates, the protocol has to pick a
// rounding direction whenever this product is not exact. That choice
// decides who absorbs the sub-unit dust: the depositor or the pool.

// VULNERABLE: rounds UP. If the true ratio works out to 999.4 base units of
// token B, `safe_mul_div_up` returns 1000, so the LP pays 1 extra base unit
// per deposit. The extra unit stays inside the pool (i.e., accrues to
// existing LP holders). Spread across many deposits, this is a silent tax
// on new participants in favor of incumbents.
let b_star = safe_mul_div_up(max_a, reserve_b, reserve_a);

// SAFE: round DOWN for the user's *required* input (999 instead of 1000).
// The pool then verifies the resulting ratio is within an acceptable
// slippage band before accepting the deposit.
//
// Rule of thumb: protocol wins on outputs (payouts, fees collected),
// user wins on inputs (required deposits). Either flip leaks value.
let b_star = safe_mul_div_down(max_a, reserve_b, reserve_a);

// Context: the validator-committee loop tallies `(capacity * n_shards) / weight`
// for every node. One line, two opposite failure modes: overflow if any
// node sets a huge capacity, div-by-zero if a node's stake yields zero
// shards. Either abort halts epoch advancement for the whole network.

public(package) fun select_committee_and_calculate_votes(self: &mut StakingInnerV1) {
    [...]
    node_ids.length().do!(|idx| {
        [...]
        // VULNERABLE: two failure modes on one line.
        //   1. node_capacity huge => (capacity * n_shards) overflows u64 => abort.
        //      A malicious node sets a giant capacity to brick this loop.
        //   2. weight == 0 (stake too low for any shards) => div-by-zero => abort.
        let capacity_vote = (pool.node_capacity() * (self.n_shards as u64)) / weight;
        capacity_votes.insert(capacity_vote, weight);
    });
    [...]
}

// SAFE (conceptual):
//   - Promote to u128 for the multiply: ((cap as u128) * (n_shards as u128)) / weight.
//   - Guard the divisor: assert!(weight > 0, E_NO_SHARDS); or skip the node.
//   - Bound node_capacity at registration so adversarial values cannot enter the loop.

// Context: the registry counter is `u8` (max 255), but the bitmap it indexes
// holds only 128 slots. The VM only aborts when the counter wraps at 255,
// but indexes 128..254 silently land out-of-range against the bitmap and
// corrupt downstream state long before that abort ever fires. The relevant
// bound is the *consumer's* range, not the integer type's range.

// VULNERABLE:
public fun next_id(registry: &mut TransceiverRegistry): u8 {
    let id = registry.next_id;
    registry.next_id = id + 1; // VM only aborts at id == 255, not at id == 128.
    id
}

// SAFE: bound the counter at the actual data-structure capacity.
public fun next_id(registry: &mut TransceiverRegistry): u8 {
    let id = registry.next_id;
    assert!((id as u64) < BITMAP_CAPACITY, E_REGISTRY_FULL); // 128, not 256
    registry.next_id = id + 1;
    id
}

// Context: the textbook ceiling-division trick `(a + b - 1) / b` materializes
// the sum `a + b - 1` BEFORE the divide. For values near `u128::MAX`, that
// intermediate addition aborts on overflow even though the true ceiling
// result would fit comfortably in `u128`.

// VULNERABLE: classic ceiling-division helper.
fun div_up(a: u128, b: u128): u128 {
    (a + b - 1) / b // overflow if a + b - 1 > u128::MAX
}

// SAFE: split into floor + fix-up so no oversized intermediate exists.
fun div_up(a: u128, b: u128): u128 {
    let q = a / b;
    if (a % b == 0) q else q + 1
}

// Context: the math relies on the implicit invariant `withdrawn <= vested_amount`.
// Revocation reduces `vested_amount`, which can flip the invariant. Move
// aborts on subtraction underflow (unlike Solidity's silent wrap), so a
// state-lifecycle edge case becomes a permanent abort: every future
// `claim` call dies on the same subtraction line.

// VULNERABLE: every future call aborts on the subtraction.
let claimable = vested_amount - withdrawn;

// SAFE: saturate at zero. The invariant about which is larger now lives
// in the math, not in the implicit state-machine assumption.
let claimable = if (vested_amount > withdrawn) { vested_amount - withdrawn } else { 0 };

// Context: `liquidate_col_y` takes a `SupplyPool<$X, $SX>` parameter, where
// `$SX` is the share-token type that the supply pool issues for debt
// accounting. The function pulls *all* debt shares out of the position via
// `take_all()` and feeds them into `supply_pool.repay_max_possible(...)`.
// Because `$SX` is never asserted to match the share type stored on the
// position, an attacker can pass a sibling `SupplyPool<$X, $SX'>` whose
// share type is foreign. `repay_max_possible` then finds zero matching
// shares to burn, returns `x_repaid = 0`, and the call short-circuits as a
// no-op for the debt leg. The reward/seize leg further down still transfers
// collateral, so the liquidator walks away with `Balance<$Y>` for free.

// VULNERABLE: no assertion that `$SX` matches the position's debt-share type.
public(package) macro fun liquidate_col_y<$X, $Y, $SX, $LP>(
    $position: &mut Position<$X, $Y, $LP>,
    $config: &PositionConfig,
    $price_info: &PythPriceInfo,
    $debt_info: &DebtInfo,
    $repayment: &mut Balance<$X>,
    $supply_pool: &mut SupplyPool<$X, $SX>,
    $clock: &Clock,
): Balance<$Y> {
    [...]
    let mut debt_shares = position.debt_bag_mut().take_all();
    // BUG: $SX may not match the share type stored on $position.
    // repay_max_possible then burns 0 shares, x_repaid == 0, debt is untouched.
    let (_, x_repaid) = supply_pool.repay_max_possible(&mut debt_shares, &mut r, $clock);
    [...]
}

// SAFE: assert the position's debt-share type matches the supply pool's `$SX`
// before any repayment math runs.
public(package) macro fun liquidate_col_y<$X, $Y, $SX, $LP>(
    $position: &mut Position<$X, $Y, $LP>,
    /* ... */
    $supply_pool: &mut SupplyPool<$X, $SX>,
    $clock: &Clock,
): Balance<$Y> {
    assert!(
        $position.debt_share_type() == type_name::get<$SX>(),
        E_WRONG_SUPPLY_POOL
    );
    /* ... */
}

// Context: Thala's `stable_pool::flashloan<A, B, C, D>` borrows the asset
// in whichever generic position has a non-zero amount, and the loan is
// returned typed as the *first* generic. The liquidation helper wants to
// borrow `AptosCoin` to repay a debt, then seize THL collateral. The
// author listed `ThalaAPT` first and `AptosCoin` second, but the returned
// `loan` is bound to the first slot. Type checking passes because both
// types are valid generics; the function silently produces a
// `Coin<ThalaAPT>` instead of `Coin<AptosCoin>`, and the rest of the
// liquidation operates on the wrong asset.

// VULNERABLE: borrows ThalaAPT (first generic), but caller meant AptosCoin.
let (zero_0, loan, zero_2, zero_3, flashloan) = stable_pool::flashloan<ThalaAPT, AptosCoin,
        Null, Null>(
    repay_amount, 0, 0, 0
);
// `loan: Coin<ThalaAPT>`, not Coin<AptosCoin>. Downstream liquidate() now
// repays the wrong debt leg and produces an inconsistent position.
let seized = liquidate<THL, AptosCoin>(account, vault_address, loan, min_shares_out);

// SAFE: place the asset to be borrowed in the FIRST generic slot, with a
// non-zero amount only on that slot. Better still, expose a non-positional
// API (e.g. `flashloan_of<T>(amount)`) so the type witness, not the
// argument index, decides which asset is borrowed.
let (loan, flashloan) = stable_pool::flashloan_of<AptosCoin>(repay_amount);
let seized = liquidate<THL, AptosCoin>(account, vault_address, loan, min_shares_out);

// Context: Magna Airlock represents each vesting allocation as a merkle
// leaf and stores per-allocation runtime state (withdrawn amounts,
// termination timestamps) in a dynamic-object-field hung off `vesting_uid`.
// The DOF key is the user-supplied `allocation_id`, but `allocation_id` is
// not unique across leaves: distinct leaves (different beneficiary, amount,
// or schedule) can share the same id. When two leaves share an id, the
// second `ofield::add` either aborts the second registration or silently
// reuses the first leaf's `DistributionState`. The first beneficiary to
// call `withdraw` then drains the merged accounting and locks every other
// claimant out, with no way to recover (merkle roots cannot be removed).

// VULNERABLE: keyed only by allocation_id, not by leaf hash.
ofield::add(
    vesting_uid,
    allocation.allocation_id, // BUG: not unique per leaf.
    DistributionState {
        beneficiary: allocation.original_beneficiary,
        withdrawn: 0,
        terminated_timestamp: 0,
    },
);

// SAFE: key by the merkle leaf hash so two distinct leaves cannot collide.
// `leaf_hash` mixes every field that distinguishes one allocation from
// another, so a collision implies a true merkle duplicate (which is
// rejected upstream during root construction).
ofield::add(
    vesting_uid,
    leaf_hash, // hash(allocation_id || beneficiary || amount || schedule || ...)
    DistributionState { /* ... */ },
);

// Context: a Pair<L, C> assigns liability asset L and collateral asset C.
// Nothing prevents L == C, so a user can deposit X as collateral and borrow
// X against it, sidestepping the collateralization invariant entirely.
// Liquidation never triggers because seized collateral exactly cancels
// the debt in the same unit.

// VULNERABLE: no liability != collateral check at pair creation.
public fun create_pair<L, C>(/* ... */): Pair<L, C> {
    Pair {
        liability_type:  type_of<L>(),
        collateral_type: type_of<C>(),
        /* ... */
    }
}

// SAFE: assert distinct asset types.
public fun create_pair<L, C>(/* ... */): Pair<L, C> {
    assert!(type_of<L>() != type_of<C>(), E_SAME_ASSET_PAIR);
    Pair { /* ... */ }
}
// A short asserter assert!(liability_type != collateral_type) is a one-liner 
// that closes a whole class of economic attacks.

// Context: pool creators choose lot_size, tick_size, and min_size freely.
// With adversarial parameters, two distinct base quantities collapse to
// the same quote quantity, and the price-quantity map then keys on the
// collided value and orders that should be separate overwrite each other.

// VULNERABLE: factory accepts any combination.
public fun create_pool<B, Q>(
    lot_size:  u64,   // e.g. 1
    tick_size: u64,
    min_size:  u64,   // e.g. 1000
    /* ... */
): Pool<B, Q> { /* ... */ }

// Collision example with lot_size = 1, min_size = 1000:
//   base_qty = 1000  -> quote_qty rounds to k
//   base_qty = 1999  -> quote_qty rounds to k  (same bucket)

// SAFE (conceptual): enforce a granularity invariant so distinct base
// quantities cannot share a quote bucket.
assert!(min_size >= MIN_SIZE_FLOOR, E_BAD_PARAMS);
assert!(min_size % lot_size == 0,   E_BAD_PARAMS);
assert!(quotes_distinct(lot_size, tick_size, min_size), E_BAD_PARAMS);

// Context: stLPT's oracle is intentionally biased downward to be safe on
// the *collateral* side (don't lend too much against it). The same oracle
// is then reused on the *borrow* side, where a low price means a discount:
// the attacker borrows stLPT cheap and unwinds at true market value.
// Direction of bias must depend on which leg consumes the price.

// VULNERABLE: same `price()` returned for both legs.
public fun price(asset: &Asset): u64 {
    asset.conservative_low_price // safe for collateral, toxic for borrow
}

let collateral_value = amount * price(&stlpt) / PRECISION; // OK, undervalues
let borrow_cost      = amount * price(&stlpt) / PRECISION; // BUG: also undervalues
                                                            //      => cheap borrow

// SAFE: separate oracle reads per leg, with role-aware bias.
public fun price_for_collateral(asset: &Asset): u64 { asset.conservative_low_price }
public fun price_for_borrow(asset: &Asset):     u64 { asset.conservative_high_price }

// Context: oracle samples carry (timestamp, price, confidence). The selector
// wants "newest sample whose confidence is acceptable", but combines all
// three checks into one short-circuited expression. The recency leg fires
// first, so a newer-but-low-confidence sample updates max_timestamp_ms
// and silently filters out an older trustworthy price.

// VULNERABLE: one combined gate.
if (is_valid(ts) && ts > max_timestamp_ms && confidence_ok(conf)) {
    max_timestamp_ms = ts; // BUG: timestamp wins even when confidence fails,
    selected_price   = p;  //      because && short-circuits left-to-right.
}

// SAFE: independent gates, in the right order.
if (!is_valid(ts))             continue;
if (!confidence_ok(conf))      continue;   // confidence first
if (ts <= max_timestamp_ms)    continue;   // then recency
max_timestamp_ms = ts;
selected_price   = p;

// Context: `message` is a Solidity-side ABI-packed payload (big-endian).
// `from_bcs::*` deserializes BCS, which is little-endian. Decoding the same
// bytes with the wrong endianness flips every multi-byte integer.
//
// Example: ccdm_nonce = 1 on Solidity is 0x00..01 (32 bytes, big-endian).
// `from_bcs::to_u256` reads those bytes as 0x01..00 = 1 << 248.

// VULNERABLE: byte-order mismatch.
let market_hash       = from_bcs::to_address(vector::slice(&message, 0, 32));
let ccdm_nonce        = from_bcs::to_u256(vector::slice(&message, 32, 64));
let expected_messages = from_bcs::to_u8(vector::slice(&message, 64, 65));

// SAFE (conceptual): reverse each multi-byte integer slice before decoding,
// or use a Solidity-aware ABI decoder. `from_bcs` is not a generic ABI decoder.

// Context: payload is a 65-byte header followed by N × 32-byte depositor
// records. The intended check is "after the header, the remainder is a
// multiple of 32". The author wrote the wrong form.

// VULNERABLE: unreachable. `len % 32` is always in [0, 32), never 65.
assert!(vector::length(&message) % 32 == 65, ERR_DEPOSIT_MANAGER_MALFORMED_PAYLOAD);

// SAFE: subtract the header length first, then check divisibility.
let len = vector::length(&message);
assert!(len >= 65 && (len - 65) % 32 == 0, ERR_DEPOSIT_MANAGER_MALFORMED_PAYLOAD);

// Context: Magna Airlock parses vesting allocations from BCS payloads.
// Three classes of bug all stem from "deserialize, then trust":
//   - parallel arrays without a length-equality assert
//   - period denominators without a > 0 assert
//   - sum(unlock_amounts) != allocation.amount with no equality assert
// Because merkle roots cannot be removed, any malformed leaf bricks the
// allocation forever.

// VULNERABLE:
Schedule::Calendar {
    unlock_timestamps: stream.peel_vec!(|stream| stream.peel_u32()), // Missing: length
    unlock_amounts:    stream.peel_vec!(|stream| stream.peel_u64()), // equality assert
}
Piece {
    start_time:        stream.peel_u32(),
    period_length:     stream.peel_u32(), // Missing: assert period_length > 0
    number_of_periods: stream.peel_u32(), // Missing: assert number_of_periods > 0
    amount:            stream.peel_u64(),
}

// SAFE (conceptual): post-decode invariant block.
//   assert!(vector::length(&unlock_timestamps) == vector::length(&unlock_amounts), E_LEN);
//   assert!(period_length > 0 && number_of_periods > 0, E_ZERO);
//   assert!(sum(&unlock_amounts) == allocation.amount, E_SUM);

// Context: Aptos's framework `randomness` API is unavailable during
// `init_module`, so teams hand-roll a selector. The straightforward
// `random % range` introduces a bias of (2^256 mod range) / 2^256:
// negligible per draw for small ranges, exploitable when the selector
// is called at scale.

// VULNERABLE: biased.
fun u64_range(low: u64, high: u64): u64 {
    let r = random_u256();
    let range = (high - low) as u256;
    ((r % range) as u64) + low
}

// SAFE: rejection sampling. Discard draws in the biased remainder window.
fun u64_range(low: u64, high: u64): u64 {
    let range = (high - low) as u256;
    let limit = (U256_MAX / range) * range; // largest multiple of range <= U256_MAX
    loop {
        let r = random_u256();
        if (r < limit) { return ((r % range) as u64) + low }
    }
}