`Frankencoin.denyMinter()` should revert when `block.timestamp >= minters[_minter] `

[code423n4]

Lines of code

https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/Frankencoin.sol#L153

Vulnerability details

Impact

The Frankencoin.denyMinter() function does not correctly check the application period, which allows an FPS holder to deny a minter after the deadline for the application period has passed. As a result, the denied minter would have to be registered as a minter again and pay an extra 1000 zchf as fees.

Proof of Concept

   /**
    * Qualified pool share holders can deny minters during the application period.
    * Calling this function is relatively cheap thanks to the deletion of a storage slot.
    */
   function denyMinter(address _minter, address[] calldata _helpers, string calldata _message) override external {
      if (block.timestamp > minters[_minter]) revert TooLate();
      reserve.checkQualified(msg.sender, _helpers);
      delete minters[_minter];
      emit MinterDenied(_minter, _message);
   }

The denyMinter() function is designed to allow qualified pool share holders to deny minters during the application period. The minters mapping keeps track of the application period for each minter.

Currently, the denyMinter() function only checks if the current block timestamp is greater than the minters[_minter] timestamp. This means that an FPS holder can deny a minter when the current block.timestamp is equal to the minters[_minter] timestamp. This allows an FPS holder to deny a minter after the deadline for the application period has passed, which could potentially affect the security and stability of the system.

Tools Used

Manual Review

Recommended Mitigation Steps

To mitigate this issue, we recommend updating the denyMinter() function as follows:

   function denyMinter(address _minter, address[] calldata _helpers, string calldata _message) override external {
      if (block.timestamp >= minters[_minter]) revert TooLate();
      reserve.checkQualified(msg.sender, _helpers);
      delete minters[_minter];
      emit MinterDenied(_minter, _message);
   }

[c4-pre-sort]

0xA5DF marked the issue as duplicate of #370

[c4-judge]

hansfriese changed the severity to QA (Quality Assurance)

[c4-judge]

hansfriese marked the issue as grade-b

[c4-judge]

This previously downgraded issue has been upgraded by hansfriese

[c4-judge]

hansfriese changed the severity to QA (Quality Assurance)