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

'Soulmate2' is not correctly emitted in `SoulmateAreReunited`

4th05

Summary

The event SoulmateAreReunited does not report the correct address for soulmate2 because the emit is placed in the '' of a second if statement

Vulnerability Details

address soulmate2 = idToOwners[nextID][1]; is placed outside of the if statements and the address of the 'soulmate2' is not assigned to the variable 'address soulmate2' when a SoulmateAreReunited is emitted.

Impact

This has an impact on the address displayed by this event that shows always 'address(0)' as 'soulmate2' regardless the address used as 'soulmate2' in test.
It can be noticed using command '-vvvv' in Foundry.

function test_mint() public {
​
// Mint Soulmatetoken
vm.prank(soulmate1);
soulmateContract.mintSoulmateToken();
vm.prank(soulmate2);
soulmateContract.mintSoulmateToken();
assertTrue(soulmateContract.soulmateOf(soulmate1)==soulmate2);
}
Traces:
[204940] ProtocolTest::test_mint()
├─ [0] VM::prank(soulmate1: [0x65629adcc2F9C857Aeb285100Cc00Fb41E78DC2f])
│ └─ ← ()
├─ [32993] Soulmate::mintSoulmateToken()
│ ├─ emit SoulmateIsWaiting(soulmate: soulmate1: [0x65629adcc2F9C857Aeb285100Cc00Fb41E78DC2f])
│ └─ ← 0
├─ [0] VM::prank(0x0000000000000000000000000000000000000002)
│ └─ ← ()
├─ [157321] Soulmate::mintSoulmateToken()
│ ├─ emit SoulmateAreReunited(soulmate1: soulmate1: [0x65629adcc2F9C857Aeb285100Cc00Fb41E78DC2f], soulmate2: 0x0000000000000000000000000000000000000000, tokenId: 0)
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: 0x0000000000000000000000000000000000000002, id: 0)
│ └─ ← 0
├─ [602] Soulmate::soulmateOf(soulmate1: [0x65629adcc2F9C857Aeb285100Cc00Fb41E78DC2f]) [staticcall]
│ └─ ← 0x0000000000000000000000000000000000000002
└─ ← ()

Tools Used

Foundry

Recommendations

Replace 'solumate2' with 'msg.sender' in line 82.

function mintSoulmateToken() public returns (uint256) {
// Check if people already have a soulmate, which means already have a token
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;
// 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);
+ emit SoulmateAreReunited(soulmate1, msg.sender, nextID);
_mint(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-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.