Cyfrin / 2023-07-foundry-defi-stablecoin

37 stars 32 forks source link

No limit on the amount of allowed feeds can lock funds #1152

Open codehawks-bot opened 1 year ago

codehawks-bot commented 1 year ago

No limit on the amount of allowed feeds can lock funds

Severity

Medium Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/d1c5501aa79320ca0aeaa73f47f0dbc88c7b77e2/src/DSCEngine.sol#L350

Summary

There is no cap to the amount of feeds supported by the engine. This can cause user deposits to be stuck in the engine.

Vulnerability Details

Deposits do not look at the health factor

    function depositCollateral(address tokenCollateralAddress, uint256 amountCollateral)
        public
        moreThanZero(amountCollateral)
        isAllowedToken(tokenCollateralAddress)
        nonReentrant
    {
        s_collateralDeposited[msg.sender][tokenCollateralAddress] += amountCollateral;
        emit CollateralDeposited(msg.sender, tokenCollateralAddress, amountCollateral);
        bool success = IERC20(tokenCollateralAddress).transferFrom(msg.sender, address(this), amountCollateral); 
        if (!success) {
            revert DSCEngine__TransferFailed();
        }
    }

However - when a user wants to redeem their collateral the engine checks the health factor and goes over every supported feed to check the usd price.

    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;
    }

If s_collateralTokens.length is large enough, there might not be enough gas in the block gas limit to possibly execute this function since all collateral feeds are checked even if the user does not have any of that collateral

Impact

Loss of funds

Tools Used

Recommendations

Set a limit of allowed feeds and only check the feeds if the user has more then zero amount of that collateral