code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1077 https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1098

Vulnerability details

Impact

The impact of this is high as a user who has staked an S1 or S2 Citizen NFT can stake specific amounts of BYTES tokens with that NFT without receiving any additional rewards that they are entitled to. The user will not notice that this is happening and so be cheated out of rewards by the staking contract. Fundamentally this breaks the invariant (in certain scenarios) that e.g. staking a single time of n BYTES tokens will not yield that same outcome as staking 100 times of n/100 BYTES tokens.

Proof of Concept

POC including preliminary state and steps (foundry test):

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

// utilities
import {Test} from "forge-std/Test.sol";
import {console} from "forge-std/console.sol";

// importing project files
import {LPToken} from "../test/lpToken.sol";
import {BYTES2} from "../staking/BYTES2.sol";
import {NeoTokyoStaker} from "../staking/NeoTokyoStaker.sol";
import {S2NftStub} from "./S2NftStub.sol";

// EXPLOIT DESC: this is the POC for the high severity finding of when you are staking
//                BYTES2 tokens for a S2 (could be S1 too) citizen that if you don't stake
//                enough tokens then it results in you not receiving any points for your deposit
contract TestNoRewards2 is Test {

    // protocol users
    address admin = makeAddr('admin'); // project admin
    address user = makeAddr('user'); // user address

    // protocol contracts
    LPToken lpToken;
    S2NftStub S2citizen; // stub of the S2 Citizen NFT, just ERC721 with a mint function for testing
    BYTES2 bytes2Token;
    NeoTokyoStaker neoTokyoStaker;

    function setUp() public {
        // deploy the LP token - this is technically unnecessary for this POC
        vm.prank(admin);
        lpToken = new LPToken();

        // deploy the S2 Citizen NFT stub
        vm.prank(admin);
        S2citizen = new S2NftStub();
        vm.prank(admin);
        S2citizen.mintForUser(user,10); // minting the user n number of S2 citizen NFTs

        // deploy the BYTES2 token
        vm.prank(admin);
        bytes2Token = new BYTES2(
            address(0),address(0),address(0),address(0)
        );
        // minting n BYTES2 tokens for the user
        // --there was a mint(..) function added to the BYTES2 token to facilitate this testing
        vm.prank(admin);
        bytes2Token.mint(user,1_000e18);

        // deploy the staking contract
        vm.prank(admin);
        neoTokyoStaker = new NeoTokyoStaker(
            address(bytes2Token),address(0),address(S2citizen),address(lpToken),
            address(0),address(0),1_000e18,1_000e18
        );

        // setting the staker address in BYTES2 token contract
        // --rewards for staking S2 citizens starts in 10 seconds & issues 1e18 in BYTES2 per sec in rewards
        vm.prank(admin);
        bytes2Token.changeStakingContractAddress(address(neoTokyoStaker));

        // configure the reward emission using configurePools(..)
        NeoTokyoStaker.RewardWindow memory rewardWindow = NeoTokyoStaker.RewardWindow(
            {startTime:10,reward:1e18}
        );
        NeoTokyoStaker.RewardWindow[] memory rewardWindows = new NeoTokyoStaker.RewardWindow[](1);
        rewardWindows[0]=rewardWindow;

        NeoTokyoStaker.PoolConfigurationInput memory poolConfigurationInputS2 = NeoTokyoStaker.PoolConfigurationInput(
            {assetType:NeoTokyoStaker.AssetType.S2_CITIZEN,daoTax:0,rewardWindows:rewardWindows}
        );

        NeoTokyoStaker.PoolConfigurationInput[] memory poolConfigurationInputs = new NeoTokyoStaker.PoolConfigurationInput[](1);
        poolConfigurationInputs[0]=poolConfigurationInputS2;

        vm.prank(admin);
        neoTokyoStaker.configurePools(poolConfigurationInputs);

        // configure the BYTES pool
        poolConfigurationInputS2 = NeoTokyoStaker.PoolConfigurationInput(
            {assetType:NeoTokyoStaker.AssetType.BYTES,daoTax:0,rewardWindows:rewardWindows}
        );
        poolConfigurationInputs[0]=poolConfigurationInputS2;

        vm.prank(admin);
        neoTokyoStaker.configurePools(poolConfigurationInputs);

        // configure the timelock options
        // --for S2_CITIZEN tokens, must lock up tokens for at least 1 second(s), in return recieve multiplier of 1
        uint256 timelockOption = 1; // timeLock duration = 1
        timelockOption = timelockOption << 128;
        timelockOption = timelockOption | 1; // timeLock multiplier = 1

        uint256[] memory timelockIds = new uint256[](1);
        timelockIds[0]=0;

        uint256[] memory encodedSettings = new uint256[](1);
        encodedSettings[0]=timelockOption;

        vm.prank(admin);
        neoTokyoStaker.configureTimelockOptions(
            NeoTokyoStaker.AssetType.S2_CITIZEN,timelockIds,encodedSettings
        );

        // configure the timelock options for BYTES
        vm.prank(admin);
        neoTokyoStaker.configureTimelockOptions(
            NeoTokyoStaker.AssetType.BYTES,timelockIds,encodedSettings
        );

        // setting block.timestamp so that the S2 citizen reward window is active
        vm.warp(block.timestamp+10); // 1+10
    }

    function testNoRewards() public {
        // approving the staking contract to use all of the user's S2 citizens
        vm.prank(user);
        S2citizen.setApprovalForAll(address(neoTokyoStaker),true);

        // user stakes citizenId=1 S2 citizen NFT - using the timelock w/ multiplier = 1
        // -- with the timelock config, the user will get 1 point for simply staking the S2 citizen
        vm.prank(user);
        neoTokyoStaker.stake(
            NeoTokyoStaker.AssetType.S2_CITIZEN,0,1,0,0
        );

        // user approves the BYTES2 token for the neoTokyoStaker contract
        vm.prank(user);
        bytes2Token.approve(address(neoTokyoStaker),type(uint256).max);

        // gathering starting points & BYTES2 token for the S2 citizen staking position
        (uint256 startingStakedBytes,,uint256 startingPoints) = neoTokyoStaker.stakedS2(user,1);

        // user stakes some BYTES2 tokens (2e18-1) for their staked S2 citizen NFT
        vm.prank(user);
        neoTokyoStaker.stake(
            NeoTokyoStaker.AssetType.BYTES,0,2e18-1,1,2
        );

        // checking the amount of BYTES2 staked and points provided to user for S2 citizen (id=1)
        (uint256 endStakedBytes,,uint256 endPoints) = neoTokyoStaker.stakedS2(user,1);

        // invalid state - user has staked BYTES2 tokens but not received any additional points
        // effectively the user is being cheated out of staking rewards
        assertGt(endStakedBytes,startingStakedBytes);
        assertEq(endPoints,startingPoints);
    }
}

Tools Used

Manual review

Recommended Mitigation Steps

Ensure that the user is only able to stake BYTES tokens for a S1 or S2 citizen in increments which do not result in precision loss (e.g. considering that _BYTES_PER_POINT is fixed at 200, a user staking less than 2e18 BYTES tokens will not receive any additional points).