sherlock-audit / 2024-05-sophon-judging

7 stars 6 forks source link

VAD37 - Token Price fluctuation between pool sDAI, wstETH, weETH leading to unfair point allocation #22

Closed sherlock-admin3 closed 5 months ago

sherlock-admin3 commented 5 months ago

VAD37

medium

Token Price fluctuation between pool sDAI, wstETH, weETH leading to unfair point allocation

Summary

SophonFarming.sol has three main pools during initialization, with five additional token pools to be added later: wstETH, weETH, ezETH, rsETH, rswETH, uniETH, pufETH, and sDAI.

The DAI token is an outlier as its price is 3000 times higher than other tokens and fluctuates with the price of ETH. To ensure even distribution of pool points, the admin must set the DAI point allocation to 3,000,000, while other pools are set to 1,000.

This is not ideal, and the current admin configuration does not adequately compensate for this.

Allocation points can be updated later by the admin. However, because the ETH/DAI token price changes daily, the admin cannot constantly update the DAI allocation. This leads to unfair point allocation accrual between pools.

Vulnerability Detail

Here is the price of pool tokens converted: 1 ETH ~= 3800 sDAI 1 ETH = 1 stETH = 0.856 wstETH 1 ETH = 1 eETH = 1.03897 weETH

Here is code of point allocation calculation:

    function updatePool(uint256 _pid) public {
        PoolInfo storage pool = poolInfo[_pid];
        if (getBlockNumber() <= pool.lastRewardBlock) {
            return;
        }
        uint256 lpSupply = pool.amount;//deposit + boostAmount
        uint256 _pointsPerBlock = pointsPerBlock;//25e18
        uint256 _allocPoint = pool.allocPoint;//20000 could also be > 1e18
        if (lpSupply == 0 || _pointsPerBlock == 0 || _allocPoint == 0) {
            pool.lastRewardBlock = getBlockNumber();
            return;
        }
        uint256 blockMultiplier = _getBlockMultiplier(pool.lastRewardBlock, getBlockNumber());//block passed * 1e18
        uint256 pointReward =
            blockMultiplier *//300e18
            _pointsPerBlock *// * 25e18
            _allocPoint /    // * 20000
            totalAllocPoint; // 80000
        //@spread points evenly between pools
        pool.accPointsPerShare = pointReward /
            lpSupply +
            pool.accPointsPerShare;
        //accPointsPerShare+= 1-1000e18 *25e18 * 20000/80000 / 0-100000e18
        pool.lastRewardBlock = getBlockNumber();
    }

To simplify: accPointsPerShare += blockPassed * pointsPerBlock * poolAllocPoint / totalAllocPoint / totalTokenDepositAndBoostPoints

Each pool has its own allocation point divided by the total points of all pools. It is normal for some pools to be worth more than others depending on the admin configuration.

The admin must set the DAI pool allocation point 3000 times higher than other pools due to its price. 4000 sDAI is worth as much as 1 wstETH.

The current admin configuration for allocation points between the sDAI and wstETH pools is 20000:20000. https://github.com/sherlock-audit/2024-05-sophon/blob/main/farming-contracts/scripts/deploy.py#L256-L257

Because points are accumulated based on the amount of tokens deposited, DAI is obviously deposited much more than wstETH. Despite having the same points allocation, sDAI receives far fewer points than wstETH.

Additionally, because the price of ETH is not fixed, the conversion price between token pools is not fixed, leading to unfair point allocation between pools.

Impact

The sDAI pool accrues far fewer reward points than the wstETH pool due to price differences.

Because the ETH/DAI price fluctuates, users depositing in the DAI pool might receive unfair reward points compare to other pool.

Code Snippet

https://github.com/sherlock-audit/2024-05-sophon/blob/main/farming-contracts/contracts/farm/SophonFarming.sol#L412-L436 https://github.com/sherlock-audit/2024-05-sophon/blob/main/farming-contracts/scripts/deploy.py#L256-L257

Tool used

Manual Review

Recommendation

It is unnecessary to have DAI pool along with other ETH pool.

sherlock-admin4 commented 5 months ago

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

0xmystery commented:

invalid because the protocol team should be well aware of their intended design. Additionally, arbitraging and deciding on the supposed mix of pool farming is solely at users' discretion