Summary
A call to Shelf's withdraw function casues MoneyVault::withdrawUSDC to fail.
Vulnerability Details
Consider the `MoneyVault::withdrawUSDC˙function:
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);
}
Within the function, withdraw(account, amount); would serve the purpose of updating the internal accounting regarding the depositied USDC balance of the caller of the function. However, according to the internal accounting of MoneyVault, everybody has a zero balance of USDC:
MoneyVault USDC balance does not and cannot come from deposits (that the internal accounting would track), but from a direct transfer (or transfers) from the godfather (that internal accounting does not track).
Given that as per the internal accounting of MoneyVault everybody has a zero balance of USDC, any calls to MoneyVault::withdrawUSDC will revert at line ``withdraw(account, amount);` with an arithmetic under-/overflow error.
This is demonstarted by the following test:
Proof of Code
function testCannotWithdrawFromMoneyVault() public {
EmergencyMigration migration;
assertEq(address(kernel.getModuleForKeycode(Keycode.wrap("MONEY"))), address(moneyShelf));
migration = new EmergencyMigration();
MoneyVault moneyVault = migration.migrate(kernel, usdc, crimeMoney);
vm.startPrank(godFather);
kernel.executeAction(Actions.DeactivatePolicy, address(laundrette));
kernel.executeAction(Actions.ActivatePolicy, address(laundrette));
vm.stopPrank();
vm.startPrank(godFather);
kernel.executeAction(Actions.ChangeAdmin, godFather);
kernel.grantRole(Role.wrap("moneyshelf"), address(moneyVault));
kernel.executeAction(Actions.ChangeAdmin, address(laundrette));
vm.stopPrank();
assertEq(kernel.hasRole(address(moneyVault), Role.wrap("moneyshelf")), true);
uint256 savedFunds = 1e6;
deal(address(usdc), address(moneyVault), savedFunds);
uint256 crimeFunds = savedFunds;
deal(address(crimeMoney), godFather, crimeFunds);
vm.prank(kernel.admin());
kernel.grantRole(Role.wrap("gangmember"), godFather);
vm.startPrank(godFather);
vm.expectRevert();
laundrette.withdrawMoney(godFather, godFather, savedFunds);
vm.stopPrank();
}
}
Impact
USDC will be stuck in MoneyVault, even the godfather cannot withdraw it.
Tools Used
Manual review, Foundry.
Recommendations
Modify MoneyVault as follows:
// 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
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);
}
}
