Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: medium
Invalid

Anyone can call the `RapBattle::getRapperSkill` function

Summary

Anyone can call the RapBattle::getRapperSkill function with input argument the defenderTokenId and see the rapperSkill of this tokenId.

Vulnerability Details

In the RapBattle contract an event OnStage is emittet after someone calls the RapBattle::goOnStageOrBattle function and is waiting for an opponent. The event has arguments: defender, tokenId and credBet.

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

The function RapBattle::getRapperSkill is public and anyone can call it with input argument the tokenId of the defender.

@> function getRapperSkill(uint256 _tokenId) public view returns (uint256 finalSkill) {
IOneShot.RapperStats memory stats = oneShotNft.getRapperStats(_tokenId);
finalSkill = BASE_SKILL;
if (stats.weakKnees) {
finalSkill -= VICE_DECREMENT;
}
if (stats.heavyArms) {
finalSkill -= VICE_DECREMENT;
}
if (stats.spaghettiSweater) {
finalSkill -= VICE_DECREMENT;
}
//only this is right!!
if (stats.calmAndReady) {
finalSkill += VIRTUE_INCREMENT;
}
}

That allows an user to use the tokenId of the defender (or defenderTokenId) and see the rapperSkill. Then this user can decide if he has chance to win the battle and if he has, he will call the RapBattle::goOnStageOrBattle function. In that way the malicious user can choose the opponent for the battle according the rapperSkills. Additionally, there is a risk of front-running, where a user with knowledge of the defender's rapper skill may attempt to get their transaction included first by paying a higher gas price.

Impact

The function RapBattle::getRapperSkill can be called by anyone. Also, the address, tokenId and credBet of the user who is waiting for battle (defender) are emitted on event and can be seen by anyone. (Also, they are stored in public variables and anyone can see their values on blockchain.) So another user who wants to participate in battle can use the tokenId of the defender to see the defender's rapper skills.

There is a variable totalBattleSkill that serves as the range for the random number and how the individual skill levels influence the probability of winning. The higher a rapper's skill level relative to the totalBattleSkill, the greater the range of numbers that will result in their victory.

The following test function testSeeOpponentSkill shows this scenario. User calls goOnStageOrBattle and is waiting for an opponent. Bob see his tokenId and calls the function getRapperSkill with it. Then Bob can compare his rapper skills with the defender's skills and decide if he will participate in the battle (if he has a chance to win the opponent). Bob can pay higher gas price to front-run his transaction and participate in battle with the chosen opponent. In that way Bob will gain unfair advantage to choose the most appropriate opponent in order to win the battle.

You can add the test function in the file OneShot.t.test.sol and execute it with the foundry command: forge test --match-test "testSeeOpponentSkill" -vvv.

function testSeeOpponentSkill() public mintRapper {
vm.startPrank(user);
oneShot.approve(address(rapBattle), 0);
rapBattle.goOnStageOrBattle(0, 0);
vm.stopPrank();
address bob = makeAddress("bob");
//Bob can see the bet, addrrss and tokenId of the potential opponent
vm.startPrank(bob);
//In this test is used the getter function for the public variable in Solidity
//The tokenId can be seen also from the emitted event
uint256 tokenId = rapBattle.defenderTokenId();
//Bob can use the retrieved tokenId to see the rapperSkill of the opponent
uint256 finalSkill = rapBattle.getRapperSkill(tokenId);
console.log("Skill:", finalSkill);
}

Tools Used

Manual Review, Foundry

Recommendations

The RapBattle::getRapperSkill function is only called in RapBattle::_battle function. So the getRapperSkill function can be internal and nobody will be able to call it in order to see the opponent's rapperSkill.

- function getRapperSkill(uint256 _tokenId) public view returns (uint256 finalSkill) {
+ function getRapperSkill(uint256 _tokenId) internal view returns (uint256 finalSkill) {
IOneShot.RapperStats memory stats = oneShotNft.getRapperStats(_tokenId);
finalSkill = BASE_SKILL;
if (stats.weakKnees) {
finalSkill -= VICE_DECREMENT;
}
if (stats.heavyArms) {
finalSkill -= VICE_DECREMENT;
}
if (stats.spaghettiSweater) {
finalSkill -= VICE_DECREMENT;
}
//only this is right!!
if (stats.calmAndReady) {
finalSkill += VIRTUE_INCREMENT;
}
}
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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