Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: high
Valid

Challenger Can Battle Without RapperNft or CredTokens.

Summary

The RapBattle contract allows a challenger to participate in a battle without verifying that they own the NFT or have the required Cred tokens. This can be exploited by a challenger who initiates a battle with a random tokenId and matching credBet, without actually owning the NFT or Cred tokens. If the challenger wins, they receive the defender's Cred tokens. If they lose, the contract's attempt to transfer the challenger's Cred tokens to the defender fails, and the transaction reverts, leaving the defender uncompensated.

Proof of Code

Add the following function in OneShotTest.t.sol

function test_AttackerCanBattleWithoutRapperNftorCredTokens() public twoSkilledRappers {
vm.startPrank(user);
oneShot.approve(address(rapBattle), 0);
cred.approve(address(rapBattle),4);
rapBattle.goOnStageOrBattle(0, 4); //tokenid=0,credbet=4
vm.stopPrank();
address attacker = makeAddr("attacker");
vm.startPrank(attacker);
console2.log("cred balance challenger ", cred.balanceOf(attacker));
rapBattle.goOnStageOrBattle(0, 4);//credBet=4 which attacker does not own.
console2.log("cred balance challenger ", cred.balanceOf(attacker));
assertEq(cred.balanceOf(attacker),4) //holds true whenever attacker wins the battle. transaction reverts otherwise.
vm.stopPrank();
}
  • Defender calls goOnStageOrBattle with their NFT (tokenId) and Cred tokens (credBet), which are transferred to the contract.

  • Challenger calls goOnStageOrBattle with any existing tokenId and the same credBet amount, without owning the NFT or Cred tokens.

  • If the challenger wins, the credToken.transfer call succeeds, and they receive the defender's Cred tokens.

  • If the challenger loses, the credToken.transferFrom call fails, the transaction reverts, and the defender receives nothing.

Impact

This vulnerability allows a challenger to gamble with no risk, potentially stealing the defender's Cred tokens if they win, and facing no loss if they lose, as the transaction will revert due to the failed token transfer. This defeats the whole purpose of RapperNft and staking.

Tools Used

Manual review

Recommendations

  • Modify the goOnStageOrBattle function to include checks that verify the challenger's ownership of the NFT and their possession of the required Cred tokens, as well as approval for the contract to spend those tokens. Implement the following checks for the challenger before initiating a battle:

Check that the challenger owns the NFT.

++ require(oneShotNft.ownerOf(_tokenId) == msg.sender, "Challenger must own the token");
Check that the challenger has enough Cred tokens and has approved them for the contract
++ require(credToken.allowance(msg.sender, address(this)) >= _credBet, "Challenger must approve bet amount");
++ require(credToken.balanceOf(msg.sender) >= _credBet, "Challenger must have enough Cred tokens");

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

missing check for sufficient `_credBet_` approval

oldmonk Submitter
over 1 year ago
inallhonesty Lead Judge
over 1 year ago
inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Challenger can use any nft to battle - not necessarily theirs

missing check for sufficient `_credBet_` approval

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.