Potential Arithmetic Underflow in redeemCollateral Function Due to Insufficient Pre-Withdrawal Checks

Severity

Low Risk

Relevant GitHub Links

https://github.com/Cyfrin/foundry-defi-stablecoin-f23/blob/e37b7a7e481c25c0bb14edfccc0234c1956b6a8b/src/DSCEngine.sol#L264

Summary

A potential arithmetic underflow in the _redeemCollateral function of DSCEngine.sol could lead to unintended transaction reverts.

Vulnerability Details

The _redeemCollateral function in DSCEngine.sol decreases the amount of collateral deposited by a user:

s_collateralDeposited[from][tokenCollateralAddress] -= amountCollateral;

However, there are currently no checks to ensure that amountCollateral is less than or equal to s_collateralDeposited[from][tokenCollateralAddress] before performing the subtraction. If amountCollateral exceeds the deposited collateral, an arithmetic underflow occurs, causing the transaction to revert.

Impact

This issue presents a low severity risk. While no funds are at risk and the contract state cannot be manipulated due to the automatic underflow protection provided by the Solidity 0.8.x compiler, users attempting to redeem more collateral than they have deposited will experience a transaction revert. This could lead to confusion and a negative user experience.

Tools Used

This issue was identified through manual code review and fuzz testing.

Recommendations

Improve the user experience and contract predictability by introducing a custom error and a modifier to ensure the amountCollateral is less than or equal to the user's deposited collateral:

error DSCEngine__InsufficientCollateralDeposited(uint256 available, uint256 required);

modifier ensureSufficientCollateral(address from, address tokenCollateralAddress, uint256 amountCollateral) {
    if (s_collateralDeposited[from][tokenCollateralAddress] < amountCollateral) {
        revert InsufficientCollateralDeposited(s_collateralDeposited[from][tokenCollateralAddress], amountCollateral);
    }
    _;
}

function _redeemCollateral(/* parameters */) ensureSufficientCollateral(from, tokenCollateralAddress, amountCollateral) {
    // Function logic
    s_collateralDeposited[from][tokenCollateralAddress] -= amountCollateral;
}

This ensureSufficientCollateral modifier checks whether the amountCollateral exceeds the user's deposited collateral before execution of the function. If the check fails, the transaction reverts with a clear error message indicating the available and required collateral amounts.

Cyfrin / 2023-07-foundry-defi-stablecoin