Root + Impact
Description
-
The engine converts collateral amounts to USD (and back) so it can compute health factors and liquidation payouts. It must work for the supported collateral, WETH (18 decimals) and WBTC (8 decimals).
-
`_get_usd_value` and `_get_token_amount_from_usd` hard-code an 18-decimal token (`PRECISION = 1e18`) and never read the token's `decimals()`. For WBTC, a collateral amount is valued 1e10 times too low, and a USD amount converts to 1e10 times too many WBTC base units.
// Root cause in the codebase with @> marks to highlight the relevant section
```python
def _get_usd_value(token: address, amount: uint256) -> uint256:
...
return (
(convert(price, uint256) * ADDITIONAL_FEED_PRECISION) * amount
) // PRECISION
def _get_token_amount_from_usd(token: address, usd_amount_in_wei: uint256) -> uint256:
...
return (
(usd_amount_in_wei * PRECISION) // (
convert(price, uint256) * ADDITIONAL_FEED_PRECISION
)
)
```
Risk
Likelihood:
-
It triggers whenever a user deposits WBTC (an in-scope collateral token on ZKsync Era), through `deposit_collateral`, `mint_dsc` or `liquidate`. No special conditions are needed.
-
Every health-factor computation sums `_get_usd_value` over both collateral tokens, so a WBTC balance is always miscounted.
Impact:
-
WBTC collateral is effectively worthless to the protocol. 1 WBTC at $60,000 is valued at about $0.000006, so a WBTC depositor can mint almost no DSC. If they hold WBTC next to a DSC debt, they are liquidatable almost immediately.
-
Liquidating a WBTC position computes a payout about 1e10 too large. The `_redeem_collateral` subtraction underflows and reverts, so WBTC-backed positions cannot be liquidated. Bad debt can build up unchecked.
-
The protocol's stated goal, that you could "swap out WETH & WBTC for any basket of assets and the code would work the same", is also false for any token that isn't 18 decimals.
Proof of Concept
Assumes WBTC is an 8-decimals token with a Chainlink-style feed at $60,000 (8 decimals, so the price is 6e12). Adapt the fixture names to your `conftest.py`.
```python
import boa
def test_poc_wbtc_undervalued(dsce, wbtc, wbtc_usd_feed, user):
one_wbtc = 10**8
price = wbtc_usd_feed.latestRoundData()[1]
expected_usd = price * 10**10 * one_wbtc // 10**8
actual_usd = dsce.get_usd_value(wbtc.address, one_wbtc)
print("expected:", expected_usd, "actual:", actual_usd)
assert actual_usd == expected_usd // 10**10
def test_poc_cannot_mint_against_wbtc(dsce, wbtc, user):
one_wbtc = 10**8
with boa.env.prank(user):
wbtc.approve(dsce.address, one_wbtc)
dsce.deposit_collateral(wbtc.address, one_wbtc)
with boa.reverts("DSCEngine__BreaksHealthFactor"):
dsce.mint_dsc(10**18)
def test_poc_liquidation_amount_overstated(dsce, wbtc):
units = dsce.get_token_amount_from_usd(wbtc.address, 10**18)
print("units:", units)
assert units > 10**12
```
Run: `mox test tests/test_poc_decimals.py -s`
Result: the first test shows the 1e10 undervaluation. The second shows a user holding about $60,000 of WBTC cannot mint $1 of DSC. The third shows liquidation converts $1 of debt into roughly 166,666 WBTC, far more than any user holds, so `_redeem_collateral` would underflow.
Recommended Mitigation
Normalize by each token's own decimals:
**Why it works:** for an 18-decimal token (WETH), `10 ** 18` equals the old `PRECISION`, so behavior is unchanged. For WBTC, `10 ** 8` is used, so 1 WBTC (`1e8`) at $60,000 is valued at `6e22`, which is $60,000 in 18-decimal USD. A $1 debt now converts to about 1,666 WBTC base units instead of 1.67e13.
**Verify the fix** (these tests fail before the patch and pass after):
```python
def test_fix_wbtc_valued_correctly(dsce, wbtc, wbtc_usd_feed):
price = wbtc_usd_feed.latestRoundData()[1]
assert dsce.get_usd_value(wbtc.address, 10**8) == price * 10**10 # 1 WBTC = full price
def test_fix_wbtc_roundtrip(dsce, wbtc):
usd = 60_000 * 10**18
units = dsce.get_token_amount_from_usd(wbtc.address, usd)
assert units == 10**8 # $60k -> 1 WBTC
def test_fix_weth_unchanged(dsce, weth, weth_usd_feed):
price = weth_usd_feed.latestRoundData()[1]
assert dsce.get_usd_value(weth.address, 10**18) == price * 10**10 # 18-dec path unchanged
```
**Optional hardening:**
- Read each token's decimals once in `__init__` and store them in a map (or an immutable array for the two collateral tokens), to avoid an external call on every price lookup.
- Add `assert price > 0` after the oracle call in `_get_token_amount_from_usd`, so a zero price reverts with a clear message instead of dividing by zero.
