Foundry DeFi Stablecoin CodeHawks Audit Contest

Summary

When constructing our DSCEngine we should provide array of token addresses, which could be used as collateral and Chain Link price feed addresses for the exact tokens. However the only check that is currently there is that the number of the tokens is the same as the number of the price feeds. If by any mistake the creator has provided ETH address on first place in addresses array and the corresponding price feed address (ETH / USD) on the second place in price feed array, the engine will be created if the sizes of the arrays match and this will lead to whole broken contract, because calculating given collateral asset using wrong price feed won't provide real data.

Vulnerability Details

It is important to validate such important fields, because this is the logic, which holds the whole application. We have an invariant for the data feed (TOKEN1 / TOKEN2). We want TOKEN2 to always be USD. However there is never check for that, so creator could unintentionally to mismatch price feed address and we won't be aware, until later when a user deposit 1 ETH ($1 800), expecting to be able to mint 900 DSCE ($900), but indeed he would be able to mint less than 0.5, if when wrong pricefeed is set to ETH / BTC for example. This immediately breaks the our coin properties stability and being USD pegged.

Impact

The following example is for mismatching the places of the token addresses and their corresponding price when creating the contract. I think this the scenario, which is with highest probability, but the code is also vulnerable of providing data, which does not returned the value compared to USD.
PoC

1. 1 - Creator of the contract (Bob) want to make the engine work with ETH and BTC as collaterals.

2. 2 - Bob unintentionally deploy the contract providing arrays as follows : tokenAddresses = [ethAddress, btcAddress], priceFeedAddresses = [btcUsdPriceFeed, ethUsdPriceFeed].

3. 3 - This will pass the check if (tokenAddresses.length != priceFeedAddresses.length) {revert DSCEngine__TokenAddressesAndPriceFeedAddressesMustBeSameLength();} , but there is a big problem, because logic that follows is assuming that index x of the tokenAddress array corresponds to the price feed for same token in the price feed array for x.

4. 4 - The contract will be created and inside s_priceFeeds we will have -> [{ethTokenAddress} = {btcUsdPriceFeedAddress}, {btcTokenAddress} = {ethUsdPriceFeedAddress}].

5. 5 - This will result in DSCE token price very different from the main project idea (1 DSCE = 1 USD).

6. 6 - Because when Alice buy 1 ETH for $1 800 from an exchange and want to deposit inside our broken contract, she will be pleasantly surprised. Instead of being able to withdraw DSCE 900, because if we want 200% overcollateralized system and token which is being pegged to the USD, this is the expected value, she will be able to mint DSCE 15 000, because the BTC price feed has returned that for 1 (She provived ETH, but we have linked it to BTC price feed) of the collateral token corresponding prize in USD is $30 000 (For example, the ratios of $1 800 ETH and $30 000 is accurate) and to be able to mint half of the collateral value makes 15 000.

7. 7 - Now she doesn't care if the system will take her collateral worth of $1 800, because she has DSCE worth of $15 000, or no. Because this broken model will change the corresponding price of DSCE and won't be pegged to USD anymore.

Tools Used

Manual Review

Recommendations

Consider checking the ERC20 "symbol()" of the current token and the "description()" of the corresponding price feed while iterating over the array in the constructor: