search

code-423n4 / 2021-07-sherlock-findings

0 stars 0 forks source link

`Payout.deduction` computation uses wrong decimals #109

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

cmichel

Vulnerability details

The _doSherX function computes the burned SherX tokens deduction as:

// excludeUsd = amounts[i].mul(sx.tokenUSD[tokens[i]])
deduction =
      excludeUsd.div(curTotalUsdPool.div(SherXERC20Storage.sx20().totalSupply)).div(10e17);

This seems to only work if tokens[i] has 18 decimals, as the computation gives a precision of:

tokenIDecimals - 18 + sherXTokens

If tokens[i] has a precision of less than 18 (like USDC/USDT), fewer tokens will be burned breaking the accounting as the USD pool per SherX price decreased drastically.

Recommendation

Assuming tokenUSD is always in 18 decimals (I could not figure this out as this parameter is only set from off-chain), and all listed tokens always have <= 18 decimals, multiply deduction by 10**(18-tokenIDecimals) to receive the amount in SherX tokens.

Evert0x commented 3 years ago

As noted in the readme

TokenX = 18 decimals, price precision = 18 decimals
TokenY = 8 decimals, price precision = 28 decimals
TokenZ = 6 decimals, price precision = 30 decimals

Does this fix the issue?

Evert0x commented 3 years ago

After discussing this finding with the warden it was concluded this is a non-issue.

ghoul-sol commented 3 years ago

marking as invalid