ArmedGoose - Hardcoded precision is not suitable for all tokens, resulting in unfavorable calculations both for the users and the protocol

[sherlock-admin]

ArmedGoose

high

Hardcoded precision is not suitable for all tokens, resulting in unfavorable calculations both for the users and the protocol

Summary

As per the contest specification, protocol is planning to work with any ERC20 tokens (excluding fee-on-transfer or rebasing which is not the case here). Even limiting the tokens to ones that are commonly traded on UniswapV3, there are some tokens that have non-18 decimals like USDC, USDT (6), or WTBC (8) - and they are commonly used ones (even protocol's unit tests contain WBTC symbol as an example). Multiple funds processing operations rely on COLLATERAL_BALANCE_PRECISION equal to 1e18 which have unwanted effects on tokens with less decimals that still can be used with the protocol (see details for more information).

Vulnerability Detail

Normally, using one unit of token with 18 decimals and later dividing it by 1e18 gives one wei. But if the token has less decimals, dividing one unit of it by 1e18 returns zero, which may have unspecified effect on the protocol. Analogically, multiplying some fraction of value by 1e18 will cause to return a signifinactly larger value in case of low-decimal tokens. The Code snippet section contains detailed explanation of exemplary occurences.

Impact

Users using low-decimal collateral tokens such as WBTC, USDT, USDC may suffer extremely high rates due to precision being not scaled to proper decimals which may cause their collateral to burn instantly due to unreal interest rates. For the protocol, the fees in those tokens, when calculated by dividing by 1e18, will always be zero, which means protocol will not get fees off them. In short, for common low-decimal tokens the protocol is not usable.

Code Snippet

Exemplary occurences and their effect if low-decimal tokens are used instead:

1. Handling fees. collectProtocol

uint256 amount = platformsFeesInfo[token] / Constants.COLLATERAL_BALANCE_PRECISION;

For example here, 1 WBTC = 1_00_000_000 wbtc wei. If there is 1 WBTC balance, this will return zero. only if there is at least 1e18 wbtc wei, that is, 1e10 WBTC, which is unlikely to happen as its too large value, this will return 1. Similar situation will happen with USDC or USDC.

2. Collateral rate - for example checkDailyRateCollateral and function _getDebtInfo that is called inside.

        if (collateralBalance > 0) {
            uint256 everySecond = (
                FullMath.mulDivRoundingUp(
                    borrowing.borrowedAmount,
                    currentDailyRate * Constants.COLLATERAL_BALANCE_PRECISION,
                    1 days * Constants.BP
                )
            );

Here, the currentDailyRate can be at least MIN_DAILY_RATE = 5 as defined by Constants.sol and it is enforced in updateHoldTokenDailyRate, so even owner cannot set otherwise.

Consider this calculation for borrowed 1000USDT (6 decimals):

borrowing.borrowedAmount = 1000
currentDailyRate = 5
Constants.COLLATERAL_BALANCE_PRECISION = 1e18 (or 1 followed by 18 zeros)
1 days in Solidity is equivalent to the number of seconds in a day, which is 86400.
Constants.BP = 10000

result in ceil((borrowing.borrowedAmount * currentDailyRate * Constants.COLLATERAL_BALANCE_PRECISION) / (1 days * Constants.BP)) = 5_000_787_037_037_037_038 which in 18 decimals is equal 5 (lossing the precision). But in case of USDC, it is an unreal value of 5_000_787_037_037 USDC rate per second (the latter value divided by 1e6). So any place where collateral rate is calculated with multiplications, for 8 decimals tokens, it will be 1e10 times too high.

Tool used

Manual Review

Recommendation

Do not use hardcoded precision, instead adjust it to processed tokens e.g. using decimals() value or limit the protocol to be used only with whitelisted, 18 decimals tokens.

Duplicate of #176