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.

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.

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.

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

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.

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.

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

New Function:

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.