Summary
More validation is needed for the value that affects the price calculation and set only in the constructor.
Vulnerability Details
In the constructor it don’t check tokenAddresses
for duplicate values and push them into the s_collateralTokens
array.
constructor(address[] memory tokenAddresses, address[] memory priceFeedAddresses, address dscAddress) {
if (tokenAddresses.length != priceFeedAddresses.length) {
revert DSCEngine__TokenAddressesAndPriceFeedAddressesMustBeSameLength();
}
for (uint256 i = 0; i < tokenAddresses.length; i++) {
s_priceFeeds[tokenAddresses[i]] = priceFeedAddresses[i];
s_collateralTokens.push(tokenAddresses[i]);
}
i_dsc = DecentralizedStableCoin(dscAddress);
}
https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/d1c5501aa79320ca0aeaa73f47f0dbc88c7b77e2/src/DSCEngine.sol#L112-L123
When using the s_collateralTokens
array, it also does not check for duplicate values. If s_collateralTokens
contains duplicate addresses, it counts as duplicate collateral tokens and is calculated as paying more collateral tokens than actually paid.
function getAccountCollateralValue(address user) public view returns (uint256 totalCollateralValueInUsd) {
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;
}
https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/d1c5501aa79320ca0aeaa73f47f0dbc88c7b77e2/src/DSCEngine.sol#L350-L359
Also, when setting s_priceFeeds
, it does not check that the priceFeed
is actually the feed that should be set. If deployer accidentally set the wrong feed, contract can use the wrong price oracle and calculate the wrong price.
Before setting the feed, it is recommended to call the priceFeed.description
function to check whether is the desired feed.
constructor(address[] memory tokenAddresses, address[] memory priceFeedAddresses, address dscAddress) {
if (tokenAddresses.length != priceFeedAddresses.length) {
revert DSCEngine__TokenAddressesAndPriceFeedAddressesMustBeSameLength();
}
for (uint256 i = 0; i < tokenAddresses.length; i++) {
s_priceFeeds[tokenAddresses[i]] = priceFeedAddresses[i];
s_collateralTokens.push(tokenAddresses[i]);
}
i_dsc = DecentralizedStableCoin(dscAddress);
}
https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/d1c5501aa79320ca0aeaa73f47f0dbc88c7b77e2/src/DSCEngine.sol#L112-L123
Impact
If deployer set a variable incorrectly in the constructor, you can never modify it again because there is no setter function. Since it is a variable that affects the calculation, more confirmation is needed.
Tools Used
vscode
Recommendations
Validate the values of tokenAddresses
and priceFeedAddresses
.