hats-finance / Fenix-Finance-0x83dbe5aa378f3ce160ed084daf85f621289fb92f

0 stars 0 forks source link

Not calling `update_period` for a week will fully lose the rewards for 2 weeks #18

Open hats-bug-reporter[bot] opened 8 months ago

hats-bug-reporter[bot] commented 8 months ago

Github username: @deadrosesxyz Twitter username: @deadrosesxyz Submission hash (on-chain): 0xa064fe8d4eacbc9c23301e44ef16a16ec72b84acd6ae7cc0a7cdfaaf344dc981 Severity: medium

Description: Description\ Not calling update_period for a week will fully lose the rewards for 2 weeks

Attack Scenario\ Let's look at update_period

    function update_period() external returns (uint256) {
        uint256 _period = active_period;
        if (block.timestamp >= _period + WEEK && isStarted) {
            // only trigger if new week
            _period = (block.timestamp / WEEK) * WEEK;
            active_period = _period;

            if (!isFirstMint) {
                weekly = weekly_emission();
            } else {
                isFirstMint = false;
            }

            uint256 weeklyCache = weekly;
            uint256 teamEmissions = (weeklyCache * teamRate) / PRECISION;

            uint256 gauge = weeklyCache - teamEmissions;

            uint256 currentBalance = fenix.balanceOf(address(this));
            if (currentBalance < weeklyCache) {
                fenix.mint(address(this), weeklyCache - currentBalance);
            }

            require(fenix.transfer(owner(), teamEmissions));

            fenix.approve(address(voter), gauge);
            voter.notifyRewardAmount(gauge);

            emit Mint(msg.sender, weeklyCache, circulating_supply());
        }
        return _period;
    }

First thing that we can see is that the update_period is not in a loop, so if the function isn't called for a week, tokens will not be minted at all for that week and that week will be skipped ,resulting in loss of funds for a week. The problem with the 2nd week actually comes within the Voter contract.

    function notifyRewardAmount(uint256 amount) external {
        require(msg.sender == minter, "!minter");
        IERC20Upgradeable(base).safeTransferFrom(msg.sender, address(this), amount);

        uint256 _totalWeight = totalWeightAt(_epochTimestamp() - 1 weeks); // minter call notify after updates active_period, loads votes - 1 week // problmatic line 

        uint256 _ratio = 0;

        if (_totalWeight > 0) _ratio = (amount * 1e18) / _totalWeight; // 1e18 adjustment is removed during claim
        if (_ratio > 0) {
            index += _ratio;
        }

        emit NotifyReward(msg.sender, base, amount);
    }

When notifiying the reward, it will get the weight at active_period - 1 weeks. Since we skipped a period, it means that it will get the balance at the week for which there were no updates, which has totalWeight 0. Since the totalWeight is 0, rewards will not be distributed and will be lsot.

Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional)

BohdanHrytsak commented 8 months ago

Thank you for the submission.

This problem is known and has been mentioned in the following examples: https://github.com/Satsyxbt/Fenix/?tab=readme-ov-file#publicly-known-issues