Arithmetic underflow when liquidation reward + collateral is greater than under collateralized user's balance

[Ed-Marcavage]

Please let me know if this is a known issue, if not, I will be happy to propose a PR/solution. Here is the finding:

In function liquidate when tokenAmountFromDebtCovered + bonusCollateral is greater than the under-collateralized user's balance (i.e. s_collateralDeposited[from][tokenCollateralAddress]), this causes _redeemCollateral to throw an arithmetic underflow or overflow error here: s_collateralDeposited[from][tokenCollateralAddress]-= amountCollateral;.

If prices drop suddenly, this may prevent liquidations.

Test Case (see @see-here comment):

function testLiquidate() public {
        address USER = makeAddr("user");
        address myLiquidator = makeAddr("myLiquidator");

        uint256 startingCollateral = 10 ether;
        uint256 startingAmountToMint = 10_000 ether;
        uint256 liquidatorCollateral = 20 ether;
        uint256 liquidatorAmountToMint = 10_000 ether;

        vm.startPrank(USER);
        ERC20Mock(weth).mint(USER, startingCollateral);
        ERC20Mock(weth).approve(address(dsce), startingCollateral);
        dsce.depositCollateralAndMintDsc(
            weth,
            startingCollateral,
            startingAmountToMint
        );
        vm.stopPrank();
        // @see-here - when set to 1100e8 it succeeds, but when set to 1099e8 it fails
        int256 ethUsdUpdatedPrice = 1099e8;
        MockV3Aggregator(ethUsdPriceFeed).updateAnswer(ethUsdUpdatedPrice);

        vm.startPrank(myLiquidator);
        ERC20Mock(weth).mint(myLiquidator, liquidatorCollateral);
        ERC20Mock(weth).approve(address(dsce), liquidatorCollateral);
        dsce.depositCollateralAndMintDsc(
            weth,
            liquidatorCollateral,
            liquidatorAmountToMint
        );
        dsc.approve(address(dsce), liquidatorAmountToMint);
        dsce.liquidate(weth, USER, liquidatorAmountToMint);
        vm.stopPrank();
    }

    function _redeemCollateral(
        address from,
        address to,
        address tokenCollateralAddress,
        uint256 amountCollateral
    ) private {
        console.log(
            "s_collateralDeposited[from][tokenCollateralAddress]",
            s_collateralDeposited[from][tokenCollateralAddress]
        );
        console.log("amountCollateral", amountCollateral);
        s_collateralDeposited[from][tokenCollateralAddress] -= amountCollateral;

In the test case above, when ethUsdUpdatedPrice is set to 1100e8 the following variables subtract without issue (in _redeemCollateral): s_collateralDeposited[from][tokenCollateralAddress] -> 10e18 amountCollateral -> ~99e17

However, when ethUsdUpdatedPrice is set to 1099e8, an integer underflow occurs as the reward + collateral is greater than the under-collateralized user's balance

s_collateralDeposited[from][tokenCollateralAddress] -> 10e18 amountCollateral -> 1000.9e16

[PatrickAlphaC]

Yep! This is a known issue with the protocol :)

Great find though! I hope you do the security and auditing course, your eye will do well there.

(Keep in mind, this is a suuppppeerrr minimized dummy stablecoin codebase!)

[Ed-Marcavage]

Thank you! I completed your HH course a while back and the auditing course as well; but realized in the process of the auditing course, I need a deeper understanding of solidity & blockchain SWE principles in general, so I am currently going through the 'Advanced Foundry' course, submitting findings for code hacks first flights, and will rewatch the auditing course again in the near future.

Seriously, thank you so much for all this content. I want to get into smart contract security and software security in general, and your content has been a blessing.