Mystery Box

Summary

There is no access control check on the function MysteryBox::changeOwner. This allows anyone to change the contract's owner to an address they control and call functions that require this elevated privilege.

Vulnerability Details

Overview:
A malicious entity is able to call MysteryBox::changeOwner(<maliciousAddress>) and is then able to drain the entire protocol by simply calling MysteryBox::withdraw.

Additionally, they could raise the price of a mystery box to an amount prohibitively high, by calling MysteryBox::setBoxPrice(<extortionatePrice>). This would prevent players from purchasing mystery boxes in the future and kill all activity of the protocol.

Conversely, they may set the price to 0, by calling MysteryBox::setBoxPrice(0) so that the protocol no longer generates fees. In this scenario, players would be able to purchase mystery boxes for the cost of gas. Any ether remaining in the protocol would be drained, as rare rewards would be earned quickly without a paywall slowing the purchase of mystery boxes.

Proof of Concept:
The steps below demonstrate the process to seize ownership of the contract and drain its balance:

1. Change ownership of the contract to a malicious address
   MysteryBox::changeOwner(attackerAddress)

2. Drain the contract of funds by calling withdraw
   MysteryBox::withdrawFunds()

Tools Used

Manual Review

Impact

This vulnerability allows an attacker to drain the contract completely and dramatically alter the protocol functionality by changing the mystery box price.

Recommended Mitigation

Add a check to ensure that the msg.sender is the owner of the contract, as is done with MysteryBox::addReward.