Under a specific set of circumstances, a OneShot NFT holder can lose ownership of their OneShot NFT in a rap battle and it cannot be reclaimed without protocol intervention.
If an NFT holder enters a rap battle as the 1st entrant ("defender"), the RapBattle::goOnStageOrBattle
function transfers ownership of that NFT to the RapBattle contract. A 2nd entrant ("challenger") enters the battle and matches the bet, but it is an amount of CRED which he does not possess. If the defender wins, the RapBattle::goOnStageOrBattle
function will revert when it attempts to transfer CRED from the challenger to the defender. This occurs before the contract transfers the defender's NFT back to the defender, resulting in the NFT still being owned by the RapBattle contract.
Medium - Impact is Medium due to defender losing the NFT as well as the time & gas used to stake/unstake & enter battle. Likelihood is Low due to the number of factors needing to align for this attack to work.
Visual Studio Code, Foundry, manual review
Add the following test function to OneShotTest.t.sol:
Execute the test until it fails - indicating the user is no longer the owner of the NFT.
Note: I had to hard code the value of the random variable in RapBattle::_battle
to trigger a win for the defender, as in my environment the number generated kept landing on a challenger win.
One way to address this is to verify in RapBattle::goOnStageOrBattle
that the caller has sufficient CRED to cover their bet.
This can be done by adding this line of code to the top of that function:
A better way would be to simplify the protocol design by not transferring the NFT from the defender to begin with and therefore remove the need to transfer it back. This would give the defender the same treatment as the challenger (whose NFT is not transferred to RapBattle). Plus it would save gas.
To implement this change in design, simply remove these lines from the RapBattle contract:
https://github.com/Cyfrin/2024-02-one-shot/blob/47f820dfe0ffde32f5c713bbe112ab6566435bf7/src/RapBattle.sol#L46
https://github.com/Cyfrin/2024-02-one-shot/blob/47f820dfe0ffde32f5c713bbe112ab6566435bf7/src/RapBattle.sol#L80
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.