hats-finance / Fenix-Finance-0x83dbe5aa378f3ce160ed084daf85f621289fb92f

0 stars 0 forks source link

`MerklGaugeMiddleman.sol::_notifyReward` does not update when other functions call #56

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

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

Github username: -- Twitter username: datamcrypto Submission hash (on-chain): 0xb78a15e2c90b3be4e69b32193fb384adba563a09eb4a56649e0c194d1d9ea0a6 Severity: high

Description: Description\ `function notifyReward(address gauge, uint256 amount) internal { DistributionParameters memory params = gaugeParams[gauge]; if (params.uniV3Pool == address(0)) revert InvalidParams();

    IERC20 tokenChache = token;

    if (amount_ == 0) amount_ = tokenChache.balanceOf(address(this));

    if (amount_ > 0) {
        params.epochStart = uint32(block.timestamp);
        params.amount = amount_;

        IDistributionCreator creatorCache = merklDistributionCreator;
        if (amount_ > creatorCache.rewardTokenMinAmounts(address(tokenChache)) * params.numEpoch) {
            uint256 distributionAmount = creatorCache.createDistribution(params);
            emit CreateDistribution(msg.sender, gauge_, amount_, distributionAmount);
        } else {
            tokenChache.safeTransfer(msg.sender, amount_);
        }
    }
}`

If amount is greater than zero, the function proceeds to set the epochStart and amount in the params struct for the gauge. However, modifying params directly does not persist since params is a memory variable; changes to it are not written back to gaugeParams[gauge]

Attack Scenario\ The logical consequence of this issue is that the contract's state, specifically the gaugeParams[gauge_], remains unchanged despite the function's apparent intention to update it. This means that any logic depending on the updated epochStart and amount in subsequent contract interactions will not behave as intended because the updates are not persisted.

In the provided code, DistributionParameters for a specific gauge are retrieved and stored in a memory variable named params. This is done with the line: `DistributionParameters memory params = gaugeParams[gauge]; After this, the code conditionally updates params.epochStart and params.amount based on the current block timestamp and the reward amount (amount_), respectively: params.epochStart = uint32(block.timestamp); params.amount = amount_;` However, because params is a memory variable, these updates are not reflected in the contract's storage. The modifications are effectively lost once the function execution completes, as memory variables only exist during the execution of a function call.

Attachments

  1. Proof of Concept (PoC) File Initial State: Let's saygaugeParams[gauge_] has epochStart = 0 and amount = 0 for a gauge identified by gauge_.

Calling notifyReward: The function is called with gauge and amount_ = 1000. The current block timestamp is 12345678.

Expected Behavior: The platform expects that calling this function will update the epochStart to 12345678 and amount to 1000 for gauge_, and then possibly create a distribution if the amount criteria are met.

Actual Outcome: While the function executes, params is updated in memory with epochStart = 12345678 and amount = 1000. However, since params is a memory variable, these changes are not written back to gaugeParams[gauge] in storage. After the function execution, gaugeParams[gauge] still has epochStart = 0 and amount = 0.

Long-term Impact: Any future logic that relies on these updated parameters (e.g., determining eligibility for rewards based on epochStart or calculating rewards based on amount) will operate based on outdated information, potentially leading to incorrect reward distributions or eligibility checks.

  1. Revised Code File (Optional) To fix this issue, after updating the params memory variable, the updated params should be explicitly written back to storage, like so: gaugeParams[gauge_] = params;
BohdanHrytsak commented 4 months ago

Thank you for the submission.

The specified gaugeParams settings are just a template for creating new distributions on MerkleDistributor, it is not expected to keep any count of amounts, time, etc. The purpose of gaugeParams is to provide settings for distributions for a particular Gauge, to specify the actual values of the amount and time that are relevant. And send the correct settings to merkl