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

Event `Soulmate::SoulmateAreReunited` emitted inside `Soulmate::mintSoulmateToken` has wrong parameters

carlosalbawork

Summary

Event Soulmate::SoulmateAreReunited emitted inside Soulmate::mintSoulmateToken has wrong parameters

Vulnerability Details

The event Soulmate::SoulmateAreReunited is emitted after some user calls Soulmate::mintSoulmateToken when another user is looking for a soulmate.

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);
_mint(msg.sender, nextID++);
}
​
return ownerToId[msg.sender];
}

However, the variable soulmate2 is never updated after the check of soulmate2 == address(0), so soulmate2 still remains as address(0) and is emitted with the event!

Impact

Inputs wrong information into the event.

Tools Used

Manual Review

Proof of Concept:

//Only enters if soulmate2 is address(0)
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;
//soulmate2 is never changed to msg.sender, so remains address(0) instead
emit SoulmateAreReunited(soulmate1, soulmate2, nextID);
_mint(msg.sender, nextID++);
}

Recommendations

Changing soulmate2 for msg.sender

//Only enters if soulmate2 is address(0)
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;
//soulmate2 is never changed to msg.sender, so remains address(0) instead
- emit SoulmateAreReunited(soulmate1, soulmate2, nextID);
+ emit SoulmateAreReunited(soulmate1, msg.sender, nextID);
_mint(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.