0xRobocop - getPriceUSD() function at the StableOracleDAI.sol returns an incorrect value

[sherlock-admin]

0xRobocop

high

getPriceUSD() function at the StableOracleDAI.sol returns an incorrect value

Summary

See Vulnerability Detail

Vulnerability Detail

The function getPriceUSD() at the StableOracleDAI.sol contract is supposed to return how much USD, 1 DAI is worth, this is computed with the following formula:

return
   (wethPriceUSD * 1e18) /
   ((DAIWethPrice + uint256(price) * 1e10) / 2);

Which seems correct, but a wrong assumption on the value of price makes it incorrect. Lets analyze it.

wethPriceUSD is computed as: uint256 wethPriceUSD = ethOracle.getPriceUSD(); which quoting the interface it should return how much USD, 1 ETH is worth with 18 decimals.

DAIWethPrice is computed as:

uint256 DAIWethPrice = DAIEthOracle.quoteSpecificPoolsWithTimePeriod(
      1000000000000000000, // 1 Eth
      0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2, // WETH (base token)
      0x6B175474E89094C44Da98b954EedeAC495271d0F, // DAI (quote token)
      pools, // DAI/WETH pool uni v3
      600 // period
);

Again, quoting the interface, quoteSpecificPoolsWithTimePeriod returns:

Amount of quoteToken received for baseAmount of baseToken.

In this case quoteToken is DAI, so it returns how much DAI, 1 ETH is worth with 18 decimals.

And lastly price is computed as:

// chainlink price data is 8 decimals for WETH/USD, so multiply by 10 decimals to get 18 decimal fractional
//(uint80 roundID, int256 price, uint256 startedAt, uint256 timeStamp, uint80 answeredInRound) = priceFeedDAIETH.latestRoundData();
(, int256 price, , , ) = priceFeedDAIETH.latestRoundData();

The error is here, the developer assumes that this price returned by chainlink corresponds to how much DAI, 1 ETH is worth with 8 decimals.

In reality the value returned is the opposite, it returns how much ETH, 1 DAI is worth and it has 18 decimals. This can be confirmed putting the address used in the contract which is 0x773616E4d11A78F511299002da57A0a94577F1f4 in Etherscan and calling decimals() and latestRoundData().

Impact

In this case the price of DAI in USD will be under estimated causing a loss for the user who mints USSD using DAI as collateral.

Code Snippet

https://github.com/sherlock-audit/2023-05-USSD/blob/6d7a9fdfb1f1ed838632c25b6e1b01748d0bafda/ussd-contracts/contracts/oracles/StableOracleDAI.sol#L50-L52

Tool used

Manual Review

Recommendation

Inverse the value returned by chainlink, and remember that it has 18 decimals.

Or better use the chainlink price feed for DAI / USD which is 0xAed0c38402a5d19df6E4c03F4E2DceD6e29c1ee9

Duplicate of #102

[0xRobocop]

Escalate for 10 USDC

This is not a duplicate of #236

#236 Talks about an incorrect assumption in the decimals returned by the chainlink oracle, which is only one part of the problem. And this can be seen in the mitigation recommended by the Watson to remove * 1e10.

My issue identified another incorrect assumption which is not the decimals returned, but which price is returned, the contract expects to consume the price to be how much DAI, 1 ETH is worth (DAI / ETH). But the chainlink feed actually returns how much ETH, 1 DAI is worth (ETH / DAI). This can be confirmed putting the address used in the contract which is 0x773616E4d11A78F511299002da57A0a94577F1f4 in Etherscan and calling latestRoundData().

return
   (wethPriceUSD * 1e18) /
   ((DAIWethPrice + uint256(price) * 1e10) / 2);

The incorrect assumption can also be seen in the above formula, it adds DAIWethPrice and price thinking price is also DAI / WETH, but in reality is WETH / DAI. So the mitigation is not only to adjust to the correct decimals, but to inverse the value returned by the chainlink feed.

[sherlock-admin]

Escalate for 10 USDC

This is not a duplicate of #236

#236 Talks about an incorrect assumption in the decimals returned by the chainlink oracle, which is only one part of the problem. And this can be seen in the mitigation recommended by the Watson to remove * 1e10.

My issue identified another incorrect assumption which is not the decimals returned, but which price is returned, the contract expects to consume the price to be how much DAI, 1 ETH is worth (DAI / ETH). But the chainlink feed actually returns how much ETH, 1 DAI is worth (ETH / DAI). This can be confirmed putting the address used in the contract which is 0x773616E4d11A78F511299002da57A0a94577F1f4 in Etherscan and calling latestRoundData().

return
   (wethPriceUSD * 1e18) /
   ((DAIWethPrice + uint256(price) * 1e10) / 2);

The incorrect assumption can also be seen in the above formula, it adds DAIWethPrice and price thinking price is also DAI / WETH, but in reality is WETH / DAI. So the mitigation is not only to adjust to the correct decimals, but to inverse the value returned by the chainlink feed.

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[ctf-sec]

Duplicate of #102

[hrishibhat]

Result:
High
Duplicate of #102

[sherlock-admin]

Escalations have been resolved successfully!

Escalation status:

• 0xRobocop: accepted