You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository has been archived by the owner on Sep 17, 2023. It is now read-only.
sherlock-admin opened this issue
Mar 13, 2023
· 0 comments
Labels
DuplicateA valid issue that is a duplicate of an issue with `Has Duplicates` labelHighA valid High severity issueRewardA payout will be made for this issue
Use of hard coded decimals of 18 for collateral token
Summary
The protocol assumes the decimal of the collateral token is unanimously 18 as the decimal of TAU. This could lead to arithmetic issue if the decimal of a non-GLP token is non-18 and it is used as a yield-bearing collateral for a different vault.
Vulnerability Detail
Here is a typical scenario:
The decimal of the newly selected collateral token is 8.
The code line in _computeCR() associated with calculating newCollRatio:
10 ** priceDecimals in the denominator serves to ensure _price is 1e18 scaled. Currently, the decimal of GLP token is 18. So this is not going to be an issue. However, in this scenario, the decimal of the collateral for a new vault entailed is 8, and it is going to make newCollRatio 10 ** (18 - 10), i.e. 1e8 scaled instead of the intended 1e18.
(Note: In an opposite scenario, if the decimal of the collateral token entailed were more than 18, newCollRatio would end up over scaled.)
Impact
Consequently, this makes the accountHealth tremendously go underwater and set diff == MAX_LIQ_DISCOUNT by default when liquidate() is called.
(Note: In the opposite scenario, this would make users' accountHealth super good and never liquidable since _calcLiquidationDiscount() would readily revert for the unhealthy accounts.
Additionally, the erroneous logic is going to directly affect the view functions dependent on _computeCR(), i.e. getMaxLiquidation() and _getCollRatio().
And apparently, _getMaxLiquidation() involving a ratio that has collateral appear as the subtrahend of the numerator will also be likewise affected.
Consider having the collateral token amount 18 decimals scaled whenever it is included in a ratio or a division.
Here is a useful pure function that may be implemented in BaseVault.sol along with some needed additions since no state variables are allowed in the library, TauMath.sol:
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
DuplicateA valid issue that is a duplicate of an issue with `Has Duplicates` labelHighA valid High severity issueRewardA payout will be made for this issue
RaymondFam
medium
Use of hard coded decimals of 18 for collateral token
Summary
The protocol assumes the decimal of the collateral token is unanimously 18 as the decimal of
TAU
. This could lead to arithmetic issue if the decimal of a non-GLP token is non-18 and it is used as a yield-bearing collateral for a different vault.Vulnerability Detail
Here is a typical scenario:
The decimal of the newly selected collateral token is 8.
The code line in _computeCR() associated with calculating
newCollRatio
:File: TauMath.sol#L18
10 ** priceDecimals
in the denominator serves to ensure_price
is 1e18 scaled. Currently, the decimal ofGLP
token is 18. So this is not going to be an issue. However, in this scenario, the decimal of the collateral for a new vault entailed is 8, and it is going to makenewCollRatio
10 ** (18 - 10), i.e. 1e8 scaled instead of the intended 1e18.(Note: In an opposite scenario, if the decimal of the collateral token entailed were more than 18,
newCollRatio
would end up over scaled.)Impact
Consequently, this makes the
accountHealth
tremendously go underwater and setdiff == MAX_LIQ_DISCOUNT
by default whenliquidate()
is called.(Note: In the opposite scenario, this would make users'
accountHealth
super good and never liquidable since_calcLiquidationDiscount()
would readily revert for the unhealthy accounts.Additionally, the erroneous logic is going to directly affect the view functions dependent on
_computeCR()
, i.e.getMaxLiquidation()
and_getCollRatio()
.And apparently,
_getMaxLiquidation()
involving a ratio that hascollateral
appear as the subtrahend of the numerator will also be likewise affected.File: BaseVault.sol#L251-L253
Code Snippet
File: TauMath.sol#L11-L27
File: BaseVault.sol#L432-L445
File: BaseVault.sol#L240-L261
Tool used
Manual Review
Recommendation
Consider having the collateral token amount 18 decimals scaled whenever it is included in a ratio or a division.
Here is a useful pure function that may be implemented in BaseVault.sol along with some needed additions since no state variables are allowed in the library, TauMath.sol:
For instance,
getMaxLiquidation()
invokingcomputeCR()
may be refactored as follows:Duplicate of #35
The text was updated successfully, but these errors were encountered: