First Flight #16: Mafia Takedown
First Flight #16
Beginner FriendlyDeFiFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: low
Valid

Godfather cannot reclaim `Kernel` admin rights via `Laundrette::retrieveAdmin`

paprikrumplikas

Summary

Godfather cannot reclaim admin rights for the Kernel contract, as a call to Laundrette::retrieveAdmin will always fail.

Vulnerability Details

By default, the Kernel contract recognizes two distinguished roles: admin and executor, both of which are set to the address of the deployer of Kernel:

constructor() {
executor = msg.sender;
admin = msg.sender;
}

adminis later changed to the address of Laundrette by Deployer::deploy:

kernel.executeAction(Actions.ChangeAdmin, address(laundrette));

With this, Godfather loses access to functions Kernel::grantRole and Kernel::revokeRole which are both gated by the onlyAdmin modifier.

Laundrette::retrieveAdmin should supposedly allow Godfather to reclaim admin rights :

function retrieveAdmin() external {
kernel.executeAction(Actions.ChangeAdmin, kernel.executor());
}

However, a call to this function will always fail. The call chain would be as follows:

  1. Godfather calls Laundrette::retrieveAdmin.

  2. Laundrette calls kernel.executeAction(Actions.ChangeAdmin, kernel.executor());

  3. Kernel::executeAction's onlyExecutor modifier restricts its use to msg.sender == executor.

  4. Even though Godfather is the executor and the originator of the call flow is him, in Kernel's context the msg.sender is not the Godfather but Laundrette.
    (msg.sender is a dynamic variable and is always the address that directly initiates a given call to a contract).

The following piece of test demonstrates that

  • Godfather is not the admin of Kernel

  • He cannot reclaim admin rights via Laundrette::retrieveAdmin

  • He can reclaim admin rights via a direct call to Kernel::executeAction

  • By doing so, he breaks other Laundrette functionality like Laundrette:addToGang and Laundrette::quitTheGang

Proof of Code
function testGodfatherCannotReclaimKernelRightsViaLaundrette() public {
// Godfather is not the Kernel admin
assert(godFather != kernel.admin());
// Laundrette contract is the Kernel admin
assert(address(laundrette) == kernel.admin());
// Godfather is the kernel executor
assert(godFather == kernel.executor());
​
// Godfather fails to reclaim admin rights via `Laundrette`, as Kernel recognizes `Laundrette˛the msg.sender
vm.prank(godFather);
bytes memory expectedRevertReason = abi.encodeWithSignature("Kernel_OnlyExecutor(address)", laundrette);
vm.expectRevert(expectedRevertReason);
laundrette.retrieveAdmin();
​
// Godfather can reclaim admin rights via kernel.executeAction
vm.startPrank(godFather);
kernel.executeAction(Actions.ChangeAdmin, kernel.executor());
vm.stopPrank();
​
assert(godFather == kernel.admin());
​
// But this breaks other Laundrette functionality
vm.startPrank(godFather);
// first grants himself gangmember role directly via a call to kernel
kernel.grantRole(Role.wrap("gangmember"), godFather);
expectedRevertReason = abi.encodeWithSignature("Kernel_OnlyAdmin(address)", laundrette);
// kernel recognizes laundrette as the msg.sender which is not the admin anymore -> revert
vm.expectRevert(expectedRevertReason);
laundrette.quitTheGang(godFather);
vm.stopPrank();
}

Impact

The impact is limited:

  • although Godfather cannot reclaim admin rights to Kernel via Laundrette::retrieveAdmin, he can still do so by executing kernel.executeAction(Actions.ChangeAdmin, godFather);`;

  • under normal circumstances, Godfather should not be the admin anyways, because that breaks other Laundrette functionality like Laundrette:addToGang and Laundrette::quitTheGang. Probably the only meaningful use of this function would be for circumventing another bug (where Godfather has no gangmember role and cannot access Laundrette functions reserved for this role.

Tools Used

Manual reivew, Foundry.

Recommendations

Revisit the need for need for Laundrette::retrieveAdmin. If all other bugs are fixed, this function might not be needed:

- function retrieveAdmin() external {
- kernel.executeAction(Actions.ChangeAdmin, kernel.executor());
- }
Updates

Lead Judging Commences

n0kto Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

`retrieveAdmin` not working

GodFather is not a gang member

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.