Mystery Box

First Flight #25
Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: low
Invalid

[L-01] No events are included in the contract, this impairs the ability to monitor state changes and other significant actions

Summary

The MysteryBox contract emits no events for any of the functions where key actions of the owner and players are executed.

These core functions update the state of the smart contract and it is imperative that these are stored as a log to ensure the protocol is auditable, transparent and can be monitored efficiently.

Vulnerability Details

Events are integral for smart contracts, as it facilitates the logging of state changes. The logs built by emitting events are immutable and can be indexed and queried off-chain. The absence of such logs greatly hinders the ability to audit, debug, monitor and trace transactions and state changes.

There are several functions in the MysteryBox contract that update state and would be expected to emit an event to log these changes. These functions are listed below:

  • setBoxPrice

  • addReward

  • buyBox

  • openBox

  • transferReward

  • withdrawFunds

  • claimAllRewards

  • claimSingleReward

  • changeOwner

Impact

Omitting events from key functions in the MysteryBox contract greatly affects the following:

  1. Auditability & Transparency: Events build a historical immutable record of previous transactions. This allows external parties to audit previous activity and ensures all activity is visible and transparent for past and new participants.

  2. Efficient Data Retrieval: Events facilitate off-chain data lookups, which is quicker and more efficient. Additionally, when events are indexed by specific parameters, queries can be filtered by these parameters, further improving query efficiency.

  3. Real-time Feedback for Frontends: Often frontends rely on a function emitting an event to update the interface for the interaction the user has completed.

  4. Debugging & Development: When trying to diagnose or debug an issue, it will be more difficult to replicate or analyse without a history of transactions that led up to the issue. Thus the developer and auditor experience is negatively affected.

Tools Used

Manual review & Aderyn

Recommended Mitigation

Add the following events to the contract:

event BoxPriceUpdated(uint256 indexed newPrice);
event AddedReward(string name, uint256 value);
event BoughtBox(address indexed player);
event OpenedBox(address indexed player);
event WithdrewFunds(address indexed owner, uint256 amount);
event TransferedReward(address indexed to, address indexed from, uint256 indexed index);
event ClaimedAllRewards(address indexed player, uint256 totalValue);
event ClaimedSingleReward(address indexed player, uint256 value);
event ChangedOwner(address indexed newOwner);

Emit the events in the corresponding functions:

function setBoxPrice(uint256 _price) public {
require(msg.sender == owner, "Only owner can set price");
boxPrice = _price;
+ emit BoxPriceUpdated(_price);
}
function addReward(string memory _name, uint256 _value) public {
require(msg.sender == owner, "Only owner can add rewards");
rewardPool.push(Reward(_name, _value));
+ emit AddedReward(_name, _value);
}
function buyBox() public payable {
require(msg.value == boxPrice, "Incorrect ETH sent");
boxesOwned[msg.sender] += 1;
+ emit BoughtBox(msg.sender);
}
function openBox() public {
require(boxesOwned[msg.sender] > 0, "No boxes to open");
// Generate a random number between 0 and 99
uint256 randomValue = uint256(keccak256(abi.encodePacked(block.timestamp, msg.sender))) % 100;
// Determine the reward based on probability
if (randomValue < 75) {
// 75% chance to get Coal (0-74)
rewardsOwned[msg.sender].push(Reward("Coal", 0 ether));
} else if (randomValue < 95) {
// 20% chance to get Bronze Coin (75-94)
rewardsOwned[msg.sender].push(Reward("Bronze Coin", 0.1 ether));
} else if (randomValue < 99) {
// 4% chance to get Silver Coin (95-98)
rewardsOwned[msg.sender].push(Reward("Silver Coin", 0.5 ether));
} else {
// 1% chance to get Gold Coin (99)
rewardsOwned[msg.sender].push(Reward("Gold Coin", 1 ether));
}
boxesOwned[msg.sender] -= 1;
+ emit OpenedBox(msg.sender);
}
function withdrawFunds() public {
uint256 totalFunds = address(this).balance;
require(msg.sender == owner, "Only owner can withdraw");
(bool success,) = payable(owner).call{value: totalFunds}("");
require(success, "Transfer failed");
+ emit WithdrewFunds(msg.sender, totalFunds);
}
function transferReward(address _to, uint256 _index) public {
require(_index < rewardsOwned[msg.sender].length, "Invalid index");
rewardsOwned[_to].push(rewardsOwned[msg.sender][_index]);
delete rewardsOwned[msg.sender][_index];
+ emit TransferedReward(_to, msg.sender, _index);
}
function claimAllRewards() public {
uint256 totalValue = 0;
for (uint256 i = 0; i < rewardsOwned[msg.sender].length; i++) {
totalValue += rewardsOwned[msg.sender][i].value;
}
require(totalValue > 0, "No rewards to claim");
(bool success,) = payable(msg.sender).call{value: totalValue}("");
require(success, "Transfer failed");
delete rewardsOwned[msg.sender];
+ emit ClaimedAllRewards(msg.sender, totalValue);
}
function claimSingleReward(uint256 _index) public {
require(_index <= rewardsOwned[msg.sender].length, "Invalid index");
uint256 value = rewardsOwned[msg.sender][_index].value;
require(value > 0, "No reward to claim");
(bool success,) = payable(msg.sender).call{value: value}("");
require(success, "Transfer failed");
delete rewardsOwned[msg.sender][_index];
+ emit ClaimedSingleReward(msg.sender, value);
}
function changeOwner(address _newOwner) public {
owner = _newOwner;
+ emit ChangedOwner(_newOwner)
}
Updates

Appeal created

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
0xnelli Submitter
about 1 year ago
inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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

Give us feedback!