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

A challenger can win a battle without no risk

y0ng0p3

Summary

A challenger can win a battle without betting no CRED.

Vulnerability Details

When going on stage, a user can be either the defender or the challenger if there's no defender yet.
In the first case, their RPR NFT is transferred to the arbiter (RapBattle.sol) and so their Credibility tokens (CRED).
However, in the second case, these precautions have not been taken. Their approvals to RapBattle is not even checked.
Even so, the challenger could send their CRED to another account before the battle ends, risking nothing in the end.

If the challenger (msg.sender) wins the battle, they will earn promised rewards.
If the challenger losses the battle, the CreToken transfer from their account could revert may be due to not enough funds, or no approval, or credToken unexpected behaviour.
This would revert the transaction and annul the battle even if we already had a winner.
This is unfair to the defender, who risks losing CRED in the battle.

POC

Put the test excerpt below in `test/OneShotTest.t.sol`
// Test that a challenger can go on stage without no risk
function testChallengerWithNoRisk() public twoSkilledRappers {
vm.startPrank(user);
oneShot.approve(address(rapBattle), 0);
cred.approve(address(rapBattle), 3);
console.log("User allowance before battle:", cred.allowance(user, address(rapBattle)));
rapBattle.goOnStageOrBattle(0, 3);
vm.stopPrank();
​
vm.startPrank(challenger);
oneShot.approve(address(rapBattle), 1);
// cred.approve(address(rapBattle), 3);
console.log("Challenger allowance before battle:", cred.allowance(challenger, address(rapBattle)));
assert(cred.allowance(challenger, address(rapBattle)) == 0);
​
rapBattle.goOnStageOrBattle(1, 3);
vm.stopPrank();
​
assert(cred.balanceOf(challenger) >= 4);
assert(cred.balanceOf(user) <= 4);
}

In the terminal, run the following command:

  • forge test --mt testChallengerWithNoRisk

Impact

Challenger can go on stage and win a battle without risk.

Tools Used

Manual review

Recommendations

As with the defender, transfer the challenger's `CRED` and `RPR` to the `RapBattle` smart contract before the battle.
File: src/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);
+ oneShotNft.transferFrom(msg.sender, address(this), _tokenId);
_battle(_tokenId, _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.