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

USDC Transfer Issue in Emergency Migration from MoneyShelf to MoneyVault

josh4324

Summary

The EmergencyMigration script (EmergencyMigration.s.sol) successfully migrates the MoneyShelf contract to the MoneyVault contract during emergencies. However, there is a critical issue where the USDC tokens held in the MoneyShelf contract are not transferred to the MoneyVault contract during the migration process. This oversight can result in the USDC funds remaining in the MoneyShelf contract, leaving them vulnerable and defeating the purpose of the migration.

Vulnerability Details

According to the documentation, the purpose of the migration from the MoneyShelf contract to the MoneyVault contract is to protect the funds from legal or criminal threats. The migration process is handled by the EmergencyMigration.s.sol script, which successfully transitions the contract state. However, the actual USDC balance is not transferred from MoneyShelf to MoneyVault, causing a significant vulnerability where the funds remain in the less secure MoneyShelf contract.

Vulnerability Details:
Location: EmergencyMigration.s.sol

Issue: USDC tokens are not transferred from the MoneyShelf contract to the MoneyVault contract during the migration.

Type: Incomplete migration process, leading to asset protection failure.

contract EmergencyMigration is Script {
address public godFather;
​
function run() public {
// To set at the deployment, no leak of addresses before.
Kernel kernel = Kernel(address(0));
IERC20 usdc = IERC20(address(0));
CrimeMoney crimeMoney = CrimeMoney(address(0));
​
migrate(kernel, usdc, crimeMoney);
}
​
// In case of any issue, the GodFather call this function to migrate the money shelf to a contract that only him
// can manage the money.
// This function has to success in any case to protect the money.
function migrate(Kernel kernel, IERC20 usdc, CrimeMoney crimeMoney) public returns (MoneyVault) {
vm.startBroadcast(kernel.executor());
MoneyVault moneyVault = new MoneyVault(kernel, usdc, crimeMoney);
kernel.executeAction(Actions.UpgradeModule, address(moneyVault));
vm.stopBroadcast();
​
// Once the problem is solved, GodFather migrate to a new contract and redistribute manually
// all the money to gang members thanks to event monitoring and his accountant.
​
return moneyVault;
}
}

Impact

This vulnerability can lead to the following issues:

  1. Funds Vulnerability: USDC tokens remain in the MoneyShelf contract, making them susceptible to risks that the migration was meant to mitigate.

  2. Incomplete Migration: The main objective of migrating funds to a more secure contract (MoneyVault) is not achieved, nullifying the protective measures.

Proof of Concept

  1. GodFather deploys migration script, the script does not contain any method to transfer the USDC inside the moneyShelf contract

  2. Migration script runs successfully but the USDC is not moved into the moneyVault contract

Proof of Code

function test_migrate() public {
assertEq(address(kernel.getModuleForKeycode(Keycode.wrap("MONEY"))), address(moneyShelf));
​
address add1 = makeAddr("add1");
address add2 = makeAddr("add2");
​
vm.prank(godFather);
laundrette.addToTheGang(add1);
​
vm.prank(godFather);
laundrette.addToTheGang(add2);
​
vm.prank(godFather);
usdc.transfer(add1, 100e6);
vm.prank(godFather);
usdc.transfer(add2, 100e6);
​
vm.prank(add1);
usdc.approve(address(moneyShelf), 100e6);
​
vm.prank(add2);
usdc.approve(address(moneyShelf), 100e6);
​
vm.prank(add2);
laundrette.depositTheCrimeMoneyInATM(add2, add2, 100e6);
​
vm.prank(add1);
laundrette.depositTheCrimeMoneyInATM(add1, add1, 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));
​
console.log("moneyshelf",usdc.balanceOf(address(moneyShelf)));
console.log("moneyvault",usdc.balanceOf(address(moneyVault)));
​
assert(usdc.balanceOf(address(moneyShelf)) == 200e6);
assert(usdc.balanceOf(address(moneyVault)) != 200e6);
}

Tools Used

Manual Review

Recommendations

Include USDC Transfer in Migration Script:

Ensure that the script transfers all USDC tokens from MoneyShelf to MoneyVault during the migration process.

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.