Beginner FriendlyDeFiFoundry
100 EXP
View results
Submission Details
Severity: low
Valid

`Laundrette::addToTheGang` fails when the godfather doesn't have the role "gangmember" assigned from the beginning

Summary

The addToTheGang function can only be called by the godfather account (also called executor) and accounts with the gangmember role. However the godfather doesn't have the gangmember role right from the beginning when the contracts get deployed, resulting in an error Kernel_OnlyAdmin when he calls this function.
Additional to this issue, the only one who can call Kernel::grantRole is the Admin and in this case the Laundrette contract. The Laundrette contract can't call addToTheGang because it's not the executor it can't call the Kernel functions directly.

Vulnerability Details

It's an implementation error, that causes confusion and the function to revert when the godfather doesn't have the role and increases the code complexity. The godfather would need to claim the admin role first before he can grant the gangmember role to himself which allows him to call the function.

Proof of Code

Paste the following code into the Laundrette.t.sol file, then run the test:

Code
address testAccount = makeAddr("test");
function test_addToGang() public {
vm.startPrank(godFather);
laundrette.addToTheGang(testAccount);
assertEq(kernel.hasRole(testAccount, Role.wrap("gangmember")), true);
}
forge test --mt test_addToGang -vvv

Impact

The function Laundrette::addToTheGang will fail when the godfather calls it the first time.
This also increases the code complexity.

Tools Used

  • forge test

Recommendations

There are two approaches to this but I'd recommend the first one.

  1. Remove the onlyRole("gangmember") modifier from the addToTheGang function because in my opinion the godfather should be the only account who is allowed to add members to the gang and therefore this additional check is not needed.

- function addToTheGang(address account) external onlyRole("gangmember") isGodFather { }
+ function addToTheGang(address account) external isGodFather { }
  1. During the deployment of the contracts in the Deployer contract, grant the gangmember role to the godfather so he is allowed to call addToTheGang.

function deploy() public returns (Kernel, IERC20, CrimeMoney, WeaponShelf, MoneyShelf, Laundrette) {
.
.
.
kernel.grantRole(Role.wrap("moneyshelf"), address(moneyShelf));
+ kernel.grantRole(Role.wrap("gangmember"), godFather);
.
.
.
}
Updates

Lead Judging Commences

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

GodFather is not a gang member

Support

FAQs

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