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

Godfather can't withdraw funds from `MoneyVault` and anyone can send USDC into the `MoneyVault` breaks protocol invariant and lead to stuck funds

Mill

Summary

According to the Documentation "Only the GodFather can withdraw and no one can deposit in this contract", but actually anyone can still deposit USDC and the GodFather can't withdraw anything from this contract leading to stuck funds.

Vulnerability Details

Everyone can send USDC into this contract but no one can actually withdraw the funds anymore so all tokens are stuck.

Proof of Code

Code
function test_donationAndWithdrawAfterMigration() public {
uint256 depositBalance = 50e6;
​
vm.startPrank(godFather);
usdc.transfer(testAccount, depositBalance);
vm.stopPrank();
​
// We do a migration
EmergencyMigration migration = new EmergencyMigration();
MoneyVault moneyVault = migration.migrate(kernel, usdc, crimeMoney);
​
vm.startPrank(testAccount);
// This breaks the first invariant "no one can deposit in this contract"
usdc.transfer(address(moneyVault), depositBalance);
assertEq(usdc.balanceOf(address(moneyVault)), depositBalance);
vm.stopPrank();
​
// The GodFather needs the gangmember role again because only gangmember can interact with the laundrette
vm.prank(kernel.admin());
kernel.grantRole(Role.wrap("gangmember"), godFather);
// now the GodFather wants to withdraw
vm.startPrank(godFather);
// This doesn't work because of the policy protection in the Shelf contract
vm.expectRevert();
moneyVault.withdrawUSDC(godFather, godFather, depositBalance);
// This doesn't work as well because no balance was recorded when the testAddress send USDC directly to the
// Vault
vm.expectRevert();
laundrette.withdrawMoney(testAccount, godFather, depositBalance);
vm.stopPrank();
}

Impact

No one is able to withdraw USDC from the MoneyVault leading to stuck funds. If the protocol had a correct implementation of moving funds as described in the other issue: "Funds doesn't move from MoneyShelf to MoneyVault after EmergencyMigration causing the assets be unprotected", then all funds would be stuck in the MoneyVault contract forever. Also the invariant " no one can deposit in this contract" is broken and if someone deposits into this contract the funds are stuck again.

Tools Used

  • forge test

Recommendations

It's not necessary for the protocol that no one can deposit USDC in this contract because the GodFather will be the only one who can withdraw anyways.

The MoneyVault should let the Godfather withdraw directly without calling the Laundrette, so we need these adjustments:

  1. Because the Godfather is the only one who should receive USDC we can check for msg.sender and remove the address account and address to from the function.

  2. We check if the msg.sender is equal to the executor

  3. We can skip the withdraw function because we don't need to update the balance of the accounts that deposited, we only want the money.

  4. We don't need to burn the crimeMoney token because this has no value right ? We only care about getting the USDC.

  5. We check that the the input amount doesn't exceed the available balance.

  6. Then we transfer to the msg.sender which is the Godfather.

- function withdrawUSDC(address account, address to, uint256 amount) external {
- require(to == kernel.executor(), "MoneyVault: only GodFather can receive USDC");
- withdraw(account, amount);
- crimeMoney.burn(account, amount);
- usdc.transfer(to, amount);
}
​
+ function withdrawUSDC(uint256 amount) external {
+ require(msg.sender == kernel.executor(), "MoneyVault: only GodFather can receive USDC");
+ require(amount <= usdc.balanceOf(address(this)),"MoneyVault: you can only withdraw what's available")
+ usdc.transfer(msg.sender, amount);
+ }
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.