Beginner FriendlyDeFiFoundry
100 EXP
View results
Submission Details
Severity: high
Valid

Arbitrary `to` address used

Summary

The function MoneyShelf::depositUSDC uses arbitrary from and to address.

Vulnerability Details

Passing an arbitrary from address to transferFrom (or safeTransferFrom) can lead to loss of funds, because anyone can transfer tokens from the from address if an approval is made.

Impact

If an approval has been made, an attacker can call the MoneyShelf::depositUSDC function and pass the user's address as the account parameter and their address as the to parameter, hence sending user's funds to the MoneyShelf and stealing the tokens the user is meant to recieve for depositing USDC (CrimeMoney tokens).

Proof of Concept

  • User approves MoneyShelf to spend thier USDC

  • Attacker calls the MoneyShelf::depositUSDC function and pass the user's address as the account parameter and their address as the to parameter.

  • Attacker steals users crimeeMoney tokens.

Code
function test_canHijackUserFunds() public {
vm.startPrank(godFather);
usdc.transfer(user, 100e6);
vm.stopPrank();
vm.startPrank(user);
usdc.approve(address(moneyShelf), 100e6);
vm.stopPrank();
vm.startPrank(attacker);
laundrette.depositTheCrimeMoneyInATM(user, attacker, 100e6);
assertEq(usdc.balanceOf(user), 0);
assertEq(usdc.balanceOf(address(moneyShelf)), 100e6);
assertEq(crimeMoney.balanceOf(attacker), 100e6);
assertEq(crimeMoney.balanceOf(user), 0);
}

Tools Used

Slither
Aderyn

Recommendations

Use msg.sender in place of the account and to parameters.

- function depositUSDC(address account, address to, uint256 amount) external {
- deposit(to, amount);
- usdc.transferFrom(account, address(this), amount);
- crimeMoney.mint(to, amount);
+ function depositUSDC(uint256 amount) external {
+ deposit(msg.sender, amount);
+ usdc.transferFrom(msg.sender, address(this), amount);
+ crimeMoney.mint(msg.sender, amount);
}
}
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.