New Guardian cannot be set

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898/src/AstariaRouter.sol#L349

Vulnerability details

setGuardian is utilised by the previous guardian to set a new guardian but there is a possibility that the old guardian can renounce its status before the new guardian calls __acceptGuardian.

The guardian role will not be transferred to the new guardian since s.newGuardian will be changed to address(0). The condition to be checked here will be invalid. The sequence of calls to __acceptGuardian and renounceGuardian are unknown. Therefore, it will leave the router without a guardian which will prevent critical files from being updated through fileGuardian such as FileType.Implementation, FileType.CollateralToken, FileType.LienToken and FileType.TransferProxy.

Remove this modification to s.newGuardian(https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898/src/AstariaRouter.sol#L349) . Even with this change, there is a possibility that the guardian can renounce its role without even setting a new guardian. The other solution is to make setGuardian as an internal function and add it to the __renounceGuardian.

See this added POC which illustrates this issue:

//test/AstariaTest.t.sol
function testGuardian()public{

  address _guardian =address(1);

ASTARIA_ROUTER.setNewGuardian( _guardian);

 ASTARIA_ROUTER.__renounceGuardian();

 vm.startPrank(_guardian);

//address set as new guardian cannot accept role since it's now set to address(0)

  vm.expectRevert(bytes("")) ;
   ASTARIA_ROUTER.__acceptGuardian();

    vm.stopPrank ();
  }``` 

[c4-judge]

Picodes marked the issue as duplicate of #556

[Picodes]

This is down to the fact that the current guardian can remove the guardian role forever

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)

[c4-judge]

Picodes marked the issue as grade-c