Cyfrin / 2023-07-foundry-defi-stablecoin

37 stars 32 forks source link

Admin can mint unlimited tokens before ownership transfer #1145

Closed codehawks-bot closed 1 year ago

codehawks-bot commented 1 year ago

Admin can mint unlimited tokens before ownership transfer

Severity

Gas Optimization / Informational

Relevant GitHub Links

https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/d1c5501aa79320ca0aeaa73f47f0dbc88c7b77e2/script/DeployDSC.s.sol#L23-L28

Summary

The deployer of the stablecoin contract can mint unlimited tokens before the ownership is transferred to DSCEngine.

Vulnerability Details

In deployment, the stablecoin token contract is deployed first. The deployer acts as the owner of the stablecoin until the DSCEngine contract is deployed and the ownership is transferred to it.

        // deploy script
        ..........
        vm.startBroadcast(deployerKey);
        DecentralizedStableCoin dsc = new DecentralizedStableCoin();
        DSCEngine engine = new DSCEngine(tokenAddresses, priceFeedAddresses, address(dsc));

        dsc.transferOwnership(address(engine));
        vm.stopBroadcast();
        .......

    // stable coin contract
    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;
    }

Until then the deployer can mint unlimited tokens.

Impact

The deployer can gain a lot of tokens without putting up any collateral. If people are unaware of this, they might put up collateral for receiving the token in which case they will be at a loss.

Tools Used

Manual Review

Recommendations

Being clear in the documentation about this power of the deployer or change the pattern to launch the token contract from the engine contract itself.

PatrickAlphaC commented 1 year ago

This is expected. No one would use it if this was the case. The deploy script intentionally instantly transfers ownership.