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

Challengers Can Battle Using Any Rapper NFT

EloiManuel

Summary

The RapBattle contract is not checking whether the current challenger is the owner of the rapper NFT in use for a battle.

Vulnerability Details

When entering a battle as a challenger, the code snippet inside the else statement below is executed:

oneShotNft.transferFrom(msg.sender, address(this), _tokenId);
credToken.transferFrom(msg.sender, address(this), _credBet);
} else {
// credToken.transferFrom(msg.sender, address(this), _credBet);
_battle(_tokenId, _credBet);
}

Notice that it it not checking the ownership of the rapper NFT nor transferring it from msg.sender (contrary to what happens in the defender's logic above).

Impact

An attacker could call goOnStageOrBattle() using a _tokenId of a rapper NFT with good stats without actually owning it, increasing the chances of winning battles and earning CredToken from it.

Proof of Concept

function testBattleOtherNFT() public mintRapper {
// In order to use 1 cred, lets stake my rapper for 1 day
vm.startPrank(user);
oneShot.approve(address(streets), 0);
streets.stake(0);
vm.warp(1 days + 1);
streets.unstake(0);
cred.approve(address(rapBattle), 1);
oneShot.approve(address(rapBattle), 0);
rapBattle.goOnStageOrBattle(0, 1);
rapBattle.goOnStageOrBattle(99, 1);
vm.stopPrank();
}

Tools Used

Manual analysis and Foundry.

Recommendations

It is recommended to check whether the challenger does own the _tokenId and/or escrow it during the battle as shown below:

Proposed solution:

} else {
// credToken.transferFrom(msg.sender, address(this), _credBet);
+ oneShotNft.transferFrom(msg.sender, address(this), _tokenId);
_battle(_tokenId, _credBet);
}

and

totalPrize = 0;
// Return the defender's NFT
oneShotNft.transferFrom(address(this), _defender, defenderTokenId);
+ // Return the challenger's NFT
+ oneShotNft.transferFrom(address(this), msg.sender, _tokenId);
}

If the challenger does not own the rapper NFT with token ID _tokenId, the call to goOnStageOrBattle() will revert.

Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year 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.