The Graph Rounding Error Bugfix Review

On January 7 and January 24th, 2024, whitehat @GregadETH submitted High and Critical vulnerabilities in the Graph ecosystem, which consisted of 2 rounding errors that had the potential to result in the loss of user funds or unclaimed yield.
The Graph quickly patched the issue, and paid out a bounty total of $290,497 to the whitehat.
The Graph overview
The Graph considers itself a “decentralized indexing protocol”. What does this mean? It enables mainly developers to access and query data across different blockchains using its “subgraphs”, which are APIs. As with other decentralized protocols, it relies on the coordination of multiple parties, i.e. indexers, curators, and delegators, to provide its services, and thereby generate value.
Although blockchain data is by default, public, the process of collecting it is not as straightforward. Assuming you have the technical knowhow and the right setup to download and interpret the raw block data from a node, you’d still have to hunt the needle in the haystack of transactions, and do it efficiently.
By using The Graph, users can more easily access data from the blockchain via its GraphQL API, paying only for the specific information they need with GRT, the protocol’s native token. This token is also used by indexers (node operators of the network) to stake and index data based on specific curated subgraphs (APIs).
The part of The Graph that we will be discussing in this vulnerability analysis mainly concerns this staking functionality.
Vulnerability Analysis
The root cause of the vulnerability are two rounding errors, occurring in two separate assets and causing discrete impacts on each. Ensuring the precision or proper handling of floating-point arithmetic is crucial to avoiding financial discrepancies or exploitation of contract logic. Its versatility and frequency of finding makes it #6 of the Top 10 common smart contract vulnerabilities identified by Immunefi in 2023.
Locating the first issue
When a new subgraph API is created, the “curator”, or creator of the subgraph, stakes a certain amount of GRT tokens to do so, and pays a curation tax of 1% of the staked amount to the platform. This amount is variable, as a higher amount increases the likelihood that they will be indexed and earn fees. From this part onwards, the curator will be referred to as “user”.
The first rounding issue occurred in `Curation.sol` and `L2Curation.sol`, precisely in the `mint` function. The expected behavior of this function is that a user passes an amount of tokens to mint and then pays the `curationTax` amount of tokens to the called contract.

pragma solidity ^0.8.13;
//* @notice Deposit Graph Tokens in exchange for signal of a SubgraphDeployment curation pool.
* @param _subgraphDeploymentlD Subgraph deployment pool from where to mint signal
* @param _tokensln Amount of Graph Tokens to deposit
* @param _signalOutMin Expected minimum amount of signal to receive
* @return Amount of signal minted
* @return Amount of curation tax burned
*/
function mint(bytes32 _subgraphDeploymentlD, uint256 _tokensln, uint256 _signalOutMin) external override notPartialPaused returns (uint256, uint256) {
//Need to deposit some funds
require(_tokensln != 0, "Cannot deposit zero tokens");
//Exchange GRT tokens for GCS of the subgraph pool
(uint256 signalOut, uint256 curationTax) = tokensToSignal(_subgraphDeploymentID, _tokensln);
//Slippage protection
require(signalOut >= _signalOutMin, "Slippage protection");
address curator = msg.sender; CurationPool storage curationPool = pools[_subgraphDeploymentID];
Here, the `curationTax` calculation implemented in the body of the `tokensToSignal` is working incorrectly when the amount of minted tokens is below 100.
Let’s take a closer look at the rounding implementation in the calculation in the `tokensToSignal` function presented below.

pragma solidity ^0.8.13;
/**
* @notice Calculate amount of signal that can be bought with tokens in a curation pool.
* This function considers and excludes the deposit tax.
* @param _subgraphDeploymentlD Subgraph deployment to mint signal
* @param _tokensln Amount of tokens used to mint signal
* @return Amount of signal that can be bought
* @return Amount of tokens that will be burned as curation tax */
function tokensToSignal(bytes32 _subgraphDeploymentlD, uint256 _tokensIn) public view override returns (uint256, uint256)
{
uint256 curationTax = tokensIn.mul(uint256(curationTaxPercentabe)).div(MAX PPM);
uint256 signalOut = _tokensToSignal(_subgraphDeploymentID, _tokensIn.sub(curationTax)); return (signalOut, curationTax);
}
On the moment of the submission of the bug report the curation tax percentage was capped at 1% (variable equal to `uint256(10000)`), and the constant `MAX_PPM` was capped at 100% (variable equal to `uint256(1000000)`). That means that if an attacker is passing `uint256(99)` as a `tokensIn` variable, he will need to pay 990000/1000000 of the `curationTax`, which is rounded down to 0.
Normally, high gas costs on mainnet would prevent this from becoming a lucrative attack. But because this target contract is also deployed on Arbitrum, an L2 with much cheaper gas, the attacker could successfully avoid paying the `curationTax` completely by minting needed amount of tokens in batches of 99 tokens per call as presented in the PoC created by @GregadETH.
This could essentially rob the protocol of owed revenue and cause loss of user funds or yield owed to protocol participants.
pragma solidity ^0.8.13;
function split_mint() public {
uint256 start_gas = gasleft();
IERC20 grt = IERC20(addressGRT);
console.log("-- Splitting the mint into %d 99GRT pieces, single burn --", (mint_amount + 98) / 99);
console.log("Starting GRT balance: %d", grt.balanceOf(address(this)));
console.log("Minting signal from %d GRT...", mint_amount);
grt.approve(addressCuration, mint_amount);
uint256 signal = 0;
uint256 tax = 0;
uint256 it_signal = 0;
uint256 it_tax = 0;
uint256 remaining_tokens = mint_amount;
while (remaining_tokens > 99) {
(it_signal, it_tax) = curation.mint(bytes32(uint256(1)), 99, 0);
remaining_tokens -= 99;
signal += it_signal;
tax += it_tax;
}
(it_signal, it_tax) = curation.mint(bytes32(uint256(1)), remaining_tokens, 0);
signal += it_signal;
tax += it_tax;
console.log("Got %d signal, paid %d tax.", signal, tax);
console.log("Burning all %d GRS...", signal);
uint256 out_tokens = curation.burn(bytes32(uint256(1)), signal, 0);
console.log("Got %d tokens back. Return of %d PPM (%d%%).", out_tokens, 1000000 * out_tokens / mint_amount, 100 * out_tokens / mint_amount);
console.log("Gas used: %d, final GRT balance: %d\n", start_gas - gasleft(), grt.balanceOf(address(this)));
}
Deploying the fix
In order to fix this vulnerability The Graph dev team presented a new way of calculating the tax, where code calculates tokens after tax first, subtracting that from the tokensIn in order to get the curation tax to avoid rounding down to zero.
https://github.com/graphprotocol/contracts/commit/6a87193f809d403a835ab188bdd90beeb77d6c2b
pragma solidity ^0.8.13;
function tokensToSignal(bytes32 _subgraphDeploymentID, uint256 _tokensIn)
public
view
override
returns (uint256, uint256)
{
uint256 curationTax = _tokensIn.mul(uint256(curationTaxPercentage)).div(MAX_PPM);
uint256 signalOut = _tokensToSignal(_subgraphDeploymentID, _tokensIn.sub(curationTax));
// Calculate tokens after tax first, subtract that from the tokens in
// to get the curation tax to avoid rounding down to zero.
uint256 tokensAfterCurationTax = uint256(MAX_PPM)
.sub(curationTaxPercentage)
.mul(_tokensIn)
.div(MAX_PPM);
uint256 curationTax = _tokensIn.sub(tokensAfterCurationTax);
uint256 signalOut = _tokensToSignal(_subgraphDeploymentID, tokensAfterCurationTax);
return (signalOut, curationTax);
}
Locating the second issue
The second rounding issue @GregadETH uncovered was in another asset, `L2Staking.sol`.
When an indexer (node operator or hardware provider to the network) wants to provide services to the network in order to earn rewards and fees, they need to stake GRT tokens, which will stay locked for a period of time.
The Graph documentation states: “GRT that is staked in the protocol is subject to a thawing period and can be slashed if Indexers are malicious and serve incorrect data to applications or if they index incorrectly.”
However, this second rounding error results in a calculation error, which allows a malicious user to bypass the proper lock duration, unstaking tokens earlier than they should.
Let’s take a look.
In the `unstake` function, a call to the vulnerable `lockTokens` function was made. This function was responsible for adding unstaked tokens to a thawing (locked) pool and calculating the duration of the lock.

pragma solidity ^0.8.13;
/**
* @notice Unstake tokens from the indexer stake, lock them until the thawing period expires.
* @dev NOTE: The function accepts an amount greater than the currently staked tokens.
* If that happens, it will try to unstake the max amount of tokens it can.
* The reason for this behaviour is to avoid time conditions while the transaction
* is in flight.
* @param _tokens Amount of tokens to unstake
*/
function unstake(uint256 _tokens) external override notPartialPaused {
address indexer = msg.sender;
Stakes.Indexer storage indexerStake = __stakes[indexer];
require(indexerStake.tokensStaked > 0, "!stake");
// Tokens to lock is capped to the available tokens
uint256 tokensToLock = MathUtils.min(indexerStake.tokensAvailable(), _tokens);
require(tokensToLock > 0, "!stake-avail");
// Ensure minimum stake
uint256 newStake = indexerStake.tokensSecureStake().sub(tokensToLock);
require(newStake == 0 || newStake >= __minimumIndexerStake, "!minimumIndexerStake");
// Before locking more tokens, withdraw any unlocked ones if possible
uint256 tokensToWithdraw = indexerStake.tokensWithdrawable();
if (tokensToWithdraw > 0) {
_withdraw(indexer);
}
// Update the indexer stake locking tokens
indexerStake.lockTokens(tokensToLock, __thawingPeriod);
emit StakeLocked(indexer, indexerStake.tokensLocked, indexerStake.tokensLockedUntil);
}
The `lockTokens` function adds the unstaked tokens to an existing pool containing previously unstaked and locked tokens. This then calculates the new duration of lock to a value between the current lock duration and the maximum locked duration, which was capped at 28 days at the time of submission.

pragma solidity ^0.8.13;
/**
* @dev Lock tokens until a thawing period pass.
* @param stake Stake data
* @param _tokens Amount of tokens to unstake
* @param _period Period in blocks that need to pass before withdrawal
*/
function lockTokens(
Stakes.Indexer storage stake,
uint256 _tokens,
uint256 _period
) internal {
// Take into account period averaging for multiple unstake requests
uint256 lockingPeriod = _period;
if (stake.tokensLocked > 0) {
lockingPeriod = MathUtils.weightedAverage(
MathUtils.diffOrZero(stake.tokensLockedUntil, block.number), // Remaining thawing period
stake.tokensLocked, // Weighted by remaining unstaked tokens
_period, // Thawing period
_tokens // Weighted by new tokens to unstake
);
}
// Update balances
stake.tokensLocked = stake.tokensLocked.add(_tokens);
stake.tokensLockedUntil = block.number.add(lockingPeriod);
}
If there are some tokens already unstaked and sitting in the locked pool, the calculation of a new lock time will be done like this:
newLockedUntil = ((currentLockedUntil * currentlyUnstakedTokens) + (201600 * newUnstakedTokens)) / (currentUnstakedTokens + newUnstakedTokens)
If `currentUnstakedTokens` is roughly 201600 times larger than `newUnstakedTokens`, `newLockedUntil` will evaluate to `currentLockedUntil`. Thus, the attacker can pass a small enough value to the `unstake` function to withdraw small portions of the staked tokens without any additional lock applied. After that, he could proceed with the same attack plan as in the first bug described in the article: unstake small batches of tokens to avoid adding any value to the lock period.
The process of unstaking — calling unstake() and then waiting until the tokens will be unlocked — is now broken. The described attack can be used by a malicious user to avoid the lock period for unstaked tokens, enabling an attacker to immediately withdraw them without any penalty.
The impact of this varies, from an individual indexer making unfair gains and being able to unstake tokens faster than other indexers, to the possibility of multiple indexers being able to remove their stake entirely without penalty, adversely affecting the security of the network overnight.
Deploying the Fix
To fix the problem in the calculation process the separate library contract, `MathUtils.sol` was changed: where the vulnerable `weightedAverage` was changed to `weightedAverageRoundingUp`to ensure that the result is always greater than the lesser of the two input values.
This commit solves the problem of allowing users to unstake before they should.

Old Function
pragma solidity ^0.8.13;
/**
* @dev Calculates the weighted average of two values pondering each of these
* values based on configured weights. The contribution of each value N is
* weightN/(weightA + weightB).
* @param valueA The amount for value A
* @param weightA The weight to use for value A
* @param valueB The amount for value B
* @param weightB The weight to use for value B
*/
function weightedAverage(
uint256 valueA,
uint256 weightA,
uint256 valueB,
uint256 weightB
) internal pure returns (uint256) {
return valueA.mul(weightA).add(valueB.mul(weightB)).div(weightA.add(weightB));
}
New Function:
pragma solidity ^0.8.13;
/**
* @dev Calculates the weighted average of two values pondering each of these
* values based on configured weights. The contribution of each value N is
* weightN/(weightA + weightB). The calculation rounds up to ensure the result
* is always greater than the smallest of the two values.
* @param valueA The amount for value A
* @param weightA The weight to use for value A
* @param valueB The amount for value B
* @param weightB The weight to use for value B
*/
function weightedAverageRoundingUp(
uint256 valueA,
uint256 weightA,
uint256 valueB,
uint256 weightB
) internal pure returns (uint256) {
return
valueA.mul(weightA).add(valueB.mul(weightB)).add(weightA.add(weightB).sub(1)).div(
weightA.add(weightB)
);
}
Closing Acknowledgements
We would like to thank the whitehat @GregadETH for doing an amazing job in responsibly disclosing such an important bug. Big props also to the team at The Graph who quickly responded to the report and patched the issue.
If you’re a developer or a whitehat considering a lucrative bug-hunting career in web3 — this message is for you. With 10–100x the rewards commonly found in web2, your efforts will pay off exponentially by switching to web3.
Check out the Web3 Security Library, and start earning rewards on Immunefi — the leading bug bounty platform for web3 with the world’s biggest payouts.
To read more bugfix reviews, visit the Immunefi Blog. Or check out the Immunefi Top 10 Bugs page for more information on common vulnerabilities.
9