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

The event SoulmateAreReunited is triggered with incorrect parameters.

shaflow01

Summary

The event SoulmateAreReunited is triggered with incorrect parameters.

Vulnerability Details

else if (soulmate2 == address(0)) {
idToOwners[nextID][1] = msg.sender;
// Once 2 soulmates are reunited, the token is minted
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++);
}

In the mintSoulmateToken() function, when entering the else if branch, soulmate2 is set to the address(0), causing soulmate2 to always be address(0) when triggering emit SoulmateAreReunited(soulmate1, soulmate2, nextID).

Impact

The event SoulmateAreReunited is triggered with incorrect parameters.
POC

function test_MintNewToken() public {
uint tokenIdMinted = 0;
​
vm.prank(soulmate1);
soulmateContract.mintSoulmateToken();
​
assertTrue(soulmateContract.totalSupply() == 0);
​
vm.prank(soulmate2);
soulmateContract.mintSoulmateToken();
​
assertTrue(soulmateContract.totalSupply() == 1);
assertTrue(soulmateContract.soulmateOf(soulmate1) == soulmate2);
assertTrue(soulmateContract.soulmateOf(soulmate2) == soulmate1);
assertTrue(soulmateContract.ownerToId(soulmate1) == tokenIdMinted);
assertTrue(soulmateContract.ownerToId(soulmate2) == tokenIdMinted);
}

result

[PASS] test_MintNewToken() (gas: 210743)
Traces:
[210743] SoulmateTest::test_MintNewToken()
├─ [0] VM::prank(soulmate1: [0x65629adcc2F9C857Aeb285100Cc00Fb41E78DC2f])
│ └─ ← ()
├─ [33015] Soulmate::mintSoulmateToken()
│ ├─ emit SoulmateIsWaiting(soulmate: soulmate1: [0x65629adcc2F9C857Aeb285100Cc00Fb41E78DC2f])
│ └─ ← 0
├─ [393] Soulmate::totalSupply() [staticcall]
│ └─ ← 0
├─ [0] VM::prank(soulmate2: [0xe93A5E9F20AF38E00a08b9109D20dEc1b965E891])
│ └─ ← ()
├─ [157343] Soulmate::mintSoulmateToken()
│ ├─ emit SoulmateAreReunited(soulmate1: soulmate1: [0x65629adcc2F9C857Aeb285100Cc00Fb41E78DC2f], soulmate2: 0x0000000000000000000000000000000000000000, tokenId: 0)
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: soulmate2: [0xe93A5E9F20AF38E00a08b9109D20dEc1b965E891], id: 0)

Tools Used

Manual Review

Recommendations

emit SoulmateAreReunited(soulmate1, msg.sender, nextID);
Updates

Lead Judging Commences

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

finding-mintSoulmateToken-wrong-emit-soulmate2

Low severity, inconsistencies in event emission Sidenote: Separating all event findings given root causes are different with different functions involved. There could be alot of debate on whether wrong emit events consitute low severity, but I believe, - There are direct inconsistencies in the code logic - Codehawks [low severity categorization guidelines](https://docs.codehawks.com/hawks-auditors/how-to-evaluate-a-finding-severity#low-severity-findings) supports its severity as seen below, especially noting the term `Minimal to no impact` > - Minimal to no impact on the funds or the protocol's main functionality.

Support

FAQs

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