sherlock-audit / 2024-05-sophon-judging

7 stars 6 forks source link

0xNoodle - Reentrant calls to updatePool() and massUpdatePools() #57

Closed sherlock-admin2 closed 5 months ago

sherlock-admin2 commented 5 months ago

0xNoodle

high

Reentrant calls to updatePool() and massUpdatePools()

Summary

https://github.com/sherlock-audit/2024-05-sophon/blob/main/farming-contracts/contracts/farm/SophonFarming.sol#L411

https://github.com/sherlock-audit/2024-05-sophon/blob/main/farming-contracts/contracts/farm/SophonFarming.sol#L399

The updatePool() and massUpdatePools() functions in the SophonFarming contract are responsible for updating reward calculations for each pool. These functions update the accPointsPerShare and lastRewardBlock variables, which are critical for accurate reward distribution. However, if these functions are called repeatedly in quick succession without proper reentrancy protection, it could lead to inconsistencies in the reward calculations.

Vulnerability Detail

If an attacker calls updatePool() or massUpdatePools() repeatedly in quick succession, the accPointsPerShare and lastRewardBlock variables may be updated multiple times within the same block. This can lead to incorrect reward calculations for users who interact with the contract during this period.

An attacker could potentially manipulate the timing of these calls to create a situation where the reward calculations are inconsistent, leading to incorrect reward distributions.

Impact

Users receive incorrect rewards due to temporary inconsistencies in the accPointsPerShare and lastRewardBlock variables.

An attacker could exploit these inconsistencies to manipulate reward distributions, potentially leading to financial losses for other users.

Code Snippet

contract SophonFarmingTest1 is Test {
    string internal mnemonic = "test test test test test test test test test test test junk";
    string internal envMnemonicKey = "MNEMONIC";

    address internal deployer;
    address internal account1 = address(0x1);
    address internal account2 = address(0x2);
    address internal account3 = address(0x3);
    uint256 internal permitUserPK = 0x0000000000000000000000000000000000000000000000000000000000000001;

    SophonFarmingProxy public sophonFarmingProxy;
    SophonFarming public sophonFarming;
    address public implementation;
    SophonFarmingHarness public harnessImplementation;

    MockERC20 internal mock0;
    MockERC20 internal mock1;

    MockWETH internal weth;
    MockStETH internal stETH;
    MockWstETH internal wstETH;
    MockERC20 internal eETH;
    MockeETHLiquidityPool internal eETHLiquidityPool;
    MockWeETH internal weETH;
    MockERC20 internal dai;
    MockSDAI internal sDAI;

    uint256 internal wstETHAllocPoint;
    uint256 internal sDAIAllocPoint;
    uint256 internal pointsPerBlock;
    uint256 internal startBlock;
    uint256 internal boosterMultiplier;

    uint256 maxUint = type(uint256).max;

    error Unauthorized();
    error OwnableUnauthorizedAccount(address account);
    error InsufficientBalance();

    // Helper functions
    function StETHRate(uint256 amount) internal pure returns (uint256) {
        return amount / 1001 * 1000;
    }

    function WstETHRate(uint256 amount) internal view returns (uint256) {
        return amount * wstETH.tokensPerStEth() / 1e18;
    }

    function eETHLPRate(uint256 amount) internal pure returns (uint256) {
        return amount / 1001 * 1000;
    }

    function WeETHRate(uint256 amount) internal view returns (uint256) {
        return amount * weETH.tokensPereETH() / 1e18;
    }

    function getUserInfo(uint256 poolId, address user) internal view returns (SophonFarmingState.UserInfo memory) {
        SophonFarmingState.UserInfo memory userInfo;
        (userInfo.amount, userInfo.boostAmount, userInfo.depositAmount, userInfo.rewardSettled, userInfo.rewardDebt) =
            sophonFarming.userInfo(poolId, user);

        return userInfo;
    }

    // Setup
    function setUp() public {
      //  string memory envMnemonic = vm.envString(envMnemonicKey);
    //    if (keccak256(abi.encode(envMnemonic)) != keccak256(abi.encode(""))) {
   //         mnemonic = envMnemonic;
   //     }

    // deployer = vm.addr(vm.deriveKey(mnemonic, 0));

        // Deal and start prank
      //  vm.deal(deployer, 1000000e18);
     //   vm.startPrank(deployer);

        // mock WETH
        weth = new MockWETH();

        // mock stETH
        stETH = new MockStETH();

        // mock wstETH
        wstETH = new MockWstETH(stETH);
        wstETHAllocPoint = 20000;

        eETH = new MockERC20("Mock eETH Token", "MockeETH", 18);

        eETHLiquidityPool = new MockeETHLiquidityPool(eETH);

        weETH = new MockWeETH(eETH);

        // mock DAI
        dai = new MockERC20("Mock Dai Token", "MockDAI", 18);
        dai.mint(address(this), 1000000e18);

        // mock sDAI
        sDAI = new MockSDAI(dai);
        sDAIAllocPoint = 20000;

        // Set up for SophonFarming
        pointsPerBlock = 25e18;
        startBlock = block.number;
        boosterMultiplier = 2e18;

        // Deploy implementation
        implementation = address(
            new SophonFarming(
                [
                    address(dai),
                    address(sDAI),
                    address(weth),
                    address(stETH),
                    address(wstETH),
                    address(eETH),
                    address(eETHLiquidityPool),
                    address(weETH)
                ]
            )
        );

        // Deploy proxy
        sophonFarmingProxy = new SophonFarmingProxy(implementation);

        // Grant the implementation interface to the proxy
        sophonFarming = SophonFarming(payable(address(implementation)));

        // Initialize SophonFarming
        sophonFarming.initialize(wstETHAllocPoint, sDAIAllocPoint, pointsPerBlock, startBlock, boosterMultiplier);

        weth.approve(address(sophonFarming), maxUint);
        stETH.approve(address(sophonFarming), maxUint);
        wstETH.approve(address(sophonFarming), maxUint);
        dai.approve(address(sophonFarming), maxUint);
        sDAI.approve(address(sophonFarming), maxUint);
        stETH.approve(address(wstETH), maxUint);
        dai.approve(address(sDAI), maxUint);

        // Mint some tokens
        weth.deposit{value: 0.01e18}();
        stETH.submit{value: 0.02e18}(address(sophonFarming));
       // wstETH.wrap(stETH.balanceOf(deployer) / 2);
      //  dai.mint(deployer, 1000e18);
      //  sDAI.deposit(dai.balanceOf(deployer) / 2, deployer);

        sophonFarming.setEndBlock(maxUint - 1000, 1000);

        // Deploy harness implementation
        harnessImplementation = new SophonFarmingHarness(
            [
                address(dai),
                address(sDAI),
                address(weth),
                address(stETH),
                address(wstETH),
                address(eETH),
                address(eETHLiquidityPool),
                address(weETH)
            ]
        );

        harnessImplementation.setEndBlock(maxUint - 1000, 1000);

        vm.stopPrank();

}

    function testFuzz_DepositWeth_Boosted1(uint256 amountToDeposit, uint256 boostFraction) public {
    vm.assume(amountToDeposit > 1e6 && amountToDeposit <= 1_000_000_000e18);
    vm.assume(boostFraction > 0 && boostFraction <= 10);

    uint256 wsthDepositedAmount = WstETHRate(StETHRate(amountToDeposit));
    uint256 amountToBoost = amountToDeposit / boostFraction;
    uint256 boostAmount = amountToBoost * wsthDepositedAmount / amountToDeposit;
    uint256 finalBoostAmount = boostAmount * sophonFarming.boosterMultiplier() / 1e18;
    uint256 poolId = sophonFarming.typeToId(SophonFarmingState.PredefinedPool.wstETH);

    vm.startPrank(account1);
    vm.deal(account1, amountToDeposit);
    weth.deposit{value: amountToDeposit}();
    assertEq(weth.balanceOf(account1), amountToDeposit);

    weth.approve(address(sophonFarming), amountToDeposit);
    sophonFarming.depositWeth(amountToDeposit, amountToBoost, SophonFarmingState.PredefinedPool.wstETH);
    assertEq(weth.balanceOf(account1), 0);

    SophonFarmingState.UserInfo memory userInfo;
    (userInfo.amount, userInfo.boostAmount, userInfo.depositAmount, userInfo.rewardSettled, userInfo.rewardDebt) =
        sophonFarming.userInfo(poolId, account1);

    assertEq(userInfo.amount, wsthDepositedAmount - boostAmount + finalBoostAmount);
    assertEq(userInfo.boostAmount, finalBoostAmount);
    assertEq(userInfo.depositAmount, wsthDepositedAmount - boostAmount);
    assertEq(userInfo.rewardSettled, 0);
    assertEq(userInfo.rewardDebt, 0);
}

function test_UpdatePool_Scenario2(uint256 amountToDeposit, uint256 boostFraction) public {
    // Initial Deposit
    testFuzz_DepositWeth_Boosted1(amountToDeposit, boostFraction);

    // Initial State
    uint256 poolId = sophonFarming.typeToId(SophonFarmingState.PredefinedPool.wstETH);

    // Roll to block 100 and update pool
    vm.roll(100);
    sophonFarming.updatePool(poolId);

    // Set initial state directly
    SophonFarmingState.PoolInfo memory poolInfo = sophonFarming.getPoolInfo()[poolId];
    poolInfo.lastRewardBlock = 100;
    poolInfo.accPointsPerShare = 10;
    uint256 lpSupply = 1000e18; // Assume some LP supply
    uint256 pointsPerBlocks = 25e18;
    uint256 allocPoint = poolInfo.allocPoint;

    // First Call to updatePool()
    vm.roll(101);
    sophonFarming.updatePool(poolId);
    poolInfo = sophonFarming.getPoolInfo()[poolId];
    uint256 blockMultiplier = 101 - 100;

    uint256 pointReward = blockMultiplier * pointsPerBlocks * allocPoint / sophonFarming.totalAllocPoint();
    uint256 expectedAccPointsPerShare = 10 + (pointReward * 1e18 / lpSupply);

    assertEq(poolInfo.accPointsPerShare, expectedAccPointsPerShare);
    assertEq(poolInfo.lastRewardBlock, 101);

    // Second Call to updatePool() within the same block
    sophonFarming.massUpdatePools();
    poolInfo = sophonFarming.getPoolInfo()[poolId];
    blockMultiplier = 101 - 101;
    pointReward = blockMultiplier * pointsPerBlocks * allocPoint / sophonFarming.totalAllocPoint();
    expectedAccPointsPerShare = expectedAccPointsPerShare; // No change expected

    assertEq(poolInfo.accPointsPerShare, expectedAccPointsPerShare);
    assertEq(poolInfo.lastRewardBlock, 101);

    // Third Call to updatePool() in the next block
    vm.roll(102);
    //sophonFarming.updatePool(poolId);
     sophonFarming.massUpdatePools();
    poolInfo = sophonFarming.getPoolInfo()[poolId];
    blockMultiplier = 102 - 101;
    pointReward = blockMultiplier * pointsPerBlocks * allocPoint / sophonFarming.totalAllocPoint();
    expectedAccPointsPerShare = expectedAccPointsPerShare + (pointReward * 1e18 / lpSupply);

    assertEq(poolInfo.accPointsPerShare, expectedAccPointsPerShare);
    assertEq(poolInfo.lastRewardBlock, 102);

}

}

Tool used

Manual Review

Recommendation

Use OpenZeppelin's ReentrancyGuard to prevent reentrant calls to updatePool() and massUpdatePools().

sherlock-admin4 commented 5 months ago

1 comment(s) were left on this issue during the judging contest.

0xmystery commented:

invalid because no hook is entailed