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

Once the Emergency Migration occurs `Laundrette.sol::withdrawMoney` can still withdraw money from the `MoneyShelf.sol` contract

coffee

Summary

After the Emergency Migration occurs, funds are not protected and they can still be withdrawn from the MoneyShelf.sol contract.

Vulnerability Details

Once the Emergency Migration occurs, funds are supposed to be protected inside of the MoneyVault.sol contract. However, this does not happen. The Laundrette.sol::withdrawMoney function is hard coded to call the moneyShelf.withdrawUSDC function. This means that even after the Emergency Migration, funds can be withdrawn from MoneyShelf.sol.

function withdrawMoney(
address account,
address to,
uint256 amount
)
external
onlyRole("gangmember")
isAuthorizedOrRevert(account)
{
@> moneyShelf.withdrawUSDC(account, to, amount);
}

Impact

The test below passes showing that funds can be withdrawn from MoneyShelf.sol even after the Emergency Migration occurs.

function test_migrateDepositAndWithdrawFromMoneyShelfAfterMigration() public {
vm.prank(godFather);
usdc.transfer(address(this), 100e6);
usdc.approve(address(moneyShelf), 100e6);
laundrette.depositTheCrimeMoneyInATM(address(this), address(this), 100e6);
assertEq(usdc.balanceOf(address(this)), 0);
assertEq(usdc.balanceOf(address(moneyShelf)), 100e6);
assertEq(crimeMoney.balanceOf(address(this)), 100e6);
​
assertEq(address(kernel.getModuleForKeycode(Keycode.wrap("MONEY"))), address(moneyShelf));
​
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));
​
joinGang(address(this));
laundrette.withdrawMoney(address(this), address(this), 100e6);
assertEq(usdc.balanceOf(address(this)), 100e6);
assertEq(usdc.balanceOf(address(moneyShelf)), 0);
assertEq(crimeMoney.balanceOf(address(this)), 0);
}

Tools Used

--Foundry

Recommendations

It is recommended to change the Laundrette.sol::withdrawMoney function to be dynamic and call the Module using the active Keycode "MONEY". This way, after the migration, the correct Module/Contract will be called.

Updates

Lead Judging Commences

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

Emergency migration leave the USDC

Support

FAQs

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