Summary
We are not taking approval of tokens from challenger to transfer his CreadToken to rapBattle and NFT to rapBattle which will give a challenger winning edge over defender and he can win without any approval of tokens.
when, challenger calls goOnStageOrBattle then he is not approving rapBattle to transfer his credToken which will revert when ramdom favours defender and challenger will not loose anything as he has not approved rapBattle to transfer his CredToken. On the other hand, if ramdom favours challenger then he will win and get defender's CredToken.
Vulnerability Details
-
Defender can not be able to win a single battle. because, challenger is not approving rapBattle to transfer credToken which will give a challenger winning edge over defender.
-
if ramdom favours challenger then he will win and get defender's CredToken
-
but if ramdom favours defender then goOnStageOrBattle will revert and challenger will not loose anything as he has not approved rapBattle to transfer his CredToken
-
So, only challenger can win the battle.
function _battle(uint256 _tokenId, uint256 _credBet) internal {
address _defender = defender;
require(defenderBet == _credBet, "RapBattle: Bet amounts do not match");
uint256 defenderRapperSkill = getRapperSkill(defenderTokenId);
uint256 challengerRapperSkill = getRapperSkill(_tokenId);
uint256 totalBattleSkill = defenderRapperSkill + challengerRapperSkill;
uint256 totalPrize = defenderBet + _credBet;
uint256 random =
uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao, msg.sender))) % totalBattleSkill;
defender = address(0);
emit Battle(msg.sender, _tokenId, random < defenderRapperSkill ? _defender : msg.sender);
if (random <= defenderRapperSkill) {
credToken.transfer(_defender, defenderBet);
credToken.transferFrom(msg.sender, _defender, _credBet);
} else {
@> credToken.transfer(msg.sender, _credBet);
}
totalPrize = 0;
oneShotNft.transferFrom(address(this), _defender, defenderTokenId);
}
POC
function testGoOnStageOrBattleIfChallengerNotApprovingTokens() public twoSkilledRappers {
uint256 userBalanceBefore = cred.balanceOf(user);
uint256 challengerBalanceBefore = cred.balanceOf(challenger);
vm.startPrank(user);
oneShot.approve(address(rapBattle), 0);
cred.approve(address(rapBattle), 3);
rapBattle.goOnStageOrBattle(0, 3);
vm.stopPrank();
vm.startPrank(challenger);
rapBattle.goOnStageOrBattle(1, 3);
vm.stopPrank();
console.log("user.balance: ", cred.balanceOf(user));
console.log("challenger.balance: ", cred.balanceOf(challenger));
assertEq(cred.balanceOf(user), userBalanceBefore - 3);
assertEq(cred.balanceOf(challenger), challengerBalanceBefore + 3);
}
forge test --mt testGoOnStageOrBattleIfChallengerNotApprovingTokens -vvvv
No files changed, compilation skipped
Running 1 test for test/OneShotTest.t.sol:RapBattleTest
[PASS] testGoOnStageOrBattleIfChallengerNotApprovingTokens() (gas: 600311)
Logs:
user.balance: 1
challenger.balance: 7
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.53ms
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)
Impact
This vulnerability allows a challenger to win without any approval of tokens.
This will give a challenger winning edge over user.
Only challenger can win and get user's CredToken.
Tools Used
Recommendations
function _battle(uint256 _tokenId, uint256 _credBet) internal {
address _defender = defender;
require(defenderBet == _credBet, "RapBattle: Bet amounts do not match");
uint256 defenderRapperSkill = getRapperSkill(defenderTokenId);
uint256 challengerRapperSkill = getRapperSkill(_tokenId);
uint256 totalBattleSkill = defenderRapperSkill + challengerRapperSkill;
uint256 totalPrize = defenderBet + _credBet;
uint256 random =
uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao, msg.sender))) % totalBattleSkill;
// Reset the defender
defender = address(0);
emit Battle(msg.sender, _tokenId, random < defenderRapperSkill ? _defender : msg.sender);
// If random <= defenderRapperSkill -> defenderRapperSkill wins, otherwise they lose
if (random <= defenderRapperSkill) {
// We give them the money the defender deposited, and the challenger's bet
credToken.transfer(_defender, defenderBet);
credToken.transferFrom(msg.sender, _defender, _credBet);
} else {
// Otherwise, since the challenger never sent us the money, we just give the money in the contract
+ credToken.transferFrom(msg.sender, address(this), _credBet);
+ oneShotNft.transferFrom(msg.sender, address(this), _tokenId);
+ credToken.transfer(msg.sender, totalPrize);
- credToken.transfer(msg.sender, _credBet);
}
totalPrize = 0;
// Return the defender's NFT
+ if (random > defenderRapperSkill) oneShotNft.transferFrom(address(this), msg.sender, _tokenId);
oneShotNft.transferFrom(address(this), _defender, defenderTokenId);
}
