Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: high
Valid

Missing token transfer allows user to never lose any tokens

Summary

Missing token transfer allows user to never lose any tokens.

Vulnerability Details

goOnStageOrBattle and _battle functions do not transfer the challenger's credTokens for bet. Attacker can use this missing transfer in a way such that attacker does not give RapBattle contract allowance. Without the allowance, RapBattle contract cannot transfer credTokens to winner if defender wins and the transaction will revert due to the line given below.

credToken.transferFrom(msg.sender, _defender, _credBet);

But if, attacker wins the battle, he can get defender's credTokens from defenderBet as defender has already given RapBattle contract allowance to transfer his tokens. This way, challenger never loses any credTokens irrespective of the result of Rap Battle, whether he wins or loses.

Impact

  1. If the attacker loses the bet, he will not lose credTokens.

  2. Defender can never win.

  3. Battle can only proceed further when attacker wins the battle and gain the credTokens.

Tools Used

Manual Review

Recommendations

Add the below code in goOnStageOrBattle 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 {
+ credToken.transferFrom(msg.sender, address(this), _credBet);
_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.