code-423n4 / 2023-04-frankencoin-findings

5 stars 4 forks source link

Can't pause or remove a minter #230

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

The project is supposed to be self-governing. Token owners are able to suggest new minters and collateral. The auction mechanism allows participants to remove collateral from the system if it's deemed unhealthy. But, there's no way to remove a registered minter.

Why would you want to remove a registered minter? Because they can have bugs that could break the whole system. The current minter, MintingHub, for example, implements a novel auction mechanism to price collateral instead of choosing the industry standard Chainlink oracles. While I support the idea of having a fully decentralized system, it does add additional risk to the project. A risk that's taken not only by the protocol team but every ZCHF holder as well.

Obviously, these are just hypotheticals. But, the system is not equipped to handle a scenario where the minter malfunctions.

Proof of Concept

After a minter is suggested you have generally 10 days to deny it. After that, there's no way to remove it:

   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);
   }

Tools Used

none

Recommended Mitigation Steps

Implement the ability for token holders to temporarily pause a minter as well as remove it altogether.

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as primary issue

luziusmeisser commented 1 year ago

I have thought about the ability to remove old minters, but decided against it.

Instead, experimental minters should come with their own limits (time, pause function, volume limits, etc.). They are free to include that. Minters that do not include it, can be expected to be denied unless they have been really thoroughly audited.

luziusmeisser commented 1 year ago

So in fact, it is possible to pause a minter assuming the mintuer supports that functionality.

c4-sponsor commented 1 year ago

luziusmeisser marked the issue as sponsor disputed

c4-sponsor commented 1 year ago

luziusmeisser marked the issue as sponsor acknowledged

c4-judge commented 1 year ago

hansfriese marked the issue as selected for report