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

No check for ownership of rapper tokenId leads to attackers entering rap battles with someone else's tokenId and being able to win defender's credibility token bet

Summary

An attacker who does not have a rapper tokenId minted to him/her can enter a rap battle as a challenger using someone else's tokenId, or a random, non-existent tokenId as well. If he/she wins, he/she gets the defender's credibility token bet. However, if the defender wins, the transaction reverts because the attacker never approved any credibility tokens to the contract (the attacker doesn't have any credibility tokens to begin with).

Vulnerability details

The RapBattle::goOnStageOrBattle function doesn't check if the rapper tokenId is valid, nor does it check the ownership of the tokenId. This can lead to the following exploits:

  1. Exisiting rappers (users of the protocol), can enter as challengers with someone else's rapper tokenId with maxed out stats to maximize their chance of winning (rapper skill is a factor in determining the random winner).

  2. An attacker who doesn't assume a role in the protocol (doesn't have a rapper Nft minted to him/her), can enter a rap battle and win credibility tokens. This is made possible because the RapBattle::goOnStageOrBattle function doesn't require the challenger to transfer his/her rapper tokenId, nor his/her credibility token bet to the RapBattle contract. Thus, if the defender wins, the transaction reverts as the RapBattle::_battle function fails to transfer credibility token bet from challenger to the defender (the challenger never approved the credibility token bet to the contract).

  3. Additionally, it is also possible to enter a rap battle as a challenger using a non-existent rapper tokenId. However, attackers will prefer the second option (the option above).

Proof of Concept

  1. Defender enters the rap battle by approving his/her rapper tokenId and credibility token bet to the RapBattle contract. The RapBattle contract transfers the tokenId and credibility token bet to itself.

  2. The challenger (attacker) without his/her own rapper tokenId enters the battle with someone else's tokenId (with maxed out stats). Also, the challenger doesn't approve any credibility tokens to the RapBattle contract.

  3. If the defender wins, the transaction reverts because the contract failed to transfer tokens from challenger to the defender.

  4. If the challenger wins, he/she gets the credibility token bet placed by the defender.

Add the following function to the test contract in your Foundry test file:

function testAttackerEntersWithSomeoneElsesTokenId(uint256 time, bytes32 difficulty) public {
// setup phase
address defender = makeAddr("defender");
address attacker = makeAddr("attacker");
address pro = makeAddr("pro");
// defender gets his rapper tokenId minted and stakes it for a day
vm.startPrank(defender);
oneShot.mintRapper();
oneShot.approve(address(streets), 0);
streets.stake(0);
vm.warp(1 days + 1);
streets.unstake(0);
vm.stopPrank();
// pro gets his rapper tokenId minted, stakes it for as long as possible to get max stats and credibility tokens
vm.startPrank(pro);
oneShot.mintRapper();
oneShot.approve(address(streets), 1);
streets.stake(1);
vm.warp(5 days + 1);
streets.unstake(1);
vm.stopPrank();
// defender enters battle
vm.startPrank(defender);
oneShot.approve(address(rapBattle), 0);
cred.approve(address(rapBattle), 1);
rapBattle.goOnStageOrBattle(0, 1);
vm.stopPrank();
// attacker enters with pro's rapper tokenId
vm.startPrank(attacker);
// fuzz with randomized time and block difficulty
vm.warp(time);
vm.prevrandao(difficulty);
rapBattle.goOnStageOrBattle(1, 1); // didn't approve any credibility tokens to the contract
assertEq(cred.balanceOf(attacker), 1); // attacker wins defender's credibility token bet
}

The fuzz test ran with randomized time and block difficulty for 272 times with seed "0x1", indicating that the attacker was able to successfully win those battles. The transaction reverted when the defender won.

Failing tests:
Encountered 1 failing test in test/auditTests.t.sol:RapBattleTest
[FAIL. Reason: ERC20InsufficientAllowance(0x5991A2dF15A8F6A256D3Ec51E99254Cd3fb576A9, 0, 1); counterexample: calldata=0x0b56544000009fc67339fad76937597b36f50df9353a59f65aead188f5d40fda28d2b9b7975c3f3c511c1a03e2a02134ad34521063ac199d0b2430b873191f1e534e2984 args=[1102727873344863962616923325797002069849338298097995261905762206996412855 [1.102e72], 0x975c3f3c511c1a03e2a02134ad34521063ac199d0b2430b873191f1e534e2984]] testAttackerEntersWithSomeoneElsesTokenId(uint256,bytes32) (runs: 272, μ: 616430, ~: 616430)

Impact

Attackers who do not assume any role in the protocol (they don't have a rapper tokenId minted to them) can enter rap battles and win defender's credibility token bet. Also, anyone can use anyone's rapper tokenId for battle (if they enter as challenegrs). This is a serious disruption of what the protocol intends to achieve. Only users with rapper tokenIds should be able to go and battle.

Tools Used

Foundry, VSCodium.

Recommendations

In RapBattle::goOnStageOrBattle function, check if the rapper tokenId is owned by the msg.sender.

function goOnStageOrBattle(uint256 _tokenId, uint256 _credBet) external {
+ require(oneShotNft.ownerOf(_tokenId) == msg.sender, "Not owner of rapper tokenId");
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.