Stableswap will calculate pool share price incorrectly if an asset has greater than 18 decimals, impacting ema oracle entries

[c4-bot-10]

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 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))

(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

[c4-pre-sort]

0xRobocop marked the issue as duplicate of #80

[c4-judge]

OpenCoreCH marked the issue as satisfactory

[c4-judge]

OpenCoreCH changed the severity to QA (Quality Assurance)