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

[M-1] A user can use NFT of another user when he is the attacker in `RapBattle::goOnStageOrBattle()`

Clemmos

Summary

A user can use the NFT of another user for a battle.

Vulnerability Details

When a user is a defender by calling goOnStageOrBattle, the next user, the attacker, can use the NFT Rapper of another user.

function goOnStageOrBattle(uint256 _tokenId, uint256 _credBet) external {
if (defender == address(0)) {
...
} else {
// credToken.transferFrom(msg.sender, address(this), _credBet);
@>> _battle(_tokenId, _credBet);
}
}
​
function _battle(uint256 _tokenId, uint256 _credBet) internal {
address _defender = defender;
require(defenderBet == _credBet, "RapBattle: Bet amounts do not match");
uint256 defenderRapperSkill = getRapperSkill(defenderTokenId);
@>> uint256 challengerRapperSkill = getRapperSkill(_tokenId);
...
}

Impact

There is no check to know if an attacker is the owner of the NFT used for the battle. The malicious user (the attacker) can then battle without having an Rapper NFT.

Proof of Concept

  1. Two different users mint a Rapper NFT

  2. The first user call goOnStageOrBattle after approving his NFT transfer

  3. A third user call goOnStageOrBattle and pass as a parameter the NFT id of the second user.

Code :

function testBattleWithAnotherUserNft() public twoSkilledRappers {
vm.startPrank(user);
oneShot.approve(address(rapBattle), 0);
rapBattle.goOnStageOrBattle(0, 0);
vm.stopPrank();
​
address userWithNoNft = makeAddr("noNftUser");
vm.startPrank(userWithNoNft);
uint256 challengerNftId = 1; // NFT id of "challenger"
rapBattle.goOnStageOrBattle(challengerNftId, 0);
vm.stopPrank();
}

Add this test inside OneShotTest.t.sol and then execute forge test --mt testBattleWithAnotherUserNft

Tools Used

Foundry

Recommendations

Add a condition to revert if the owner of the _tokenId is not the msg.sender

+ error NotRapperNftOwner();
​
function goOnStageOrBattle(uint256 _tokenId, uint256 _credBet) external {
+ if(oneShotNft.ownerOf(_tokenId) != msg.sender) {
+ revert NotRapperNftOwner();
+ }
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);
}
}
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.