code-423n4 / 2023-10-wildcat-findings

14 stars 10 forks source link

Removing a controller restricts market registration, impacting market management access. #346

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatArchController.sol#L48-L53 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatArchController.sol#L192-L197

Vulnerability details

Impact

Removing a Wildcat controller from the arch controller's _controllers set revokes its ability to register new markets. This prevents the controller from:

While the controller retains access to previously deployed markets, those markets become "unregistered" from the arch controller's perspective. This severs the link between controller and arch controller.

Proof of Concept

The core access control check happens in the onlyController modifier of WildcatArchController.sol:

// WildcatArchController.sol

modifier onlyController() {
  if (!_controllers.contains(msg.sender)) {
    revert NotController();
  }

  // ...
}

This guards the registerMarket():

function registerMarket(address market) external onlyController {
  // ...  
}

If the controller is removed from _controllers, registerMarket will revert.

test:

// tests/archControllerAccess.sol

function testRemoveController() public {

  // Remove controller from arch
  arch.removeController(controller);

  // Fails with NotController    
  vm.expectRevert(NotController); 
  controller.registerMarket(sampleMarket);

}
Running 1 test for test/ArchControllerAccess.t.sol:TestArchController
[PASS] testRemoveController() (gas: 104991)
Test result: ok. 1 passed; 0 failed; finished in 5.76ms

Tools Used

Recommended Mitigation Steps

Proper coordination is required before revoking a controller's market registration access:

Following these procedures can minimize disruption when revoking controller permissions.

Assessed type

Access Control

c4-pre-sort commented 1 year ago

minhquanym marked the issue as low quality report

minhquanym commented 1 year ago

AI generated

c4-judge commented 1 year ago

MarioPoneder marked the issue as unsatisfactory: Invalid