Summary
A user can make battles using staked NFTs
Vulnerability Details
challenger is not obliged to transfer the NFT before the battle and there is no check for the NFT to be unstaked before battle with the defender takes place
Impact
This vulnerability allows users to make a battle with staked NFTs challenger thus being able to continue to gain benefits coming from the staking of their NFT.
This could jeopardize the entire battle-system as there are less incentives for users to try to make battles as defenders using their unstaked NFTs.
POC
contract MyTest is Test {
RapBattle rapBattle;
OneShot oneShot;
Streets streets;
Credibility cred;
IOneShot.RapperStats stats;
address user1;
address user2;
address user3;
uint256 start;
event balance (address, uint256);
function setUp() public {
oneShot = new OneShot();
cred = new Credibility();
streets = new Streets(address(oneShot), address(cred));
rapBattle = new RapBattle(address(oneShot), address(cred));
user1 = address(1);
user2 = address(2);
user3 = address(3);
oneShot.setStreetsContract(address(streets));
cred.setStreetsContract(address(streets));
start = block.timestamp;
}
function test_battle() public {
vm.startPrank(user1);
oneShot.mintRapper();
oneShot.approve(address(streets), 0);
streets.stake(0);
vm.stopPrank();
vm.startPrank(user2);
oneShot.mintRapper();
oneShot.approve(address(streets), 1);
streets.stake(1);
vm.stopPrank();
vm.startPrank(user3);
oneShot.mintRapper();
oneShot.approve(address(streets), 2);
streets.stake(2);
vm.stopPrank();
vm.warp(start+4 days);
vm.startPrank(user1);
streets.unstake(0);
vm.stopPrank();
vm.startPrank(user2);
streets.unstake(1);
vm.stopPrank();
uint256 balance2 = cred.balanceOf(user2);
uint256 balance3 = cred.balanceOf(user3);
vm.startPrank(user2);
cred.approve(address(rapBattle), 4);
oneShot.approve(address(rapBattle), 1);
rapBattle.goOnStageOrBattle(1,4);
vm.stopPrank();
vm.startPrank(user3);
cred.approve(address(rapBattle), 4);
rapBattle.goOnStageOrBattle(2,4);
vm.stopPrank();
uint256 balance_final1 = cred.balanceOf(user2);
uint256 balance_final2 = cred.balanceOf(user3);
assert(balance2+balance3 == balance_final1+balance_final2);
}
}
─ [27600] RapBattle::goOnStageOrBattle(2, 4)
│ ├─ [1183] OneShot::getRapperStats(1) [staticcall]
│ │ └─ ← (false, false, false, true, 0)
│ ├─ [1183] OneShot::getRapperStats(2) [staticcall]
│ │ └─ ← (true, true, true, false, 0)
│ ├─ emit Battle(challenger: 0x0000000000000000000000000000000000000003, tokenId: 2, winner: 0x0000000000000000000000000000000000000002)
│ ├─ [18516] Credibility::transfer(0x0000000000000000000000000000000000000002, 4)
│ │ ├─ emit Transfer(from: RapBattle: [0x5991A2dF15A8F6A256D3Ec51E99254Cd3fb576A9], to: 0x0000000000000000000000000000000000000002, amount: 4)
│ │ └─ ← true
## Tools Used
Foundry
## Recommendations
To oblige the `challenger` to tansfer the NFT before the battle takes place.
```diff
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 {
+ oneShotNft.transferFrom(msg.sender, address(this), _tokenId);
_battle(_tokenId, _credBet);
}
}
