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

Laundrette policy is not authorized to call `Kernel::executeAction` function.

vesper

Summary

Laundrette policy is not authorized to call Kernel::executeAction function.

Vulnerability Details

The current admin of gang policy is Laundrette. As admin Laundrette can grant or revoke roles. The purpose of Laundrette::retrieveAdmin is to change the current admin of gang policy. For any reason the god father must be able to change the current admin!

One concrete example : If he wants to prevent gang members to leave the gang.

The implementation of Laundrette::retrieveAdmin doesn't allow the God Father to change the current admin, because only executor can call kernel::executeAction and the executor is God Father.

Laundrette.sol

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

Kernel.sol

// ######################## ~ MODIFIERS ~ ########################
​
// Role reserved for governor or any executing address
modifier onlyExecutor() {
if (msg.sender != executor) revert Kernel_OnlyExecutor(msg.sender);
_;
}
// ######################## ~ KERNEL INTERFACE ~ ########################
​
function executeAction(Actions action_, address target_) external onlyExecutor {
if (action_ == Actions.InstallModule) {
ensureContract(target_);
ensureValidKeycode(Module(target_).KEYCODE());
_installModule(Module(target_));
} else if (action_ == Actions.UpgradeModule) {
ensureContract(target_);
ensureValidKeycode(Module(target_).KEYCODE());
_upgradeModule(Module(target_));
} else if (action_ == Actions.ActivatePolicy) {
ensureContract(target_);
_activatePolicy(Policy(target_));
} else if (action_ == Actions.DeactivatePolicy) {
ensureContract(target_);
_deactivatePolicy(Policy(target_));
} else if (action_ == Actions.MigrateKernel) {
ensureContract(target_);
_migrateKernel(Kernel(target_));
} else if (action_ == Actions.ChangeExecutor) {
executor = target_;
} else if (action_ == Actions.ChangeAdmin) {
admin = target_;
}
​
emit ActionExecuted(action_, target_);
}

Impact

The god father won't be able to change the current admin directly from the policy

Tools Used

Manual review

Foundry unit test

Recommendations

Here’s a revised implementation that should work correctly:

Laundrette.sol: Modify the retrieveAdmin function to check if the caller is the executor and then proceed with the action.

// Laundrette.sol
pragma solidity ^0.8.24;
​
interface IKernel {
function executeAction(Actions action, address target) external;
function executor() external view returns (address);
}
​
enum Actions {
InstallModule,
UpgradeModule,
ActivatePolicy,
DeactivatePolicy,
MigrateKernel,
ChangeExecutor,
ChangeAdmin
}
​
contract Laundrette {
IKernel public kernel;
​
constructor(address _kernel) {
kernel = IKernel(_kernel);
}
​
modifier onlyExecutor() {
require(msg.sender == kernel.executor(), "Laundrette: Only executor can call this function");
_;
}
​
function retrieveAdmin() external onlyExecutor {
kernel.executeAction(Actions.ChangeAdmin, msg.sender);
}
}
Updates

Lead Judging Commences

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

`retrieveAdmin` not working

Support

FAQs

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