Incorrectly assigned decimal1
parameter upon decoding
#17
Labels
3 (High Risk)
Assets can be stolen/lost/compromised directly
bug
Something isn't working
H-02
primary issue
Highest quality submission among a set of duplicates
🤖_09_group
AI based duplicate group recommendation
satisfactory
satisfies C4 submission criteria; eligible for awards
selected for report
This submission will be included/highlighted in the audit report
sponsor confirmed
Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
sufficient quality report
This report is of sufficient quality
upgraded by judge
Original issue severity upgraded from QA/Gas by judge
Lines of code
https://github.com/code-423n4/2024-07-basin/blob/7d5aacbb144d0ba0bc358dfde6e0cc913d25310e/src/functions/Stable2.sol#L317-L319
Vulnerability details
Impact
Impact is high as lots of functions depend on the
decodeWellData
, and as such will be fed incorrect data abouttoken1
's decimals. This can lead to potential overvaluation or undervaluation of the token's amount and prices, depending on the context with which its used. It also ignores the fact thatdecimals1
can return a 0 value and as such will work with that for scaling, rather than scaling it to 18. This also depends on thedecimal0
being 0;Proof of Concept
In various functions in Stable2.sol, the
decodeWellData
function is called to decode the passed in token decimals data, which is then used to scale the token reserves to a function of 18. This is because the tokens may have different decimals, ranging from as low as 0 to 18.In the
decodeWellData
function, we can see that if the well data returns 0, a returned decimal of 18 is assumed as standard for the token. And as such, each decimals are scaled to that level. However, as can be seen from the @Audit tag, to set a value "18"decimal1
, the function incorrectly checks ifdecimal0
is 0, rather than checking ifdecimal1
is 0;This means that regardless of the value returned for
decimal1
from the decoding, it's replaced with 18, even iftoken1
is a token with 6 decimals, or less than 18. As a result, the reserves will be potentially scaled with a decimal different from actual. Also, whendecimal1
is 0, rather than scaling to a value of 18, it ignores this value and attempts to work with a value of 0 instead. As the function is used extensively, in the codebase, this can lead to serious price miscalculatons.The functions are listed below:
i.
calcLpTokenSupply
ii.
calcReserve
iii.
calcRate
iv.
calcReserveAtRatioSwap
v.
calcReserveAtRatioLiquidity
The gist link below contains a test case that shows that the
calcLpTokenSupply
function (using it as our example) breaks whendecimal1
data returns 0, since its wrongly decoded. Based on the expectation, the test should pass, but it doesn't. It reverts with the error shown below.https://gist.github.com/ZanyBonzy/f387540256c90b6d1b60df6cde9d1413
Tools Used
Manual code review
Recommended Mitigation Steps
Change the check
Assessed type
en/de-code
The text was updated successfully, but these errors were encountered: