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

Funds from `moneyShelf` during migration won't be transferred to `moneyVault`

vesper

Summary

Funds from moneyShelf during migration won't be transferred to moneyVault

Vulnerability Details

Funds Locked in the Old Contract:

  • If funds are not transferred from the old contract (moneyShelf) to the new MoneyVault, they might remain locked in the old contract. This could render the funds inaccessible or difficult to retrieve.

Scenario:

  • During an AMA-led dismantling operation, mafia funds can be confiscated in their entirety.

Impact

Financial Loss:

  • The most immediate and obvious impact is the potential financial loss if funds remain locked in the old contract or are improperly transferred.

Tools Used

Manual review

Recommendations

Automated Fund Transfer Implementation

  1. Modify the MoneyVault Contract

  • Add in MoneyVault a function to receive the funds.

moneyVault refactorisation
// SPDX-License-Identifier: MIT
pragma solidity 0.8.24;
​
import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import { CrimeMoney } from "src/CrimeMoney.sol";
import { Kernel, Keycode } from "src/Kernel.sol";
import { Shelf } from "src/modules/Shelf.sol";
​
// Emergency money vault to store USDC and CrimeMoney
contract MoneyVault is Shelf {
IERC20 private usdc;
CrimeMoney private crimeMoney;
​
constructor(Kernel kernel_, IERC20 _usdc, CrimeMoney _crimeMoney) Shelf(kernel_) {
usdc = _usdc;
crimeMoney = _crimeMoney;
}
​
function KEYCODE() public pure override returns (Keycode) {
return Keycode.wrap("MONEY");
}
​
function depositUSDC(address, address, uint256) external pure {
revert("MoneyVault: depositUSDC is disabled");
}
​
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);
}
​
+ // New function to receive funds during migration
+ function receiveFunds(uint256 amount) external {
+ usdc.transferFrom(msg.sender, address(this), amount);
+ }
}
​

  1. Modify the migrate Function

  • Update the migrate function to automate the transfer of funds from the moneyShelf contract to the new MoneyVault contract.

migrate refactorisation
// SPDX-License-Identifier: MIT
pragma solidity 0.8.24;
​
import { Kernel } from "src/Kernel.sol";
import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import { CrimeMoney } from "src/CrimeMoney.sol";
import { MoneyVault } from "path/to/MoneyVault.sol";
​
contract MafiTakeDown {
function migrate(Kernel kernel, IERC20 usdc, CrimeMoney crimeMoney, address moneyShelf) public returns (MoneyVault) {
vm.startBroadcast(kernel.executor());
​
// Create new MoneyVault
MoneyVault moneyVault = new MoneyVault(kernel, usdc, crimeMoney);
​
// Execute upgrade to the new MoneyVault
kernel.executeAction(Actions.UpgradeModule, address(moneyVault));
​
+ // Transfer funds from moneyShelf to the new MoneyVault
+ uint256 balance = usdc.balanceOf(moneyShelf);
+ approveMigration(address(moneyVault), balance);
+ require(usdc.transferFrom(moneyShelf, address(moneyVault), balance), "Transfer failed");
​
vm.stopBroadcast();
​
return moneyVault;
}
}
​

  1. Adding approval in moneyShelf

moneyShelf refactorisation
// SPDX-License-Identifier: MIT
pragma solidity 0.8.24;
​
import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import { CrimeMoney } from "src/CrimeMoney.sol";
​
import { Kernel, Keycode } from "src/Kernel.sol";
​
import { Shelf } from "src/modules/Shelf.sol";
​
contract MoneyShelf is Shelf {
IERC20 private usdc;
CrimeMoney private crimeMoney;
​
constructor(Kernel kernel_, IERC20 _usdc, CrimeMoney _crimeMoney) Shelf(kernel_) {
usdc = _usdc;
crimeMoney = _crimeMoney;
}
​
function KEYCODE() public pure override returns (Keycode) {
return Keycode.wrap("MONEY");
}
​
// "permissioned" already used by Shelf functions
function depositUSDC(address account, address to, uint256 amount) external {
deposit(to, amount);
usdc.transferFrom(account, address(this), amount);
crimeMoney.mint(to, amount);
}
​
function withdrawUSDC(address account, address to, uint256 amount) external {
withdraw(account, amount);
crimeMoney.burn(account, amount);
usdc.transfer(to, amount);
}
​
+ function approveMigration(address migrator, uint256 amount) external {
+ usdc.approve(migrator, amount);
+ }
}
​
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.