Stableswap will calculate pool share price incorrectly if an asset has greater than 18 decimals, impacting ema oracle entries
Proof of Concept
In stableswap, any liquidity management and trading methods will trigger a hook that writes an oracle entry in ema-oracle.
When an asset has greater than 18 decimals, the asset price will be incorrectly scaled and allowing incorrect oracle entries in ema-oracle.
The vulnerability is in calculate_share_price() that uses saturating_sub() method when scaling decimals. saturating_sub() will allow underflow to pass silently and return 0 value.
In this case, when reserves[asset_idx].decimals is greater than 18, let p_diff = U256::from(10u128.saturating_pow(18u8.saturating_sub(reserves[asset_idx].decimals) as u32)); will simply return p_diff = 10^0=1 regardless of the exact decimals of the asset_idx.
This will incorrectly calculates denom of the returned share price, writing an incorrect oracle entry to ema-oracle.
//HydraDX-node/math/src/stableswap/math.rs
pub fn calculate_share_price<const D: u8>(
reserves: &[AssetReserve],
amplification: Balance,
issuance: Balance,
asset_idx: usize,
provided_d: Option<Balance>,
) -> Option<(Balance, Balance)> {
...
let num = p1.checked_add(p2)?.checked_sub(p3)?;
let denom = issuance.checked_mul(xann.checked_add(c)?)?;
//@audit when reserves[asset_idx].decimals >18, p_diff=10^0=1, incorrect denom will pass
|> let p_diff = U256::from(10u128.saturating_pow(18u8.saturating_sub(reserves[asset_idx].decimals) as u32));
let (num, denom) = if let Some(v) = denom.checked_mul(p_diff) {
(num, v)
} else {
...
Some((num, denom))
For reference, here's the flow involved: liquidity management/trading -> call_on_liquidity_change_hook()/call_on_trade_hook()->get_pool_state()->calculate_share_price()(math.rs)->on_liquidity_changed()/:on_trade()-> ema-oracle entry(OnActivityHandler)
Tools Used
Manual
Recommended Mitigation Steps
Consider handling decimal differences consistently in stableswap math. Similar to normalize_value(), handle both conditions when asset decimals are greater than 18 decimals or smaller than 18 decimals.
OR revert when a certain condition is disallowed instead of silently returning a value.
Lines of code
https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/math/src/stableswap/math.rs#L704
Vulnerability details
Impact
Stableswap will calculate pool share price incorrectly if an asset has greater than 18 decimals, impacting ema oracle entries
Proof of Concept
In stableswap, any liquidity management and trading methods will trigger a hook that writes an oracle entry in ema-oracle.
When an asset has greater than 18 decimals, the asset price will be incorrectly scaled and allowing incorrect oracle entries in ema-oracle.
The vulnerability is in
calculate_share_price()
that usessaturating_sub()
method when scaling decimals.saturating_sub()
will allow underflow to pass silently and return 0 value.In this case, when
reserves[asset_idx].decimals
is greater than 18,let p_diff = U256::from(10u128.saturating_pow(18u8.saturating_sub(reserves[asset_idx].decimals) as u32));
will simply return p_diff = 10^0=1 regardless of the exact decimals of theasset_idx
.This will incorrectly calculates
denom
of the returned share price, writing an incorrect oracle entry to ema-oracle.(https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/math/src/stableswap/math.rs#L704)
For reference, here's the flow involved: liquidity management/trading ->
call_on_liquidity_change_hook()
/call_on_trade_hook()
->get_pool_state()
->calculate_share_price()
(math.rs)->on_liquidity_changed()
/:on_trade()
-> ema-oracle entry(OnActivityHandler
)Tools Used
Manual
Recommended Mitigation Steps
Consider handling decimal differences consistently in stableswap math. Similar to
normalize_value()
, handle both conditions when asset decimals are greater than 18 decimals or smaller than 18 decimals. OR revert when a certain condition is disallowed instead of silently returning a value.Assessed type
Error