search

code-423n4 / 2022-02-jpyc-findings

1 stars 0 forks source link

QA Report #5

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Low

Race condition in configureMinter

Impact

The configureMinter function can be used to set the minterAllowedAmount of the minter. Consider the following scenarios:

  1. The minterAllowedAmount of minter A is 500.
  2. MasterMinter calls configureMinter to adjust minterAllowedAmount of miner A to 1000.
  3. Minter A found the transaction in the pool and he mint 500 tokens via front-running
  4. Then the minterAllowedAmount of minter A is adjusted to 1000.

The minterAllowedAmount of minter A is actually 1500 instead of 1000.

Proof of Concept

https://github.com/code-423n4/2022-02-jpyc/blob/main/contracts/v2/FiatTokenV2.sol#L337-L347 https://github.com/code-423n4/2022-02-jpyc/blob/main/contracts/v1/FiatTokenV1.sol#L327-L337

Tools Used

None

Recommended Mitigation Steps

Add increaseMinterAllowedAmount and decreaseMinterAllowedAmount methods 

    function increaseMinterAllowedAmount(address minter, uint256 increase)external
        whenNotPaused
        onlyMasterMinter
        returns (bool)
        {
            require(minters[minter]);
            minterAllowed[minter] += increase;
            return true;
        }
    function decreaseMinterAllowedAmount(address minter, uint256 decrease)external
        whenNotPaused
        onlyMasterMinter
        returns (bool)
        {
            require(minters[minter]);
            minterAllowed[minter] -= decrease;
            return true;
        }
thurendous commented 2 years ago

About this issue, firstly, we rarely use the function configureMinter. Secondly, we can removeMinter first and then call the configureMinter. Other than that, all the minters are centralised party and they should be under control and regulation. At the moment, the minter is only ourself, so there is no worry about that.