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

Insecure recipient address handling in `Laundrette::withdrawMoney` leads to potential misallocation of tokens

Mill

Summary

The withdrawMoney function allows withdrawing USDC from the moneyShelf contract where the source account and the recipient address can differ. This design flaw can lead to accidentally sending the USDC tokens to the wrong recipient.

Vulnerability Details

The function withdrawMoney calls moneyShelf.withdrawUSDC, which takes three parameters: account, to, and amount. The issue arises because the withdrawMoney function allows the account parameter (the source from which to withdraw the funds) to be different from the to parameter (the recipient of the funds). If an incorrect to address is provided, the user may end up losing all of their tokens.

Proof of Code

Paste the following code into the Laundrette.t.sol file and then run the script:

Code
function test_withdrawToWrongRecipient() public {
uint256 startingBalance = 100e6;
uint256 depositBalance = 50e6;
​
address gangMemberTwo = makeAddr("Gang Member Two");
​
// First the godfather needs the gangmember role in order to add members to the gang
vm.prank(kernel.admin());
kernel.grantRole(Role.wrap("gangmember"), godFather);
​
vm.startPrank(godFather);
laundrette.addToTheGang(gangMember);
laundrette.addToTheGang(gangMemberTwo);
usdc.transfer(gangMember, startingBalance);
usdc.transfer(gangMemberTwo, startingBalance);
vm.stopPrank();
​
// The first gang member makes a deposit
vm.startPrank(gangMember);
usdc.approve(address(moneyShelf), depositBalance);
laundrette.depositTheCrimeMoneyInATM(gangMember, gangMember, depositBalance);
assertEq(usdc.balanceOf(gangMember), startingBalance - depositBalance);
assertEq(crimeMoney.balanceOf(gangMember), depositBalance);
assertEq(usdc.balanceOf(address(moneyShelf)), depositBalance);
vm.stopPrank();
​
// The second gang member makes a deposit
vm.startPrank(gangMemberTwo);
usdc.approve(address(moneyShelf), depositBalance);
laundrette.depositTheCrimeMoneyInATM(gangMemberTwo, gangMemberTwo, depositBalance);
assertEq(usdc.balanceOf(gangMemberTwo), startingBalance - depositBalance);
assertEq(crimeMoney.balanceOf(gangMemberTwo), depositBalance);
// This is the second deposit so the balance should be depositBalance * 2
assertEq(usdc.balanceOf(address(moneyShelf)), depositBalance * 2);
vm.stopPrank();
​
// now the first gang member wants to withdraw but puts in the wrong `to` address
vm.startPrank(gangMember);
laundrette.withdrawMoney(gangMember, gangMemberTwo, depositBalance);
vm.stopPrank();
​
// Now we check the balances
// The first gang member burned his crime money
assertEq(crimeMoney.balanceOf(gangMember), 0);
// The gang member would expect that he received the money but he did not
assertFalse(usdc.balanceOf(gangMember) == startingBalance);
// instead the second gang member received it
assertEq(usdc.balanceOf(gangMemberTwo), startingBalance);
// and the moneyShelf only holds the money of the second gang member
// since we made a withdraw
assertEq(usdc.balanceOf(address(moneyShelf)), depositBalance);
}
forge test --mt test_withdrawToWrongRecipient -vvv

Impact

The vulnerability introduces the risk of misallocating tokens, potentially resulting in financial loss for users. If an incorrect recipient address (to parameter) is provided when calling the withdrawMoney function, the tokens may be sent to the wrong destination. This can lead to confusion, loss of funds, and potential disputes among users.

Tools Used

  • forge test

Recommendations

Remove the address to argument from the withdrawMoney function, because the only person who should be able to receive USDC should be the sender of the transaction (which also needs to be a gangmember) or the godFather.

function withdrawMoney(
address account,
- address to,
uint256 amount
)
external
onlyRole("gangmember")
isAuthorizedOrRevert(account)
{
- moneyShelf.withdrawUSDC(account, to, amount);
+ moneyShelf.withdrawUSDC(account, account, amount);
}
​
Updates

Lead Judging Commences

n0kto Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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