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

EmergencyMigration fails to repoint Laudrette to the new MoneyVault contract

mattjenkins

Summary

The dependencies in the module Laundrette are defined incorrectly in the configureDependencies function which is returned to the Kernel. The array is built incorrectly and the first dependency for the keycode MONEY is overwritten with the WEAPN keycode. This has another rather unfortunate side-effect that the module dependencies are then stored incorrectly in the Kernel contract itself. When it comes to a module change, the references stored in the Laundrette do not get updated, because the Laundrette contract does not get informed of the underlying module changes and keeps pointing to the old module implementation.

Vulnerability Details

The creation of the dependencies array within the policy Laundrette.sol has a simple number error when adding in the second dependency which overwrites the first dependency. This means the controlling Kernel contract does not know that the Laundrette module depends on the MONEY module and won't let Laudrrette know when the module is updated.

When the MoneyShelf contract is replaced with MoneyVault, the Kernel triggers a _reconfigurePolicies which calls all the dependent policies and triggers them to refresh their module addresses. Because the Kernel does not know the policy Laudrette uses keycode MONEY as it was overwritten, it does not call the Laundrette.configureDependencies function, and as such, the Laundrette contract remains blissfully unaware that the MoneyShelf contract has been replaced and still points to MoneyShelf.

Impact

After an Emergency Migration, the Laundrette is still using the MoneyShelf contract. This means that gang members can still withdraw their USDC, even though the protocol should have locked them out to be only accessible by the GodFather. Now I am sure that given the emergency situation that caused the EmergencyMigration in the first place, this will result in all the gangmembers trusting in the benevolence of their GodFather and not doing anything rash, but reality dictates a rush for the exits and a draining of USDC from the original MoneyShelf and a glorious mass burning of CrimeMoney.

Tools Used

Here is a test script that can be slotted into the test folder highlighting the issue which can be passed with the fix suggested below.

forge test

// SPDX-License-Identifier: MIT
pragma solidity 0.8.24;
​
import { BaseTest, console } from "test/Base.t.sol";
import { Role, Keycode } from "src/Kernel.sol";
​
import { EmergencyMigration } from "script/EmergencyMigration.s.sol";
import { MoneyShelf } from "src/modules/MoneyShelf.sol";
import { MoneyVault } from "src/modules/MoneyVault.sol";
​
contract EmergencyMigrationTest is BaseTest {
​
function test_EmergencyMigration() public {
joinGang(address(this));
​
// Set up our gangmember and give him some USDC
vm.startPrank(godFather);
usdc.transfer(address(gangmember1), 10e6);
laundrette.addToTheGang(gangmember1);
vm.stopPrank();
​
// Our trusting Gangmember deposits his USDC into the ATM
vm.startPrank(gangmember1);
usdc.approve(address(moneyShelf), 10e6);
laundrette.depositTheCrimeMoneyInATM(gangmember1, gangmember1, 10e6);
vm.stopPrank();
​
// Check we are still using MoneyShelf
assertEq(address(kernel.getModuleForKeycode(Keycode.wrap("MONEY"))), address(moneyShelf));
​
// Raid time. Godfather pulls the plug.
EmergencyMigration migration = new EmergencyMigration();
MoneyVault moneyVault = migration.migrate(kernel, usdc, crimeMoney);
​
// Check we are now using MoneyVault
assertNotEq(address(moneyShelf), address(moneyVault));
assertEq(address(kernel.getModuleForKeycode(Keycode.wrap("MONEY"))), address(moneyVault));
​
// Check to see where the USDC actually is
assertEq(usdc.balanceOf(address(moneyVault)), 0);
assertEq(usdc.balanceOf(address(moneyShelf)), 10e6);
​
// Test that our gangmember can still pull his money out of the laundrette
assertEq(usdc.balanceOf(gangmember1), 0);
vm.startPrank(gangmember1);
vm.expectRevert();
laundrette.withdrawMoney(address(gangmember1), address(gangmember1), 10e6);
vm.stopPrank();
assertEq(usdc.balanceOf(gangmember1), 0);
}
​
}
​

Recommendations

Simply fix the line of code in Laundrette.sol (line26) to

+ dependencies[1] = toKeycode("WEAPN");
- dependencies[0] = toKeycode("WEAPN");
Updates

Lead Judging Commences

n0kto Lead Judge over 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.