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

`RapBattle.goOnStageOrBattle` allows users to participate in a battle without owning `Credibility` tokens.

Ritos

Summary

Function goOnStageOrBattle in contract RapBattle don't do appropriate checks for _credBet parameter which leads to stolen Credibility tokens from users.

Vulnerability Details

Function goOnStageOrBattle in contract RapBattle if defender != address(0) do not check the Credibility balance of user and allowance from user to RapBattle to spend Credibility tokens.

POC

Assume that defender in the RapBattle contract equals address(0), which means that there is no one on stage right now.

1 First user call RapBattle.goOnStageOrBattle with some _credBet.

2 Attacker call RapBattle.goOnStageOrBattle with the same _credBet without having it on Credibility contract.

3 Function RapBattle.__battle is triggered and then we have the following 2 options:

3.1 defender is win:

credToken.transferFrom is reverted because of attacker do not call credToken.approve with the same value as _credBet and spender equal address of RapBattle contract

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

3.2 defender is loss:

Attacker stole the defenders tokens without putting at risk his founds in Credibility contract.

4 An attacker can exploit one or more vulnerabilities along with the described vulnerability to reduce the user's probability of winning to 0% :

4.1 Attacker can send _tokenId without ownership with the biggest amount from getRapperSkill which reduces the possibility of winning for the user by 10%

4.2 Attacker can then exploit weakrandomness. Picking salt for create2 to produce contract with fitting address that 100% win battle.
In case of usage this vulnerability, 4.1 needs to reduce the number of salt needed to brute force.

4.3 Even if attacker do not want exploit 4.2 he can keep call RapBattle.goOnStageOrBattle in the following blocks, until he wins.

Impact

Attacker can stole the user Credibility tokens without putting at risk his Credibility tokens.

Tools Used

Manual review.

Recommendations

Make the following changes in RapBattle.sol

https://github.com/Cyfrin/2024-02-one-shot/blob/47f820dfe0ffde32f5c713bbe112ab6566435bf7/src/RapBattle.sol#L38C1-L52C6

function goOnStageOrBattle(uint256 _tokenId, uint256 _credBet) external {
+ credToken.transferFrom(msg.sender, address(this), _credBet);
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);
}
}

https://github.com/Cyfrin/2024-02-one-shot/blob/47f820dfe0ffde32f5c713bbe112ab6566435bf7/src/RapBattle.sol#L54C1-L81C6

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.transfer(_defender, totalPrize);
- 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.transfer(msg.sender, _credBet);
+ credToken.transfer(msg.sender, totalPrize);
}
totalPrize = 0;
// Return the defender's NFT
oneShotNft.transferFrom(address(this), _defender, defenderTokenId);
}
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.