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

No validations to check the ownership of NFT leads to gaining credTokens and unfair advantage

0x0bserver

Summary

No validations to check the ownership of NFT leads to gaining credTokens and unfair advantage.

Vulnerability Details

There is no check to check whether the given tokenId is of the msg.sender or not. Attacker can use this to send any tokenId even if he doesn't own NFT of that tokenId. This way, attackers can send tokenIds who have participated in staking and have better RapperSkills. This will help attackers to gain unfair advantage when they battle defenders.

Impact

  1. Attacker can send anyone's tokenId which has the better skills to influence the battle such as sending the tokenId of defender.

  2. Users don't have to mint Rapper NFT to battle and win credTokens.

  3. Users who are still staking their NFT and have their NFT with Streets contract can still participate in RapBattle.

Tools Used

Manual Review

Recommendations

Add the below code in _battle function -

function _battle(uint256 _tokenId, uint256 _credBet) internal {
address _defender = defender;
require(oneShotNft.ownerOf(_tokenId) == msg.sender, "RapBattle: Wrong Owner");
require(defenderBet == _credBet, "RapBattle: Bet amounts do not match");
uint256 defenderRapperSkill = getRapperSkill(defenderTokenId);
uint256 challengerRapperSkill = getRapperSkill(_tokenId);
uint256 totalBattleSkill = defenderRapperSkill + challengerRapperSkill;
uint256 totalPrize = defenderBet + _credBet;
​
uint256 random =
uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao, msg.sender))) % totalBattleSkill;
​
// Reset the defender
defender = address(0);
emit Battle(msg.sender, _tokenId, random < defenderRapperSkill ? _defender : msg.sender);
​
// If random <= defenderRapperSkill -> defenderRapperSkill wins, otherwise they lose
if (random <= defenderRapperSkill) {
// We give them the money the defender deposited, and the challenger's bet
credToken.transfer(_defender, defenderBet);
credToken.transferFrom(msg.sender, _defender, _credBet);
} else {
// Otherwise, since the challenger never sent us the money, we just give the money in the contract
credToken.transfer(msg.sender, _credBet);
}
totalPrize = 0;
// Return the defender's NFT
oneShotNft.transferFrom(address(this), _defender, defenderTokenId);
}
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.