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

During `EmergencyMigration`, USDC stays in `MoneyShelf` and is not transfered to `MoneyVault`

wetzo

Summary

When MoneyShelf is migrated to MoneyVault, the USDC balance stays in MoneyShelf instead of being transferred to MoneyVault. This means that the God Father won't be able to withdraw from MoneyVault and the USDC will stay stuck in the replaced contract.

Vulnerability Details

Adding this test to EmergencyMigration.t.sol, it fails when asserting that the USDC balance moved to the new contract address, demonstrating that the USDC balance stays linked to the old contract.

function test_migrateWithdraw() public {
assertEq(address(kernel.getModuleForKeycode(Keycode.wrap("MONEY"))), address(moneyShelf));
​
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(moneyShelf.getAccountAmount(address(this)), 100e6);
​
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));
assertEq(usdc.balanceOf(address(moneyShelf)), 0);
assertEq(usdc.balanceOf(address(moneyVault)), 100e6);
​
joinGang(address(this));
vm.expectRevert("MoneyVault: only GodFather can receive USDC");
laundrette.withdrawMoney(address(this), address(this), 100e6);
​
vm.startPrank(godFather);
laundrette.withdrawMoney(address(this), address(godFather), 100e6);
vm.stopPrank();
assertEq(usdc.balanceOf(address(godFather)), 100e6);
assertEq(usdc.balanceOf(address(moneyShelf)), 0);
assertEq(crimeMoney.balanceOf(address(godFather)), 0);
}

Impact

During migration, the USDC balance is lost. It becomes impossible to withdraw it.

Tools Used

Forge test

Recommendations

The USDC could be sent to the Kernel instead of moneyShelf (but the Kernel can also migrate and USDC transfer should be addressed in this case), or transferred during migration.

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.