Mystery Box

First Flight #25
Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: medium
Valid

Improper State Management in Reward Transfer Function

Summary

The transferReward function allows users to transfer a specific reward from their own rewards list to another address. It checks the validity of the index, performs the transfer, and deletes the original entry. However, it contains vulnerabilities that could be exploited.

Vulnerability Details

The function does not properly manage the rewards array after deletion. When the reward is deleted using delete rewardsOwned[msg.sender][_index];, the array remains the same length, leaving a "hole" in the array that can be exploited. This allows malicious users to repeatedly call the function using the same index, potentially transferring rewards multiple times without reverting.

Impact

Exploitation of this vulnerability could lead to unauthorized transfers of rewards, allowing attackers to drain a user's rewards without proper consent or validation. This undermines the integrity of the rewards system and could lead to significant financial loss for users.

Tools Used

  • Slither

  • Remix IDE

Recommendations

  1. Use Safe Array Manipulation: Instead of deleting the reward directly, consider shifting the last element of the array to the index being deleted, thus maintaining array integrity and avoiding "holes".

    function transferReward(address _to, uint256 _index) public {
    require(_index < rewardsOwned[msg.sender].length, "Invalid index");
    rewardsOwned[_to].push(rewardsOwned[msg.sender][_index]);
    // Move last reward to the index being deleted
    rewardsOwned[msg.sender][_index] = rewardsOwned[msg.sender][rewardsOwned[msg.sender].length - 1];
    rewardsOwned[msg.sender].pop(); // Remove last element
    }ransferReward Function Vulnerabilities
  2. Implement Reentrancy Guards: Use the checks-effects-interactions pattern or the ReentrancyGuard from OpenZeppelin to prevent reentrant calls.

  3. Comprehensive Testing: Conduct thorough testing, including unit tests and integration tests, to validate the function's behavior under various scenarios and potential attack vectors.

Updates

Appeal created

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

A user can poison the `rewardsOwned` of another user via `transferReward` of an empty reward index

Support

FAQs

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

Give us feedback!