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

The user can battle against theirself in `RapBattle::goOnStageOrBattle` function

Summary

The RapBattle::goOnStageOrBattle function allows a user to initiate a battle against theirself by calling the function twice with different or the same tokenId.

Vulnerability Details

The RapBattle smart contract allows a user to initiate a battle against theirself by calling the goOnStageOrBattle function twice with different or the same token IDs but the same bet amount.

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

Impact

The RapBattle::goOnStageOrBattle function doesn't check if the address of the defender is not equal to the address of the challenger. That allows to someone to make a battle with theirself. Moreover, the provided tokenId also can be the same.
The following test function testGoOnStageOrBattleSameUser shows the following scenario: The user (Alice) has minted two rapper tokens. The user calls the function RapBattle::goOnStageOrBattle twice and receives the defender and challenger role. In that case the user will have sure win for one of the tokens. You can execute this test function with foundry command:
forge test --match-test "testGoOnStageOrBattleSameUser" -vvvvv

function testGoOnStageOrBattleSameUser() public mintRapper {
vm.startPrank(user);
oneShot.approve(address(rapBattle), 0);
rapBattle.goOnStageOrBattle(0, 0);
vm.stopPrank();
vm.startPrank(user);
oneShot.mintRapper();
oneShot.approve(address(rapBattle), 1);
rapBattle.goOnStageOrBattle(1, 0);
}

Tools Used

Manual Review, Foundry

Recommendations

Modify the RapBattle::goOnStageOrBattle function to include a check that prevents the msg.sender from initiating a battle if there is already the defender. This can be done by adding a require statement:

function goOnStageOrBattle(uint256 _tokenId, uint256 _credBet) external {
+ require(msg.sender != defender, "RapBattle: Cannot battle against yourself");
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:

It's YOU vs YOU

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

It's YOU vs YOU

Support

FAQs

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