Foundry DeFi Stablecoin CodeHawks Audit Contest

Summary

getUsdValue() assumes and that the token's USD feed's decimals to be 8. However, there are certain token's USD feed such AMPL/USD: https://etherscan.io/address/0xe20CA8D7546932360e37E9D72c1a47334af57706#readContract has a different decimals = 18

Vulnerability Details

aware of description from contest says

"This project is meant to be a stablecoin where users can deposit WETH and WBTC in exchange for a token that will be pegged to the USD.The system is meant to be such that someone could fork this codebase, swap out WETH & WBTC for any basket of assets they like."

and the fact that the function getUsdValue() assumes and that the token's USD feed's decimals to be 8

 function getUsdValue(address token, uint256 amount) public view returns (uint256) {//@audit check decimals 
    AggregatorV3Interface priceFeed = AggregatorV3Interface(s_priceFeeds[token]);
    (, int256 price,,,) = priceFeed.staleCheckLatestRoundData();
    // 1 ETH = $1000
    // The returned value from CL will be 1000 * 1e8
    return ((uint256(price) * ADDITIONAL_FEED_PRECISION) * amount) / PRECISION; //@audit-info ADDITIONAL_FEED_PRECISION = 1e10; PRECISION = 1e18;
}

this vulnerability lead to given price wrong

run a test by demostration

first we go to setup the amplUsdPriceFeed

  // SPDX-License-Identifier: MIT

   pragma solidity ^0.8.18;

  import {Script} from "forge-std/Script.sol";
  import {MockV3Aggregator} from "../test/mocks/MockV3Aggregator.sol";
  import {ERC20Mock} from "@openzeppelin/contracts/mocks/ERC20Mock.sol";

 contract HelperConfig is Script {
struct NetworkConfig {
        address wethUsdPriceFeed;
        address wbtcUsdPriceFeed;
/*+++*/ address amplUsdPriceFeed;
        address weth;
        address wbtc;
        uint256 deployerKey;
}

uint8 public constant DECIMALS = 8;
int256 public constant ETH_USD_PRICE = 2000e8;
int256 public constant BTC_USD_PRICE = 1000e8;
uint256 public DEFAULT_ANVIL_KEY = 0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80;

NetworkConfig public activeNetworkConfig;

constructor() {
    if (block.chainid == 11155111) {
        activeNetworkConfig = getSepoliaEthConfig();
    } else {
        activeNetworkConfig = getOrCreateAnvilEthConfig();
    }
}

function getSepoliaEthConfig() public view returns (NetworkConfig memory) {
    return NetworkConfig({
        wethUsdPriceFeed: 0x694AA1769357215DE4FAC081bf1f309aDC325306,
        wbtcUsdPriceFeed: 0x1b44F3514812d835EB1BDB0acB33d3fA3351Ee43,
      /*+++*/  amplUsdPriceFeed: address(0),
               weth: 0xdd13E55209Fd76AfE204dBda4007C227904f0a81,
              wbtc: 0x8f3Cf7ad23Cd3CaDbD9735AFf958023239c6A063,
             deployerKey: vm.envUint("PRIVATE_KEY")
    });
}

function getOrCreateAnvilEthConfig() public returns (NetworkConfig memory) {
    if (activeNetworkConfig.wethUsdPriceFeed != address(0)) {
        return activeNetworkConfig;
    }

    vm.startBroadcast();
    MockV3Aggregator ethUsdPriceFeed = new MockV3Aggregator(
        DECIMALS,
        ETH_USD_PRICE
    );
    ERC20Mock wethMock = new ERC20Mock("WETH", "WETH", msg.sender, 1000e8);

    MockV3Aggregator btcUsdPriceFeed = new MockV3Aggregator(
        DECIMALS,
        BTC_USD_PRICE
    );

       /*+++*/  MockV3Aggregator amplUsdPriceFeed = new MockV3Aggregator(
      /*+++*/  18,
      /*+++*/  3e18//price
    );
    ERC20Mock wbtcMock = new ERC20Mock("WBTC", "WBTC", msg.sender, 1000e8);
    vm.stopBroadcast();

    return NetworkConfig({
        wethUsdPriceFeed: address(ethUsdPriceFeed),
        wbtcUsdPriceFeed: address(btcUsdPriceFeed),
/*+++*/ amplUsdPriceFeed: address(amplUsdPriceFeed),
        weth: address(wethMock),
        wbtc: address(wbtcMock),
        deployerKey: DEFAULT_ANVIL_KEY
    });
}
}

then the DeployDSC

    contract DeployDSC is Script {
        address[] public tokenAddresses;
        address[] public priceFeedAddresses;

        function run() external returns (DecentralizedStableCoin, DSCEngine, HelperConfig) {
            HelperConfig config = new HelperConfig();

            (address wethUsdPriceFeed,
            address wbtcUsdPriceFeed,
            address amplUsdPriceFeed,//++++++++
            address weth,
            address wbtc,
            uint256 deployerKey) = config.activeNetworkConfig();

            tokenAddresses = [weth, wbtc];
            // priceFeedAddresses = [wethUsdPriceFeed, wbtcUsdPriceFeed];
            priceFeedAddresses = [amplUsdPriceFeed, wbtcUsdPriceFeed]; //++++++++++

            vm.startBroadcast(deployerKey);
            DecentralizedStableCoin dsc = new DecentralizedStableCoin();
            DSCEngine engine = new DSCEngine(tokenAddresses, priceFeedAddresses, address(dsc));

            dsc.transferOwnership(address(engine));
            vm.stopBroadcast();
            return (dsc, engine, config);
        }
    }

then the setup in DSCEngineTest.t.sol

function setUp() external {
    DeployDSC deployer = new DeployDSC();
    (dsc, dsce, helperConfig) = deployer.run();
    (ethUsdPriceFeed,
     btcUsdPriceFeed,
     amplUsdPriceFeed,//+++++++++++++++++
    weth,
    wbtc,
    deployerKey) = helperConfig.activeNetworkConfig();

    if (block.chainid == 31337) {
        vm.deal(user, STARTING_USER_BALANCE);
    }
    // Should we put our integration tests here?
    // else {
    //     user = vm.addr(deployerKey);
    //     ERC20Mock mockErc = new ERC20Mock("MOCK", "MOCK", user, 100e18);
    //     MockV3Aggregator aggregatorMock = new MockV3Aggregator(
    //         helperConfig.DECIMALS(),
    //         helperConfig.ETH_USD_PRICE()
    //     );
    //     vm.etch(weth, address(mockErc).code);
    //     vm.etch(wbtc, address(mockErc).code);
    //     vm.etch(ethUsdPriceFeed, address(aggregatorMock).code);
    //     vm.etch(btcUsdPriceFeed, address(aggregatorMock).code);
    // }
    ERC20Mock(weth).mint(user, STARTING_USER_BALANCE);
    ERC20Mock(wbtc).mint(user, STARTING_USER_BALANCE);
}

and run this test in DSCEngineTest.t.sol

function testGetUsdValue() public {
    uint256 ethAmount = 1e18; 
    // 1e18 weth for ampl when the value of ampl is 3e18
    uint256 usdValue = dsce.getUsdValue(weth, ethAmount);
   console.log("value:",usdValue);
}

the resul

Running 1 test for test/unit/DSCEngineTest.t.sol:DSCEngineTest
[PASS] testGetUsdValue() (gas: 29414)
Logs:
  value: 30000000000000000000000000000

Test result: ok. 1 passed; 0 failed; finished in 12.75s

Impact

this can break the entire protocol

Tools Used

manual review

Recommendations

add

  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++) {

   +--> require(priceFeedAddresses[i].decimals()==8);
        s_priceFeeds[tokenAddresses[i]] = priceFeedAddresses[i];
        s_collateralTokens.push(tokenAddresses[i]);
    }
    i_dsc = DecentralizedStableCoin(dscAddress);
}