Did not Approve to zero first before changing the allowance

Severity

Medium Risk

Relevant GitHub Links

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

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/0a25c1940ca220686588c4af3ec526f725fe2582/contracts/token/ERC20/ERC20.sol#L136C1-L140C6

Summary

A very famous attack vector is present in ERC20 during changing allowance. Here in DecentralizedStableCoin Allowance is not being set to zero first before changing allowance i.e burning, transferring from allowance

Vulnerability Details

See Summary. Due to this vulnerability approve(spender,amount) can be front run and user will be force to allow extra tokens to attacker

Some other ERC20 tokens like (USDT) do not work when changing the allowance from an existing non-zero allowance value . In these type of tokens approve() function will revert if the current approval is not zero, to protect against front running attack

FIle: "src/DecentralizedStableCoin.sol"

    function approve(address spender, uint256 amount) public virtual override returns (bool) {
            address owner = _msgSender();
            _approve(owner, spender, amount);
            return true;
        }

https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DecentralizedStableCoin.sol#L39 https://github.com/OpenZeppelin/openzeppelin-contracts/blob/0a25c1940ca220686588c4af3ec526f725fe2582/contracts/token/ERC20/ERC20.sol#L136C1-L140C6 https://docs.google.com/document/d/1YLPtQxZu1UAvO9cZ1O2RPXBbT0mooh4DYKjA_jp-RLM/edit

Impact

burnFrom is prone to front run attack and also Attacker will force user to approve more tokens to him than user intend

Tools Used

Manual

Recommendations

Make sure that Allowance is zero during changing the Allowance

Cyfrin / 2023-07-foundry-defi-stablecoin