First Flight #16: Mafia Takedown

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:

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.

2. During the deployment of the contracts in the Deployer contract, grant the gangmember role to the godfather so he is allowed to call addToTheGang.