medium

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

Reward

Total

46.94 USDC

1.54 USDC
Selected
2.16 USDC
1.54 USDC
1.54 USDC
1.54 USDC
1.54 USDC
1.54 USDC
1.54 USDC
1.54 USDC
1.54 USDC
1.54 USDC
1.54 USDC
1.54 USDC
1.54 USDC
1.54 USDC
1.54 USDC
1.54 USDC
1.54 USDC
1.54 USDC
1.54 USDC
1.54 USDC
1.54 USDC
1.54 USDC
1.54 USDC
1.54 USDC
1.54 USDC
1.54 USDC
1.54 USDC
1.54 USDC
1.54 USDC
Selected Submission

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

Severity

Medium Risk

Relevant GitHub Links

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

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