Foundry DeFi Stablecoin CodeHawks Audit Contest
Cyfrin
DeFiFoundry
15,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Value returned from `getUsdValue()` & `getTokenAmountFromUsd()` does not have consistent precision (decimals) for different tokens

t0x1c

Summary

getUsdValue() returns the USD value using the precision of the token itself, and hance varies based on the token's decimal places. This leads to inconsistent decimal places and causes loss of funds, for example when multiple tokens with different decimal places have been deposited as collateral.

Vulnerability Details

The Chainlink price feed returns price in USD for 1 ETH (or 1 BTC or any other 1 TOKEN) expressed with 8 decimal places for TOKEN/USD pairs. It's seen that currently inside getUsdValue(), the returned USD value is expressed as USD * token decimal places.

Examples -

  1. Considering $2000/ETH, getUsdValue(weth, 15e18) returns 30000e18 - considered to be USD value of 15 wETH.

  2. Considering $1000/BTC, getUsdValue(wbtc, 15e8) returns 15000e8 - considered to be USD value of 15 wBTC. This means a user with wBTC as collateral will be able to mint way less DSC tokens than they are ideally supposed to.

Impact

When multiple tokens are deposited as collateral by a user, their collateral value is calculated incorrectly, hence impacting the health factor and hence affecting the number of DSC tokens that can be minted. Also results in incorrect liquidation and loss of funds, bricking the protocol.

Below is a test to check this:

function testMultipleCollateralTokenTypes() public {
address borrower = makeAddr("borrower");
deal(weth, borrower, 100 ether); // give user 100 wETH
deal(wbtc, borrower, 100e8); // give user 100 wBTC
vm.startPrank(borrower);
ERC20Mock(weth).approve(address(dsce), 10 ether);
ERC20Mock(wbtc).approve(address(dsce), 10e8);
dsce.depositCollateral(weth, 10 ether);
dsce.depositCollateral(wbtc, 10e8);
vm.stopPrank();
​
// expectedUsdValue = (10 * $2000/ETH + 10 * $1000/BTC) expressed with 18 decimal precision = 30000e18
uint256 expectedUsdValue = (10 * 2000 + 10 * 1000) * 1e8;
​
// style 1: calculate via getUsdValue() function
uint256 actualUsdValueDirect = dsce.getUsdValue(weth, 10 ether) + dsce.getUsdValue(wbtc, 10e8);
assertEq(actualUsdValueDirect, expectedUsdValue);
​
// comment the above assertion to run below code
// style 2: calculate via getAccountCollateralValue() function
uint256 actualUsdValueCollateral = dsce.getAccountCollateralValue(borrower);
assertEq(actualUsdValueCollateral, expectedUsdValue);
​
/**
* Above assertions fail with message:
*
* Error: a == b not satisfied [uint]
* Left: 20000000001000000000000
* Right: 30000000000000000000000
*/
}

Run this via forge test --mt testMultipleCollateralTokenTypes -vv.

Tools Used

Manual review, forge test.

Recommendations

Introduce provision in getUsdValue() to maintain same price-feed precision for all tokens. This could be one solution:

// Assumption: Price Feed returned from Chainlink ("price" variable) will always have precision of 8 decimal places in case of USD pairs
​
// remove this
- return ((uint256(price) * ADDITIONAL_FEED_PRECISION) * amount) / PRECISION;
​
// add this
+ return ((uint256(price) * ADDITIONAL_FEED_PRECISION) * amount) / ERC20(token).decimals();




Same correction can be done in getTokenAmountFromUsd(), I suppose:

// remove this
- return (usdAmountInWei * PRECISION) / (uint256(price) * ADDITIONAL_FEED_PRECISION);
​
// add this
+ return (usdAmountInWei * ERC20(token).decimals()) / (uint256(price) * ADDITIONAL_FEED_PRECISION);

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.