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

Duplicated values in the tokenAddresses and priceFeedAddresses pair will lead to incorrect collateral value calculation.

1nc0gn170

Summary

There is no check in the constructor to detect duplicate entries in the tokenAddresses and priceFeedAddresses pair.

constructor(address[] memory tokenAddresses, address[] memory priceFeedAddresses, address dscAddress) {
// USD Price Feeds
if (tokenAddresses.length != priceFeedAddresses.length) {
revert DSCEngine__TokenAddressesAndPriceFeedAddressesMustBeSameLength();
}
// For example ETH / USD, BTC / USD, MKR / USD, etc
for (uint256 i = 0; i < tokenAddresses.length; i++) {
s_priceFeeds[tokenAddresses[i]] = priceFeedAddresses[i];
s_collateralTokens.push(tokenAddresses[i]);
}
i_dsc = DecentralizedStableCoin(dscAddress);
}

Vulnerability Details

Having duplicate entries can result in an incorrect collateral value calculation in the function getAccountCollateralValue. For example, if there are n duplicate entries, the resultant collateral value will be n times the actual value.

function getAccountCollateralValue(address user) public view returns (uint256 totalCollateralValueInUsd) {
// Loop through each collateral token, get the amount they have deposited, and map it to
// the price, to get the USD value
for (uint256 i = 0; i < s_collateralTokens.length; i++) {
address token = s_collateralTokens[i];
uint256 amount = s_collateralDeposited[user][token];
totalCollateralValueInUsd += getUsdValue(token, amount);
}
return totalCollateralValueInUsd;
}

As this total collateral value is used in checking health, an increase in value will result in a better health rating than the actual health. For instance, let's consider the following scenario when attempting to mint 1000 ether DSC for 1 ether collateral:

No duplicates - 0.5555e18 (Considered as bad health)

Single duplicate - 1.111e18 (Considered as good health)

Two duplicates - 1.6665e18 (Considered as good health).

POC

function testIncorrectCollateralCalculation() public {
uint256 amountCollateral = 1 ether;
uint256 amountToMint = 10 ether;
​
​
address owner = msg.sender;
vm.prank(owner);
​
MockV3Aggregator ethUsdPriceFeedMock = new MockV3Aggregator(
8,
1111e8
);
ethUsdPriceFeed = address(ethUsdPriceFeedMock);
​
MockMoreDebtDSC mockDsc = new MockMoreDebtDSC(ethUsdPriceFeed);
​
​
// @audit Duplicate address pairs
tokenAddresses = [weth, weth, weth];
feedAddresses = [ethUsdPriceFeed, ethUsdPriceFeed, ethUsdPriceFeed];
​
DSCEngine mockDsce = new DSCEngine(
tokenAddresses,
feedAddresses,
address(mockDsc)
);
mockDsc.transferOwnership(address(mockDsce));
​
vm.startPrank(user);
ERC20Mock(weth).approve(address(mockDsce), amountCollateral);
mockDsce.depositCollateralAndMintDsc(weth, amountCollateral, amountToMint);
vm.stopPrank();
​
uint256 userBalance = mockDsc.balanceOf(user);
assertEq(userBalance, 10 ether);
​
uint256 collateralValue = mockDsce.getAccountCollateralValue(user);
assertEq(collateralValue, 1111 ether);
​
// Running 1 test for test/unit/DSCEngineTest.t.sol:DSCEngineTest
// [FAIL. Reason: Assertion failed.] testIncorrectCollateralCalculation() (gas: 2646913)
// Logs:
// Error: a == b not satisfied [uint]
// Left : 3333000000000000000000
// Right: 1111000000000000000000
​
// Test result: FAILED. 0 passed; 1 failed; finished in 5.52ms
}

Impact

This issue directly interferes with the health computation and can produce unintended results in the evaluation of the health status which may help to bypass the checks to perform certain actions.

Tools Used

Foundry, Remix.

Recommendations

Check for duplicates while adding to the list.

Support

FAQs

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