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

Gang Members Can Still Withdraw In Emergency Mode

4rdiii

[H-1] Gang Members Can Still Withdraw In Emergency Mode

Description:
When configureDependencies is called via Kernel::_activatePolicy, it then pushes the returned dependencies into a Kernel::moduleDependents mapping. This mapping is used to reconfigure the policy in case any of the modules which policy is dependent on changes. However, in the configureDependencies function, when setting dependencies array to return the MONEY keycode is overwritten by WEAPN keycode.

function configureDependencies() external override onlyKernel returns (Keycode[] memory dependencies) {
dependencies = new Keycode[](2);
​
dependencies[0] = toKeycode("MONEY");
moneyShelf = MoneyShelf(getModuleAddress(toKeycode("MONEY")));
​
@> dependencies[0] = toKeycode("WEAPN");
weaponShelf = WeaponShelf(getModuleAddress(toKeycode("WEAPN")));
}
​

Impact:
This does not affect the functionality of the contracts in normal state. However, when transitioning to emergency mode, the emergency deploy function executes UpgradeModule Action, which requires correct dependencies to work. Since the Laundrette policy is not set as a dependency for the MONEY keycode, upgrading this Keycode won't trigger this policy to reconfigure. As a result, all functions in Laundrette still work as if it is still in normal mode, and gang members still can call WithdrawMoney successfully.

Proof of Concept:
To prove the concept, add the following test to EmergencyMigration.t.sol, Exploit Steps:

  1. gangM1 joins gang.

  2. gangM1 deposits some USDC to test it after emergency.

  3. godfather enters emergency mode via calling migrate.

  4. gangM1 withdraws as if in normal mode.
    PoC:

function testGangMembersCanWithdrawInEmergency() public {
//gangM1 joins gang
address gangM1 = makeAddr("M1");
joinGang(gangM1);
vm.prank(godFather);
usdc.transfer(gangM1, 100e6);
//some deposits for contract to test it after emergency
vm.startPrank(gangM1);
usdc.approve(address(moneyShelf), 100e6);
laundrette.depositTheCrimeMoneyInATM(gangM1, gangM1, 100e6);
vm.stopPrank();
assertEq(usdc.balanceOf(gangM1), 0);
assertEq(usdc.balanceOf(address(moneyShelf)), 100e6);
assertEq(address(kernel.getModuleForKeycode(Keycode.wrap("MONEY"))), address(moneyShelf));
//enter emergency mode:
​
EmergencyMigration migration = new EmergencyMigration();
MoneyVault moneyVault = migration.migrate(kernel, usdc, crimeMoney);
assertNotEq(address(moneyShelf), address(moneyVault));
assertEq(address(kernel.getModuleForKeycode(Keycode.wrap("MONEY"))), address(moneyVault));
​
//gangM1 withdraws in emergency mode:
vm.startPrank(gangM1);
laundrette.withdrawMoney(gangM1, gangM1, 100e6);
assertEq(usdc.balanceOf(gangM1), 100e6);
assertEq(usdc.balanceOf(address(moneyShelf)), 0);
}

Recommended Mitigation:
The fix is simple, just ensure the configureDependencies function returns the correct dependencies array:

function configureDependencies() external override onlyKernel returns (Keycode[] memory dependencies) {
dependencies = new Keycode[](2);
​
dependencies[0] = toKeycode("MONEY");
moneyShelf = MoneyShelf(getModuleAddress(toKeycode("MONEY")));
​
- dependencies[0] = toKeycode("WEAPN");
+ dependencies[1] = toKeycode("WEAPN");
weaponShelf = WeaponShelf(getModuleAddress(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.