First Flight #9: Soulmate
First Flight #9
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: low
Valid

`Soulmate::mintSoulmateToken` fails to validate recipient addresses in its `_mint` function, risking a DOS attack on the token.

oxenzo

Description: The attack vector described involves malicious actors exploiting the lack of recipient addresss validation of the onERC721Received function within the Soulmate::mintSoulmateToken function, specifically through the use of the _mint function.

function mintSoulmateToken() public returns (uint256) {
address soulmate = soulmateOf[msg.sender];
if (soulmate != address(0)) {
revert Soulmate__alreadyHaveASoulmate(soulmate);
}
​
address soulmate1 = idToOwners[nextID][0];
address soulmate2 = idToOwners[nextID][1];
if (soulmate1 == address(0)) {
idToOwners[nextID][0] = msg.sender;
ownerToId[msg.sender] = nextID;
emit SoulmateIsWaiting(msg.sender);
} else if (soulmate2 == address(0)) {
idToOwners[nextID][1] = msg.sender;
ownerToId[msg.sender] = nextID;
soulmateOf[msg.sender] = soulmate1;
soulmateOf[soulmate1] = msg.sender;
idToCreationTimestamp[nextID] = block.timestamp;
​
emit SoulmateAreReunited(soulmate1, soulmate2, nextID);
​
@> _mint(msg.sender, nextID++);
}
​
return ownerToId[msg.sender];
}
​
​
@> function _mint(address to, uint256 id) internal virtual {
require(to != address(0), "INVALID_RECIPIENT");
​
require(_ownerOf[id] == address(0), "ALREADY_MINTED");
​
unchecked {
_balanceOf[to]++;
}
​
_ownerOf[id] = to;
​
emit Transfer(address(0), to, id);
}
​

Impact: As a consequence of the attack, tokens sent to addresses incapable of handling them become irretrievably lost within the Ethereum blockchain. Since blockchain transactions are immutable, once tokens are sent to an address, they cannot be reversed or recovered thus resulting in a permanent loss of tokens from the token supply, adversely affecting Soulmate token holders and the overall ecosystem stability.

Proof of Concept:

  1. A malicious actor may attempt to mint a Soulmate token by pairing a non-NFT compatible smart contract address with another address presumed to support NFT functionality.

  2. A denial-of-service (DOS) attack transpires, resulting in the loss of the token within the Ethereum blockchain.

Proof Of Code

Place the following into the SoulmateTest.t.sol.

function test_NonNftContractrevertswhenMintingSoulmateContract() public {
vm.prank(soulmate1);
soulmateContract.mintSoulmateToken();
​
assertTrue(soulmateContract.totalSupply() == 0);
​
vm.prank(address(nonNftContractaddress));
vm.expectRevert();
soulmateContract.mintSoulmateToken();
​
// Both Soulmate 1 and the NonNftContract address dont have a token mainly because of the exploit in the NonNftContract.
assertTrue(soulmateContract.totalSupply() == 0);
}
}
​
​
contract NonNftContract {
// Contract that doesnt implement the IERC721Receiver.onERC721Received()
}
​

Recommended Mitigation: Use the _safeMint function instead of the _mint function since it carries out checks for the onERC721Received function which reverts back with the selector thus proving whether the recipient address is compatible to handle ERC-721 tokens or not.

​
-function _mint(address to, uint256 id) internal virtual {
- require(to != address(0), "INVALID_RECIPIENT");
​
- require(_ownerOf[id] == address(0), "ALREADY_MINTED");
​
- unchecked {
- _balanceOf[to]++;
- }
​
- _ownerOf[id] = to;
​
- emit Transfer(address(0), to, id);
- }
​
​
+function _safeMint(address to, uint256 id) internal virtual {
+ _mint(to, id);
​
+ require(
+ to.code.length == 0 ||
+ ERC721TokenReceiver(to).onERC721Received(msg.sender, address(0), id, "") ==
+ ERC721TokenReceiver.onERC721Received.selector,
+ "UNSAFE_RECIPIENT"
+ );
+ }
​
​
​
function mintSoulmateToken() public returns (uint256) {
address soulmate = soulmateOf[msg.sender];
if (soulmate != address(0)) {
revert Soulmate__alreadyHaveASoulmate(soulmate);
}
​
address soulmate1 = idToOwners[nextID][0];
address soulmate2 = idToOwners[nextID][1];
if (soulmate1 == address(0)) {
idToOwners[nextID][0] = msg.sender;
ownerToId[msg.sender] = nextID;
emit SoulmateIsWaiting(msg.sender);
} else if (soulmate2 == address(0)) {
idToOwners[nextID][1] = msg.sender;
ownerToId[msg.sender] = nextID;
soulmateOf[msg.sender] = soulmate1;
soulmateOf[soulmate1] = msg.sender;
idToCreationTimestamp[nextID] = block.timestamp;
​
emit SoulmateAreReunited(soulmate1, soulmate2, nextID);
​
- _mint(msg.sender, nextID++);
+ _safeMint(msg.sender, nextID++);
}
​
return ownerToId[msg.sender];
}
​
​
Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-safemint

Low severity, - If a user utilizes an EOA, the check is not required. - If a user utilizes a contract that they own to mint soulmate tokens, than check is required. However, this would rely on user error minting. Since there is no mention that ownership of token must be from EOAs, I believe low severity is appropriate.

oxenzo Submitter
over 1 year ago
0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-safemint

Low severity, - If a user utilizes an EOA, the check is not required. - If a user utilizes a contract that they own to mint soulmate tokens, than check is required. However, this would rely on user error minting. Since there is no mention that ownership of token must be from EOAs, I believe low severity is appropriate.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.