Lines of code

https://github.com/code-423n4/2023-05-maia/blob/cfed0dfa3bebdac0993b1b42239b4944eb0b196c/src/erc-20/ERC20Gauges.sol#L202-L220

Vulnerability details

Impact

The impact of the reentrancy vulnerability in the _incrementGaugeWeight function can be summarized as follows:

Loss of Funds: Attackers can drain funds from the contract or manipulate balances.
Unexpected State Changes: Manipulation of variables can lead to incorrect calculations or unauthorized access to resources.
Denial of Service (DoS): Legitimate users may be blocked from performing operations, causing disruption.
Exploitation of Other Vulnerabilities: Attackers can leverage reentrancy to exploit additional weaknesses or escalate privileges.

Proof of Concept

The part of the code that may be vulnerable to reentrancy attacks is the _incrementGaugeWeight function: LINK

function _incrementGaugeWeight(address user, address gauge, uint112 weight, uint32 cycle) internal {
        if (!_gauges.contains(gauge) || _deprecatedGauges.contains(gauge)) revert InvalidGaugeError();
        unchecked {
            if (cycle - block.timestamp <= incrementFreezeWindow) revert IncrementFreezeError();
        }

        IBaseV2Gauge(gauge).accrueBribes(user);

        bool added = _userGauges[user].add(gauge); // idempotent add
        if (added && _userGauges[user].length() > maxGauges && !canContractExceedMaxGauges[user]) {
            revert MaxGaugeError();
        }

        getUserGaugeWeight[user][gauge] += weight;

        _writeGaugeWeight(_getGaugeWeight[gauge], _add112, weight, cycle);

        emit IncrementGaugeWeight(user, gauge, weight, cycle);
    }

The potential reentrancy vulnerability arises from the accrueBribes function call to the IBaseV2Gauge contract. If the accrueBribes function performs an external call to an untrusted contract before the state changes are completed in the _incrementGaugeWeight function, it can allow the called contract to reenter the _incrementGaugeWeight function and potentially manipulate the contract state in unexpected ways.

Tools Used

Manual Review

Recommended Mitigation Steps

To fix the reentrancy vulnerability in the _incrementGaugeWeight function, we can apply the following measures:

Use the "checks-effects-interactions" pattern: Ensure that all state changes are completed before making any external calls. This prevents reentrant calls from manipulating the contract's state in unexpected ways.
Restrict external calls: Move the accrueBribes function call to the beginning of the _incrementGaugeWeight function, before any state changes are made. This ensures that the external call is made before any sensitive operations occur.
Implement a mutex or reentrancy guard: Add a mutex or reentrancy guard mechanism to prevent reentrant calls to the _incrementGaugeWeight function. This can be done by using a boolean flag that is set to true at the beginning of the function and reset to false at the end. Reentrant calls can be detected and rejected by checking the flag.

Here's an example of how the fix could be implemented:

bool private _isIncrementingGaugeWeight;

function _incrementGaugeWeight(address user, address gauge, uint112 weight, uint32 cycle) internal {
    require(!_isIncrementingGaugeWeight, "Function locked during execution");
    _isIncrementingGaugeWeight = true;

    if (!_gauges.contains(gauge) || _deprecatedGauges.contains(gauge)) revert InvalidGaugeError();
    unchecked {
        if (cycle - block.timestamp <= incrementFreezeWindow) revert IncrementFreezeError();
    }

    // Move external call to the beginning
    IBaseV2Gauge(gauge).accrueBribes(user);

    bool added = _userGauges[user].add(gauge);
    if (added && _userGauges[user].length() > maxGauges && !canContractExceedMaxGauges[user]) {
        revert MaxGaugeError();
    }

    getUserGaugeWeight[user][gauge] += weight;
    _writeGaugeWeight(_getGaugeWeight[gauge], _add112, weight, cycle);

    _isIncrementingGaugeWeight = false; // Reset the flag
    emit IncrementGaugeWeight(user, gauge, weight, cycle);
}

By applying these fixes, we can mitigate the reentrancy vulnerability and ensure that the contract functions as intended, without being susceptible to reentrant attacks.

Assessed type

Reentrancy

code-423n4 / 2023-05-maia-findings

Reentrancy Vulnerability: The contract inherits from the ReentrancyGuard contract, which smay be vulnerable to reentrancy attacks if not properly handled in the contract's logic. #825