Cyfrin / 2023-07-foundry-defi-stablecoin

37 stars 32 forks source link

Unnecessary balance and address checks in `burn()` and `mint()` #1116

Closed codehawks-bot closed 1 year ago

codehawks-bot commented 1 year ago

Unnecessary balance and address checks in burn() and mint()

Severity

Gas Optimization / Informational

Relevant GitHub Links

https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DecentralizedStableCoin.sol#L46-L66

Summary

The burn() and mint() functions in the DecentralizedStableCoin contract contain redundant checks that are already handled by the _burn() and _mint() functions in the ERC20 standard. These excess checks add unnecessary gas costs and complexity to the contract.

Vulnerability Details

The burn() function checks if the burn amount is less than or equal to zero and if the owner's balance is less than the burn amount. Similarly, the mint() function checks if the destination address is the zero address.

function burn(uint256 _amount) public override onlyOwner {
    uint256 balance = balanceOf(msg.sender);
    if (_amount <= 0) {
        revert DecentralizedStableCoin__MustBeMoreThanZero();
    }
    if (balance < _amount) { // <-------------------
        revert DecentralizedStableCoin__BurnAmountExceedsBalance();
    }
    super.burn(_amount);
}

function mint(address _to, uint256 _amount) external onlyOwner returns (bool) {
    if (_to == address(0)) { // <-------------------
        revert DecentralizedStableCoin__NotZeroAddress();
    }
    if (_amount <= 0) { 
        revert DecentralizedStableCoin__MustBeMoreThanZero();
    }
    _mint(_to, _amount);
    return true;
}

Tools Used

Manual review

Recommendations

Consider removing the redundant checks from the burn and mint functions.

function burn(uint256 _amount) public override onlyOwner {
    uint256 balance = balanceOf(msg.sender);
    if (_amount <= 0) {
        revert DecentralizedStableCoin__MustBeMoreThanZero();
    }
    super.burn(_amount);
}

function mint(address _to, uint256 _amount) external onlyOwner returns (bool) {
    if (_amount <= 0) { 
        revert DecentralizedStableCoin__MustBeMoreThanZero();
    }
    _mint(_to, _amount);
    return true;
}
PatrickAlphaC commented 1 year ago

I like the checks, they are more explicit.