Too many DSC tokens can get minted for fee-on-transfer tokens.

[codehawks-bot]

Too many DSC tokens can get minted for fee-on-transfer tokens.

Severity

Medium Risk

Relevant GitHub Links

2023-07-foundry-defi-stablecoin/src/DSCEngine.sol

Lines 149 to 161 in d1c5501

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

Summary

The DSCEngine contract overcalculates the collateral when operating with fee-on-transfer tokens, which can lead to too many DSC tokens being minted.

Vulnerability Details

The competition description mentions that while the first use-case for the system will be a WETH/WBTC backed stablecoin, the system is supposed to generally work with any collateral tokens. If one or more collateral tokens are fee-on-transfer tokens, i.e., when transferring X tokens, only X - F tokens arrive at the recipient side, where F denotes the transfer fee, depositors get credited too much collateral, meaning more DSC tokens can get minted, which leads to a potential depeg.

The root cause is the depositCollateral function in DSCEngine:

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

As can be seen in line

bool success = IERC20(tokenCollateralAddress).transferFrom(msg.sender, address(this), amountCollateral);

the contract assumes that the full amountCollateral is received, which might not be the case with fee-on-transfer tokens.

Impact

When the contract operates with fee-on-transfer tokens as collateral, too many DSC tokens can get minted based on the overcalculated collateral, potentially leading to a depeg.

Tools Used

None

Recommendations

Check the actual amount of received tokens:

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