GalloDaSballo
commented
2 years ago Per all the marketing material it's fairly easy to deduct that the Funding contract will either use wBTC or BADGER:
Neither of which will change it's decimals, nor have 1 decimals (8 and 18 respectively)
Personally have never seen a 1 decimal token and would be surprised if there's a 1 decimal token in any widespread tokenList such as Uniswap Token list.
For those reasons, I must disagree as the scenario depicted simply doesn't matches with any real scenario
Lines of code
https://github.com/code-423n4/2022-04-badger-citadel/blob/main/src/Funding.sol#L134 https://github.com/code-423n4/2022-04-badger-citadel/blob/main/src/Funding.sol#L197-L216
Vulnerability details
In
Funding.sol, theassetDecimalsNormalizationValueis set to10 ** asset.decimals(), later it is used to determine home many token units there are per one asset token in human representation.Firstly, this may be highly dangerous as
.decimals()return value isn't said to be constant in any standard, so inserting new values based on the current return value ofasset.decimals()is very dangerous as it may cause the price to be scaled by orders of magnitude.Secondly, for tokens that have very low decimals (more formally, for tokens that 1 token in human representation is of similar value to a single basic unit of CTDL) a huge rounding error will be applied.
For example, if 1 token
Xof decimals 0 is worth 1.5 basic units ofCTDL, theassetDecimalsNormalizationValuewill be 1 and so the price will need to be either 1 or 2, with a nearly 30% change in the price caused by the lack of precision. En even worse scenario arises when tokens which value is less than a unit ofCTDL. They can be priced wither to 0 or 1, all with huge errors.Recommended Mitigation Steps
Create another variable that will reflect the divisor for the price. Either set it to a constant value to be safe against low-decimal tokens or use an immutable value to be set appropriately on contract creation to provide enough precision.