Summary
No price to trade
Vulnerability Details
The vulnerability is in the SantasList::buyPresent function in the SantasList.sol contract starting at line 172.
The function should exchange 2e18 SantasTokens for NFT. In this function the exchange is for 1e18 tokens.
function buyPresent(address presentReceiver) external {
i_santaToken.burn(presentReceiver);
_mintAndIncrement();
}
Impact
The logic of this contract is violated. If you have 2e18 tokens, you can exchange them for 2 NFT.
Tools Used
forge
Recommendations
Working Test Case
The following test exchanges 1e18 tokens for 1 NFT. When executed, this test will pass, demonstrating that the user can exchange 1e18 tokens for 1 NFT:
function testBuyPresent() public {
vm.startPrank(address(santasList));
santaToken.mint(user);
vm.stopPrank();
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
vm.startPrank(user);
assertEq(santaToken.balanceOf(user), 1e18);
santasList.buyPresent(user);
assertEq(santaToken.balanceOf(user), 0);
assertEq(santasList.balanceOf(user), 1);
vm.stopPrank();
}
Run the test:
forge test --mt testBuyPresent -vvvv
Which yields the following output:
Running 1 test for test/unit/SantasListTest.t.sol:SantasListTest
[PASS] testBuyPresent() (gas: 106807)
Traces:
[111020] SantasListTest::testBuyPresent()
├─ [0] VM::startPrank(SantasList: [0xE6E33783D6533ad50d373DDaa6fD21b272a57B69])
│ └─ ← ()
├─ [46713] SantaToken::mint(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D])
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], amount: 1000000000000000000 [1e18])
│ └─ ← ()
├─ [0] VM::stopPrank()
│ └─ ← ()
├─ [283] SantasList::CHRISTMAS_2023_BLOCK_TIME() [staticcall]
│ └─ ← 1703480381 [1.703e9]
├─ [0] VM::warp(1703480382 [1.703e9])
│ └─ ← ()
├─ [0] VM::startPrank(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D])
│ └─ ← ()
├─ [542] SantaToken::balanceOf(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [staticcall]
│ └─ ← 1000000000000000000 [1e18]
├─ [58439] SantasList::buyPresent(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D])
│ ├─ [2348] SantaToken::burn(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D])
│ │ ├─ emit Transfer(from: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], to: 0x0000000000000000000000000000000000000000, amount: 1000000000000000000 [1e18])
│ │ └─ ← ()
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], tokenId: 0)
│ └─ ← ()
├─ [542] SantaToken::balanceOf(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [staticcall]
│ └─ ← 0
├─ [678] SantasList::balanceOf(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [staticcall]
│ └─ ← 1
├─ [0] VM::stopPrank()
│ └─ ← ()
└─ ← ()
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 3.84ms
Recommended Mitigation
We should add a check of the presentReceiver token balance so that it equals PURCHASED_PRESENT_COST, as well as add a corresponding error SantaToken__NotEnoughTokens(); and if the condition is not met, revert.
You should also add PURCHASED_PRESENT_COST to the i_santaToken.burn() function in the amount parameter.
function buyPresent(address presentReceiver) external {
+ if (i_santaToken.balanceOf(presentReceiver) < PURCHASED_PRESENT_COST) {
+ revert SantaToken__NotEnoughTokens();
+ }
+ i_santaToken.burn(presentReceiver, PURCHASED_PRESENT_COST);
_mintAndIncrement();
}
contract SantaToken is ERC20 {
error SantaToken__NotSantasList();
uint8 private constant DECIMALS = 18;
+ uint64 private constant DEFAULT_VALUE = 1e18;
address private immutable i_santasList;
...
+ function burn(address from, uint256 amount) external {
if (msg.sender != i_santasList) {
revert SantaToken__NotSantasList();
}
+ _burn(from, amount);
}
