Summary
Winner Rapper metadata for winning battles isn't updated
Vulnerability Details
Each nft has stats which tracks there weakKnees , heavyArms, spaghettiSweater, calmAndReady and battlesWon. While first 4 are updated when Rapper go on streets (stakes his Rapper nft for number of days), but battlesWon stay same as currently there is no function in the contract which can update it.
This stats is important to keep track, how much battles a Rapper has won.
POC
In existing test suite, add following unit test -
function testWinnerBattleStatsUpdate() public twoSkilledRappers {
vm.startPrank(user);
oneShot.approve(address(rapBattle), 0);
cred.approve(address(rapBattle), 3);
console.log("Defender allowance before battle:", cred.allowance(user, address(rapBattle)));
rapBattle.goOnStageOrBattle(0, 3);
vm.stopPrank();
vm.startPrank(challenger);
oneShot.approve(address(rapBattle), 1);
cred.approve(address(rapBattle), 3);
console.log("Challenger allowance before battle:", cred.allowance(challenger, address(rapBattle)));
vm.recordLogs();
rapBattle.goOnStageOrBattle(1, 3);
vm.stopPrank();
Vm.Log[] memory entries = vm.getRecordedLogs();
address winner = address(uint160(uint256(entries[0].topics[2])));
uint256 winnerBalance = cred.balanceOf(winner);
assert(cred.balanceOf(winner) == 7);
if(winner == user){
stats = oneShot.getRapperStats(0);
console2.log("defender won:", winnerBalance);
console2.log("battles won:", stats.battlesWon);
} else {
stats = oneShot.getRapperStats(1);
console2.log("challenger won:", winnerBalance);
console2.log("battles won:", stats.battlesWon);
}
}
When we run forge test --mt testWinnerBattleStatsUpdate in terminal, it results following -
[⠢] Compiling...
[⠢] Compiling 1 files with 0.8.23
[⠆] Solc 0.8.23 finished in 2.02s
Compiler run successful!
Ran 1 test for test/OneShotTest.t.sol:RapBattleTest
[PASS] testWinnerBattleStatsUpdate() (gas: 665780)
Logs:
Defender allowance before battle: 3
Challenger allowance before battle: 3
challenger won: 7
battles won: 0
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.37ms
Ran 1 test suite in 1.37ms: 1 tests passed, 0 failed, 0 skipped (1 total tests)
Impact
User battle won status doesn't update, so people can't see how much battles a particular rapper has won.
Tools Used
Manual Review
Recommendations
Since only streets contract is allowed to update metadata so add a function there, which can be called by RapBattle contract only.
add following code in streets contract
+ address private rapBattle;
/// initialize rapBattle value in constructor
constructor(address _oneShotContract, address _credibilityContract,
+ _rapBattle) {
oneShotContract = IOneShot(_oneShotContract);
credContract = Credibility(_credibilityContract);
+ rapBattle = _rapBattle;
}
function updateBattleWonForRapper (uint256 _nftId) external {
require(msg.sender == rapBattle, "Only RapBattle");
IOneShot.RapperStats memory stakedRapperStats = oneShotContract.getRapperStats(tokenId);
stakedRapperStats.battlesWon += 1;
}
one done, add following code and update the _battle function on RapBattle contract as given below
+ interface IStreets{
+ function updateBattleWonForRapper (_nftId) external;
+ }
+ IStreets private street;
function _battle(uint256 _tokenId, uint256 _credBet) internal {
/// existing code
if (random <= defenderRapperSkill) {
// We give them the money the defender deposited, and the challenger's bet
credToken.transfer(_defender, defenderBet);
credToken.transferFrom(msg.sender, _defender, _credBet);
+ street.updateBattleWonForRapper(defenderTokenId);
} else {
// Otherwise, since the challenger never sent us the money, we just give the money in the contract
credToken.transfer(msg.sender, _credBet);
+ street.updateBattleWonForRapper(_tokenId);
}
/// existing code
}
+ function setStreetContract (address _street) external onlyOwner {
+ street = IStreets(_street);
+ }
