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

Funds are not transferred to `MoneyVault` during the migration

paprikrumplikas

Summary

When the MoneyShelf module is upgraded from MoneyShelf to MoneyVault, the USDC funds are not moved to MoneyVault.
This should be handled by EmergencyMigration::migrate, but it misses to move the funds.

Vulnerability Details

The following test demonstrates that after migration, the USDC funds are still in MoneyShelf, not MoneyVault:

Proof of Code
function testMoneyNotMigratedDuringEmergencyMigration() public {
////////////////////////////////////////////////
/// Deposit in original setup, to MoneyShelf ///
////////////////////////////////////////////////
​
uint256 depositAmount = 1e12;
​
vm.startPrank(godFather);
usdc.approve(address(moneyShelf), depositAmount);
laundrette.depositTheCrimeMoneyInATM(godFather, godFather, depositAmount);
vm.stopPrank();
​
assertEq(moneyShelf.getAccountAmount(godFather), depositAmount);
​
////////////////////////////////////////////////
/// Emergency migration ////////////////////////
////////////////////////////////////////////////
​
EmergencyMigration migration;
​
assertEq(address(kernel.getModuleForKeycode(Keycode.wrap("MONEY"))), address(moneyShelf));
​
migration = new EmergencyMigration();
MoneyVault moneyVault = migration.migrate(kernel, usdc, crimeMoney);
​
// check upgade - OK
assertEq(address(kernel.getModuleForKeycode(Keycode.wrap("MONEY"))), address(moneyVault));
​
// correcting another bug
vm.startPrank(godFather);
kernel.executeAction(Actions.DeactivatePolicy, address(laundrette));
kernel.executeAction(Actions.ActivatePolicy, address(laundrette));
vm.stopPrank();
​
// correcting another bug
vm.prank(kernel.admin());
kernel.grantRole(Role.wrap("gangmember"), godFather);
​
// correcting yet another bug
vm.startPrank(godFather);
kernel.executeAction(Actions.ChangeAdmin, godFather);
kernel.grantRole(Role.wrap("moneyshelf"), address(moneyVault));
kernel.executeAction(Actions.ChangeAdmin, address(laundrette));
vm.stopPrank();
​
// check roles - OK
assertEq(kernel.hasRole(address(moneyShelf), Role.wrap("moneyshelf")), true);
​
////////////////////////////////////////////////
/// Post-migration checks and actions //////////
////////////////////////////////////////////////
​
// check balance after migration
assertEq(moneyShelf.getAccountAmount(godFather), depositAmount);
assertEq(usdc.balanceOf(address(moneyShelf)), depositAmount); // moneyShelf still has the deposits
assertEq(usdc.balanceOf(address(moneyVault)), 0); // moneyVault has no balance
​
// cannot withdraw from MoneyVault
vm.prank(godFather);
vm.expectRevert(); // arithmetic underflow or overflow
laundrette.withdrawMoney(godFather, godFather, depositAmount);
}

Impact

USDC funds will remain in the MoneyShelf. This makes the whole migration pointless.

The only way to recover the funds is to upgrade the module MoneyVault back to the original MoneyShelf - provided that MoneyShelf is not already blacklisted by authorities or emptied/controlled by another gang.

Tools Used

Manual reivew, Foundry.

Recommendations

Ensure that USDC funds are moved from the MoneyShelf contract to MoneyVault.
For this, the godfather needs to

  1. withdraw every user's USDC deposit from MoneyShelf by repeatedly calling Laundrette::withdrawMoney;

  2. transfer the USDC directly to MoneyVault (using the standard ERC20 function transfer)

Updates

Lead Judging Commences

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