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

`External users` and `GangMembers` can transfer their `CrimeMoney` to alt accounts shielding themselves from withdrawls by `godFather`

codexbugmenot

Description

The MoneyShelf::withdrawUSDC function which is called by Laundrette::withdrawMoney function burns the CrimeMoney of the account arg passed using crimeMoney.burn() and sends the usdc amount from that account to the msg.sender but if the account owner already transfers his CrimeMoney to any alt account then the burn call will always revert.

Impact

External users and GangMembers who donate money to the protocol can effectively shield themselves from withdrawl by godFather

Proof of Concept

PoC to show that godFather cannot withdraw the usdc of accounts whose CrimeMoney is already transferred to alt accounts before and after Emergency Migration.

prerequisites: For the following test to work add a new address address caponeBege = makeAddr("Capone Bege"); in the Base.t.sol

POC : External Users and GangMembers can Shield Themselves from Withdrawls
​
function test_CrimeMoneyTransferCanShieldFromWithdrawls() public {
//Depositing usdc and getting crimeMoney for address(this)
vm.prank(godFather);
usdc.transfer(address(this), 100e6);
usdc.approve(address(moneyShelf), 100e6);
laundrette.depositTheCrimeMoneyInATM(address(this), address(this), 100e6);
assertEq(usdc.balanceOf(address(this)), 0);
assertEq(usdc.balanceOf(address(moneyShelf)), 100e6);
assertEq(crimeMoney.balanceOf(address(this)), 100e6);
moneyShelf.getAccountAmount(address(this));
​
//Transfering crimeMoney to caponeBege
crimeMoney.transfer(caponeBege, 100e6);
​
// we do this to make godFather join the Gang
// but even External users can shield their usdc from being withdrawn by godFather
joinGang(address(this));
​
vm.prank(godFather);
vm.expectRevert();
laundrette.withdrawMoney(address(this), godFather, 100e6);
​
EmergencyMigration migration = new EmergencyMigration();
(MoneyVault moneyVault, Emergency emergency) =
migration.migrate(kernel, usdc, crimeMoney, laundrette, moneyShelf);
​
vm.prank(godFather);
vm.expectRevert();
emergency.withdrawMoney(address(this), godFather, 100e6);
}
​

Recommended Mitigation

Having the crimeMoney.burn in the withdraw function will always cause this issue.

  1. It can be mitigated by overriding the transfer function in CrimeMoney.sol and adding relevant checks to it i.e transfers to non GangMembers require a permisssion from GangMembers or godFather

  2. Another way it can be mitigated is to have the burn in a seperate function,this way the godFather can get the USDC first and can crimemoney.burn later and if it reverts he can be sure that the user has shielded himself and can be dealt accordingly.

Updates

Lead Judging Commences

n0kto Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Transfer CrimeMoney break the protocol/bad account tracking

Support

FAQs

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