First Flight #13: Baba Marta
First Flight #13
Beginner FriendlyFoundryGameFi
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Attacker can wipe entire `countMartenitsaTokensOwner` leading to loss of rewards

nikos

Summary

Any user can execute updateCountMartenitsaTokensOwner on any user.

Vulnerability Details

The purpose of updateCountMartenitsaTokensOwner is to add or subtract count of tokens for user. It's used in two of following functions:

However, updateCountMartenitsaTokensOwner is external which means any arbitrary user can execute it. Leaving function like that can result in state change of countMartenitsaTokensOwner inside MartenitsaToken, which is used for calculating rewards - collectReward

Impact

Users would loss his rewards.

Proof Of Concept

  1. Alice buy 3 martenitsas via buyMartenitsa

  2. Alice wants to collectReward

  3. Just before Alice transaction - Bob execute updateCountMartenitsaTokensOwner 3 times with params: [Alice address, "sub"]

  4. Alice doesn't get any reward

Tools Used

Manual Review

Recommendations

Set updateCountMartenitsaTokensOwner as internal

- function updateCountMartenitsaTokensOwner(address owner, string memory operation) external {
+ function updateCountMartenitsaTokensOwner(address owner, string memory operation) internal {
if (keccak256(abi.encodePacked(operation)) == keccak256(abi.encodePacked("add"))) {
countMartenitsaTokensOwner[owner] += 1;
} else if (keccak256(abi.encodePacked(operation)) == keccak256(abi.encodePacked("sub"))) {
countMartenitsaTokensOwner[owner] -= 1;
} else {
revert("Wrong operation");
}
}
Updates

Lead Judging Commences

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

Missing access control

Support

FAQs

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