DatingDapp
AI First Flight #6
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: medium
Invalid

`LikeRegistry::matchRewards` keeps the state variable `LikeRegistry::totalFees` at `0`

datadata

[M-2] LikeRegistry::matchRewards keeps the state variable LikeRegistry::totalFees at 0

Description

Function LikeRegistry::matchRewards updates var LikeRegistry::totalFees, but it does so with faulty data by pulling from the mapping LikeRegistry::userBalances, whose entries never change from the default 0.

In contract LikeRegistry:

function matchRewards(address from, address to) internal {
@> uint256 matchUserOne = userBalances[from];
@> uint256 matchUserTwo = userBalances[to];
userBalances[from] = 0;
userBalances[to] = 0;
​
@> uint256 totalRewards = matchUserOne + matchUserTwo;
@> uint256 matchingFees = (totalRewards * FIXEDFEE) / 100;
@> uint256 rewards = totalRewards - matchingFees;
@> totalFees += matchingFees;
...
(bool success, ) = payable(address(multiSigWallet)).call{
@> value: rewards
}("");

Risk

Likelihood:
This is guaranteed to happen any time matchRewards is called.

Impact:
The impact is that this portion of matchRewards effectively does nothing, because it is grabbing 0 from the mapping userBalances to define matchUserOne and matchUserTwo, resetting entries in userBalances to 0, defining totalRewards as 0 + 0, which equals 0, defining matchingFees as (0 * FIXEDFEE) / 100, which equals 0, defining rewards as 0 - 0, which equals 0, and then incrementing totalFees by 0, then transfering a value of 0 to the newly created contract multiSigWallet.

Proof of Concept

Using our added getter function LikeRegistry::getTotalFees, and our modifier that mints profiles for user1 and user2, we see in this function that state variable likeRegistry:totalFees stays at 0 before and after two users mutually like each other, which should call likeRegistry::matchRewards, which should be updating totalFees.

function testMutualLikesDoNotIncreaseTotalFees()
public
mintProfilesUser1AndUser2
{
// ARRANGE
uint256 totalFeesBefore = likeRegistry.getTotalFees();
console.log("totalFees before: ", totalFeesBefore);
​
// ACT
vm.prank(user1);
likeRegistry.likeUser{value: s_feeAmount}(user2);
vm.prank(user2);
likeRegistry.likeUser{value: s_feeAmount}(user1);
​
// ASSERT
uint256 totalFeesAfter = likeRegistry.getTotalFees();
console.log("totalFees after: ", totalFeesAfter);
assertEq(totalFeesBefore, totalFeesAfter);
}

Recommended Mitigation

We recommend that LikeRegistry::likeUser is updated to added non-zero values to the entries in the mapping LikeRegistry::userBalances so that matchRewards accesses non-zero values to indeed match the rewards. See our finding M-1 for more information.

We recommend that the function LikeRegistry::likeUser updates msg.sender's entry in userBalances by adding msg.value to it.

function likeUser(address liked) external payable {
...
likes[msg.sender][liked] = true;
+ userBalances[msg.sender] += msg.value;
emit Liked(msg.sender, liked);
...
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 1 hour ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!