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

The MoneyVault::withdrawUSDC function cannot be called directly and no Policy has been implemented resulting in funds not being accessible after MoneyVault deployment.

poolig4n

Description
The framework used to develop the MoneyVault and the other contracts in the Protocol requires that all end-user functions are exposed via a Policy contract. There is no Policy Contract implemented that allows the call to MoneyVault::withdrawUSDC to be executed.

Impact
Funds are locked in the MoneyVault and cannot be accessed by the Godfather, contradicting the documentation and the intent of the MoneyVault:

MoneyVault:
Only the GodFather can withdraw and no one can deposit in this contract.

Proof of Concept
File: EmergencyMigration.t.sol

+ MoneyVault public moneyVault;
​
function test_migrate() public {
assertEq(address(kernel.getModuleForKeycode(
Keycode.wrap("MONEY"))), address(moneyShelf));
​
+ //Ensure we have some money locked away before the migration
+ vm.prank(godFather);
+ usdc.transfer(address(this), 1);
+ usdc.approve(address(moneyShelf), 1);
+ laundrette.depositTheCrimeMoneyInATM(address(this), address(this), 1);
+ assertEq(usdc.balanceOf(address(this)), 0);
+ assertEq(usdc.balanceOf(address(moneyShelf)), 1);
+ assertEq(crimeMoney.balanceOf(address(this)), 1);
​
EmergencyMigration migration = new EmergencyMigration();
- MoneyVault moneyVault = migration.migrate(kernel, usdc, crimeMoney);
+ moneyVault = migration.migrate(kernel, usdc, crimeMoney);
​
assertNotEq(address(moneyShelf), address(moneyVault));
assertEq(address(kernel.getModuleForKeycode(
Keycode.wrap("MONEY"))), address(moneyVault));
}
​
​
+ function test_withdrawMoneyFails() public {
+ vm.prank(godFather);
+ vm.expectRevert();
+ moneyVault.withdrawUSDC(address(this), godFather, 1);
+ }
​

Update the EmergencyMigration.t.sol test file as shown above and run the test.
forge test --match-path test/EmergencyMigration.t.sol

Recommended mitigation

  • Implement a Policy for the MoneyVault so MoneyVault functions can be called

  • Implement robust testing of the MoneyVault functions

References
The Protocol Documentation states: This project uses the Default framework

Tools Used

  • Manual Review

Updates

Lead Judging Commences

n0kto Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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