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

Incorrect dependecies returned in Laundrette policy

Summary

Laundrette::configureDependencies returns an array of it's Module dependecies incorrectly.

Vulnerability Details

When the policy is forst activated configureDependencies is called. In Laundrette.sol the dependencies are returned incorrectly because the "WEAPN" keycode overwrites the "MONEY" keycode in the position 0 of the dependencies array, this means that during the policy activation this module dependency will not be saved in the moduleDependents mapping in the kernel. This affects the functionality of the _reconfigurePolicies function which is executed when upgrading modules.

Impact

HIGH - Module upgrades for "MONEY" keycode modules are not correctly done for the Laundrette policy, meaning that the EmergencyMigration script doesnt actually change the MoneyShelf module to the MoneyVault module.

Tools Used

Manual review + foundry test:

function testCanWithdrawAfterMigration() public {
// Get 2 gangmembers in
vm.prank(kernel.admin());
kernel.grantRole(Role.wrap("gangmember"), godFather);
vm.startPrank(godFather);
laundrette.addToTheGang(gm1);
laundrette.addToTheGang(gm2);
vm.stopPrank();
// Non gangmember donates money
vm.startPrank(godFather);
usdc.transfer(donor, 100e6);
usdc.transfer(gm1, 100e6);
usdc.transfer(gm2, 100e6);
usdc.approve(address(moneyShelf), 100e6);
vm.stopPrank();
// Donor deposits money
vm.prank(donor);
usdc.approve(address(moneyShelf), 100e6);
laundrette.depositTheCrimeMoneyInATM(donor, donor, 100e6);
// Gangmember 1 deposits money
vm.prank(gm1);
usdc.approve(address(moneyShelf), 100e6);
laundrette.depositTheCrimeMoneyInATM(gm1, gm1, 100e6);
// EMERGENCY MIGRATION
vm.startBroadcast(kernel.executor());
MoneyVault moneyVault = new MoneyVault(kernel, usdc, crimeMoney);
kernel.executeAction(Actions.UpgradeModule, address(moneyVault));
vm.stopBroadcast();
// Gangmember 2 deposits money
vm.prank(gm2);
usdc.approve(address(moneyShelf), 100e6);
vm.expectRevert();
laundrette.depositTheCrimeMoneyInATM(gm2, gm2, 100e6);
}

The last call to the depositTheCrimeMoneyInATM function should revert but it doesn't because it's still calling the MoneyShelf contract instead of MoneyVault.

Recommendations

Change line 26 from:
dependencies[0] = toKeycode("WEAPN");
to
dependencies[1] = toKeycode("WEAPN");

Updates

Lead Judging Commences

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

Laundrette incorrect dependencies

Support

FAQs

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