code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-04-frankencoin/blob/1022cb106919fba963a89205d3b90bf62543f68f/contracts/MintingHub.sol#L115-L118

Vulnerability details

Impact

Once position is created and registered with the contract address. Any valid minter can assign those postion to themselves. by doing this, the functionalities that uses the validPos modifier can be broken.

Proof of Concept

Whenever an new position is created or cloned, it would be registered by the created address.

https://github.com/code-423n4/2023-04-frankencoin/blob/1022cb106919fba963a89205d3b90bf62543f68f/contracts/MintingHub.sol#L107

https://github.com/code-423n4/2023-04-frankencoin/blob/1022cb106919fba963a89205d3b90bf62543f68f/contracts/MintingHub.sol#L128

In above two calls, msg.sender would be the MintingHub contract.

when we see the registerPosition, MintingHub contract is registered.

 function registerPosition(address _position) override external {
  if (!isMinter(msg.sender)) revert NotMinter();
  positions[_position] = msg.sender;

}

MintingHub is also the permissioned minter.

But , the contract can add valid minter using the suggestMinter function.

https://github.com/code-423n4/2023-04-frankencoin/blob/1022cb106919fba963a89205d3b90bf62543f68f/contracts/Frankencoin.sol#L83-L90

if that is the case, anyone who is valid minter can call the registerPosition function with valid position address and assign the postion to themselves.

Tools Used

Manual review

Recommended Mitigation Steps

When position is already has valid address, do not overwrite it.

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as low quality report

0xA5DF commented 1 year ago

Minters are trusted, might be a QA

c4-sponsor commented 1 year ago

luziusmeisser marked the issue as sponsor disputed

c4-judge commented 1 year ago

hansfriese changed the severity to QA (Quality Assurance)