First Flight #10: One Shot

Description:

• The protocol documentation states "Staked Rapper NFTs will earn 1 Cred ERC20/day staked up to 4 maximum". However, after a user unstake the NFT, he can stake again breaking the maximum reward established for staked NFTs.

• Impact:

  • A user can take advantage of this vulnerability and collect rewards consistently.

• Proof of Concept:

  Add the code below in `OneShotTest.t.sol`

  function testIfAUserCanReStakeTheNFT() public mintRapper{

  vm.startPrank(user);

  oneShot.approve(address(streets), 0);

  streets.stake(0);

  assert(

  streets.onERC721Received(address(0), user, 0, "")

  == bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"))

  );

  ​

  uint256 userBalanceBeforeUnstake = cred.balanceOf(user);

  ​

  console.log(userBalanceBeforeUnstake);

  ​

  vm.warp((4* 1 days) + 1);

  ​

  streets.unstake(0);

  ​

  uint256 userBalancesAfterUnstake = cred.balanceOf(user);

  ​

  assertEq(userBalancesAfterUnstake, 4);

  oneShot.approve(address(streets), 0);

  streets.stake(0);

  assert(

  streets.onERC721Received(address(0), user, 0, "")

  == bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"))

  );

  ​

  vm.warp((8*1 days) + 1);

  ​

  streets.unstake(0);

  ​

  uint256 userBalanceAfterSecondStake = cred.balanceOf(user);

  ​

  assertEq(userBalanceAfterSecondStake, 8);

  }

• Recommendation:

  Add the following changes into `Streets.sol::stake`

  + error Streets__StakingRewardsAlreadyCollected();

  function stake(uint256 tokenId) external {

  + if(stakes[tokenId].startTime != 0){

  + revert Streets__StakingRewardsAlreadyCollected();

  + }

  ​

  stakes[tokenId] = Stake(block.timestamp, msg.sender);

  emit Staked(msg.sender, tokenId, block.timestamp);

  oneShotContract.transferFrom(msg.sender, address(this), tokenId);

  }

  ​
  Add the following changes into `Streets.sol::unstake`

  function unstake(uint256 tokenId) external {

  require(stakes[tokenId].owner == msg.sender, "Not the token owner");

  uint256 stakedDuration = block.timestamp - stakes[tokenId].startTime;

  uint256 daysStaked = stakedDuration / 1 days;

  ​

  // Assuming RapBattle contract has a function to update metadata properties

  IOneShot.RapperStats memory stakedRapperStats = oneShotContract.getRapperStats(tokenId);

  ​

  emit Unstaked(msg.sender, tokenId, stakedDuration);

  - delete stakes[tokenId]; // Clear staking info

  ​

  // Apply changes based on the days staked

  if (daysStaked >= 1) {

  stakedRapperStats.weakKnees = false;

  credContract.mint(msg.sender, 1);

  }

  if (daysStaked >= 2) {

  stakedRapperStats.heavyArms = false;

  credContract.mint(msg.sender, 1);

  }

  if (daysStaked >= 3) {

  stakedRapperStats.spaghettiSweater = false;

  credContract.mint(msg.sender, 1);

  }

  if (daysStaked >= 4) {

  stakedRapperStats.calmAndReady = true;

  credContract.mint(msg.sender, 1);

  }

  ​

  // Only call the update function if the token was staked for at least one day

  if (daysStaked >= 1) {

  oneShotContract.updateRapperStats(

  tokenId,

  stakedRapperStats.weakKnees,

  stakedRapperStats.heavyArms,

  stakedRapperStats.spaghettiSweater,

  stakedRapperStats.calmAndReady,

  stakedRapperStats.battlesWon

  );

  }

  ​

  // Continue with unstaking logic (e.g., transferring the token back to the owner)

  oneShotContract.transferFrom(address(this), msg.sender, tokenId);

  }

  ​