The Laundrette
contract provides a function retrieveAdmin()
which is meant to be used by the godfather to gain admin status in the system. This can be necessary when the godfather needs the privileges to grant and revoke roles, as only the Kernel::admin
is allowed to do that. However, Laundrette::retrieveAdmin
is not able to change the admin because it doesn't have the executor status.
There are types of roles in the mafia takedown contracts:
Built-in roles, of which there are Kernel::admin
and Kernel::executor
Custom roles, which are created via Kernel::grantRole
What's important to note here, is that only Kernel::admin
is able to grant and revoke roles. Kernel::executor
on the other hand, has the ability to execute actions via Kernel::executeAction
, such as Actions.InstallModule
, Actions.ActivatePolicy
, but also actions related to built-in roles, such as Actions.ChangeAdmin
and Actions.ChangeExecutor
.
When the system is deployed, the Laundrette
contracts becomes the admin while the godfather becomes the executor:
This means, Laundrette
can grant and revoke roles, godFather
can execute actions.
The Laundrette::retrieveAdmin
function, which is meant to be used by the godfather to gain admin status, makes use of Kernel::executeAction
to change the admin:
As we've learned however, Laundrette
has to be the executor to call Kernel::executeAction
. Only the godfather can call Kernel::executeAction
.
This function will revert no matter who calls it.
There's no serious impact here because the only thing that happens is that the function will always revert. The godfather can just call Kernel::executeAction(Actions.ChangeAdmin, Kernel::executor)
directly and gains admin status like that.
Manual review
Foundry for tests
For this to work, Laundrette
would need to become the executor as well. In addition, the account used in Kernel::executeAction
needs to be parametrized. The godfather can then use the ChangeExecutor
action to make Laundrette
the executor, which would make Laundrette::retrieveAdmin
work again.
However, this is not recommended because at that point, Laundrette
would be the executor and there's no way to give that status to a different account. In addition, this would require additional access control though because that would allow anyone to call this function with any account to become a new admin.
Instead, I'd recommend to just remove Laundrette::retrieveAdmin
altogether and simply let the executor call Kernel::executeAction
directly to make any changes to Kernel::admin
.
Below is a test that can be added to Laundrette.t.sol
to verify that Laundrette::retrieveAdmin
always fails:
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.