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

`DSCEngine` accepts duplicates in `tokenAddresses` allowing a collateral to be "counted" more than one

stermi

Summary

DSCEngine can be initialized with duplicates in the tokenAddresses input. Because of this, those duplicates collateral will be counted X times when the system calculates the user collateral.

Once consequence is that if the user provides such collateral

  • the HF will be higher compared to the real value

  • the user is allowed to mint more DecentralizedStableCoin compared to what he should be

  • the user won't be liquidated when it should

Vulnerability Details

During the DSCEngine constructor, the tokenAddresses input parameter is looped and each of those tokens are added to s_collateralTokens. Because there's no sanity checks on those values, the system could be initialized with duplicates of the same collateral token.

The getAccountCollateralValue function is the function used to calculate the amount of USD that the user has provided as collateral. If the user has provided some collateral of such duplicate token, such collateral will be counted X-1 times more than it should.

The totalCollateralValueInUsd returned by the function is then used by the system to determine if:

  • The user can mint the requested amount of DecentralizedStableCoin

  • The user can be liquidated

  • Other HF checks in the system

Let's take the first case just to show it

  • Let's assume that the system is initialized s_collateralTokens = [weth, weth, wbtc]

  • Let's also assume that 1 ETH = 2000 USD

  1. Alice supplies 1 WETH via depositCollateral(weth, 1 ether). This would allow Alice to borrow at max 1000 DecentralizedStableCoin before being liquidated

  2. Because the weth token is counted twice by getAccountCollateralValue the system thinks that Alice has provided 4000 USD worth of collateral (instead of 2000 USD)

  3. Because of this, Alice can mint twice the amount of DecentralizedStableCoin tokens

Impact

User collateral will be counted X times (X = number of duplicates for each collateral token). Because of this

  • user can mint more DecentralizedStableCoin token compared to what he should be able to

  • HF will be higher compared to the real one

  • user will not be liquidated when he should

Tools Used

Manual + Test

// SPDX-License-Identifier: MIT
​
pragma solidity 0.8.19;
​
import {DSCEngine} from "../../src/DSCEngine.sol";
import {DecentralizedStableCoin} from "../../src/DecentralizedStableCoin.sol";
import {HelperConfig} from "../../script/HelperConfig.s.sol";
import {ERC20Mock} from "@openzeppelin/contracts/mocks/ERC20Mock.sol";
import {Test, console} from "forge-std/Test.sol";
import {StdCheats} from "forge-std/StdCheats.sol";
​
contract DoubleCollateralTest is StdCheats, Test {
HelperConfig public helperConfig;
​
address public ethUsdPriceFeed;
address public btcUsdPriceFeed;
address public weth;
address public wbtc;
uint256 public deployerKey;
​
uint256 amountCollateral = 10 ether;
uint256 amountToMint = 100 ether;
address public user = address(1);
​
uint256 public constant STARTING_USER_BALANCE = 10 ether;
uint256 public constant MIN_HEALTH_FACTOR = 1e18;
uint256 public constant LIQUIDATION_THRESHOLD = 50;
​
​
function setUp() external {
helperConfig = new HelperConfig();
​
(ethUsdPriceFeed, btcUsdPriceFeed, weth, wbtc, deployerKey) = helperConfig.activeNetworkConfig();
ERC20Mock(weth).mint(user, STARTING_USER_BALANCE);
ERC20Mock(wbtc).mint(user, STARTING_USER_BALANCE);
}
​
function deploySystem(bool doubleCountWETH) public returns (DSCEngine, DecentralizedStableCoin) {
​
DecentralizedStableCoin dsc = new DecentralizedStableCoin();
DSCEngine dsce;
​
if( doubleCountWETH) {
address[] memory tokenAddresses = new address[](3);
address[] memory priceFeedAddresses = new address[](3);
​
​
tokenAddresses[0] = weth;
tokenAddresses[1] = weth;
tokenAddresses[2] = wbtc;
​
priceFeedAddresses[0] = ethUsdPriceFeed;
priceFeedAddresses[1] = ethUsdPriceFeed;
priceFeedAddresses[2] = btcUsdPriceFeed;
​
dsce = new DSCEngine(tokenAddresses, priceFeedAddresses, address(dsc));
} else {
​
address[] memory tokenAddresses = new address[](2);
address[] memory priceFeedAddresses = new address[](2);
​
tokenAddresses[0] = weth;
tokenAddresses[1] = wbtc;
​
priceFeedAddresses[0] = ethUsdPriceFeed;
priceFeedAddresses[1] = btcUsdPriceFeed;
​
dsce = new DSCEngine(tokenAddresses, priceFeedAddresses, address(dsc));
}
​
dsc.transferOwnership(address(dsce));
​
if (block.chainid == 31337) {
vm.deal(user, STARTING_USER_BALANCE);
}
​
return (dsce, dsc);
}
​
function testHF() public {
// 1 ETH = 2000 USD
// normally by providing 1 ETH I should be able to borrow 1000 DSC Token
​
// Because my collateral are counted twice I can borrow twice as much because my HF does not count it right
​
(DSCEngine dsceOk, DecentralizedStableCoin dscOk) = deploySystem(false);
(DSCEngine dsceWrong, DecentralizedStableCoin dscWrong) = deploySystem(true);
​
// supply 1 ether, I should be able to borrow 1000 tokens
vm.startPrank(user);
​
// supply 1 ether
ERC20Mock(weth).approve(address(dsceOk), 1 ether);
dsceOk.depositCollateral(weth, 1 ether);
​
// borrow 1001 token reverts
vm.expectRevert();
dsceOk.mintDsc(1001 ether);
​
// borrow 1000 token success
dsceOk.mintDsc(1000 ether);
​
​
// If we try to do the same with the other one we can mint more than we should because our WETH supply is counted TWICE as collateral
// supply 1 ether
ERC20Mock(weth).approve(address(dsceWrong), 1 ether);
dsceWrong.depositCollateral(weth, 1 ether);
​
// borrow 1001 token reverts
dsceWrong.mintDsc(1001 ether);
​
vm.stopPrank();
​
// Even if we have supplied the same amount (1 WETH) but borrowed more (1001 tokens compared to 1000)
// the user's HF in dsceWrong is HIGHER compared to the one from dsceOk
​
uint256 hfFromDSCEOk = dsceOk.getHealthFactor(user);
uint256 hfFromDSCEWrong = dsceWrong.getHealthFactor(user);
​
assertGt(hfFromDSCEWrong, hfFromDSCEOk);
​
}
}

Recommendations

The DSCEngine.constructor should have more sanity check to be sure to initialize the system properly

  • tokenAddresses.length should be > 0

  • tokenAddresses should not contain duplicates

  • tokenAddresses[i] is not equal to address(0)

  • priceFeedAddresses should not contain duplicates

  • priceFeedAddresses[i] is not equal to address(0)

  • Each priceFeedAddresses should be tested to check if the price they provide at the moment is not stale

  • dscAddress is not equal to address(0)

Support

FAQs

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