6 Smart Contract Audit Findings Explained

Reading smart contract audit findings is one of the best ways to learn new bug classes. The real value is the auditor's mindset: the heuristic that led them to the bug.

Reports rarely give you that.

A finding is written to communicate the bug and its severity, not how it was found, and it often assumes context on the system you do not have.

This article covers six findings from past Guardian reports.

For each, you get the context you need, the bug, and the heuristic you can reuse in your own audits, contests and protocol design.

1. Griefing Name Sellers

NMKT-3, "Griefing name sellers," comes from a marketplace for names. For our purposes, a name is an NFT that its owner can list for sale.

The three functions

• enterBidForName: there is only ever one bid per name. A new bid overwrites the existing one if it sends a higher msg.value, and reverts otherwise.
• acceptBidForName: the owner accepts the current bid.
• withdrawBidForName: the current highest bidder cancels their bid and gets the funds back. Bidders who were overwritten claim theirs from a pendingWithdrawals mapping.

function enterBidForName(uint256 nameId) external payable {
    Bid memory existing = bids[nameId];
    require(msg.value > existing.value, "bid too low");
    pendingWithdrawals[existing.bidder] += existing.value;
    bids[nameId] = Bid(msg.sender, msg.value); // overwrites the old bid
}

function withdrawBidForName(uint256 nameId) external {
    Bid memory bid = bids[nameId];
    require(bid.bidder == msg.sender, "not the bidder");
    delete bids[nameId]; // the old bid is not reinstated
    payable(msg.sender).transfer(bid.value);
}

The attack

Say I own the name "Bob," and someone bids 10 ether. I am happy to sell at that price.

An attacker enters a bid of 11 ether, overwriting the 10 ether bid, then withdraws it at once. With an attack contract, both steps fit in one transaction.

The 10 ether bid is not reinstated. The buyer could not buy at 10 ether, and I have no bid left to accept.

An MEV-style bot could do this to every bid automatically, back-running each one.

In the report's words, any bids made with true buying intention are lost and the seller cannot sell the name.

The fix

One option is to reinstate the previous bid, though that gets messy. If only the last bid is tracked, there are ways to grief that too.

The other is to require a few blocks before a bid can be withdrawn.

That fully removes the single-transaction vector, and the attacker's bid now carries real risk that the owner accepts it.

The heuristic

Two things combine here: overwriting a piece of state, and front-running or back-running.

Developers often do not consider their protocol being used this way, so it is a vector that usually turns something up.

For a function like this, assume you can land a transaction immediately after it.

For a proof of concept, you can treat that as guaranteed. What can you gain, and how does it hurt others?

On a second look, there is a gain. Erase everyone else's bids, leave only yours, and you can bid lower and buy the name cheaply.

Always look for ways to combine state overrides with front-running and back-running. For more on the attack side, read front-running smart contracts.

2. Blacklisted Addresses

GLOBAL-2 comes from Guardian's third audit of GMX V2 (GMX Synthetics). It shows how a blacklisted address can make a position impossible to liquidate.

For background on the exchange, see GMX V2 explained.

How the transfer fails

Token transfers go through a TokenUtils library. After some validation it calls a helper, nonRevertingTransferWithGasLimit, which makes a low-level call with the transfer selector and a fixed amount of gas.

An ERC-20 transfer should return a boolean. Some tokens revert on failure, and some just return false.

Now send to a blacklisted address. If the token reverts, the call fails and the helper returns false.

If the token returns false, the call succeeds, but the return data decodes to false, so the helper returns false again.

Either way, the transfer function sees a failure, parses a revert message and reverts with a token transfer error.

Why that matters: liquidations

GMX V2 lets you open perpetual positions, say a 10 ether long. If losses grow so large that your collateral cannot sustain the position, keepers must liquidate it.

A liquidation is a special kind of decrease order, and it uses the same code path. In DecreaseOrderUtils, the output tokens are transferred out of the market token to order.receiver.

If that receiver is blacklisted for the output token, the transfer reverts and so does the order. A liquidation that cannot execute means an unliquidatable position.

That can cause tremendous bad debt for the protocol. A trader could open a position with barely any collateral and never be liquidated.

Building the exploit

In LiquidationUtils, the receiver of a liquidation order is the account that owns the position. So the attacker opens a position for an account that is blacklisted for, say, USDC.

The account does not need to transfer the USDC itself.

A separate address funds the order vault in the same transaction, using an attack contract, and the funds are credited to the blacklisted account as collateral.

As long as the keeper tries to send leftover collateral back to the receiver, the position cannot be liquidated.

The attacker can still get their money out, because order.receiver is configurable. They can:

• Set a different receiver, such as a fresh hot wallet that is not blacklisted.
• Add a swap path, so the output is swapped inside the system to a token they are not blacklisted for.

The result is a position that cannot be liquidated in the vast majority of cases, with all the upside of a normal position.

The heuristic

Never assume a token transfer will go through. An attacker can decide whether a transfer succeeds by using blacklisted addresses.

You may not even need to own one. Referencing a blacklisted address as a parameter can be enough to force a revert.

At GMX, this led to unliquidatable positions and other issues across the exchange, such as potential risk-free trades. For more token behaviors like this, see the weird ERC-20 tokens checklist.

3. Mint Failure

UF-4, "Mint failure," comes from an NFT project that rewards holders of other NFT projects. The bug is in its publicMint function.

The counter

Each pass through the mint loop calls safeMint with the current tokenIdCounter, then increments the counter. Only after safeMint does that ID exist as an NFT.

So between transactions, the counter always points to the next ID to mint: an ID with no NFT yet.

The bug

The function picks a random token ID as random % (tokenIdCounter.current() + 1).

Taking a number modulo the counter gives only IDs that exist. Adding one includes the counter itself, which has no NFT.

With a small counter, there is a decent chance of drawing that ID.

The next call to ownerOf fails, because ERC-721's ownerOf reverts for an unminted token, and the whole mint reverts.

// tokenIdCounter points at the next ID to mint, which does not exist yet
uint256 id = random % (tokenIdCounter.current() + 1); // can equal current()
address holder = ownerOf(id); // reverts when id is unminted

// fix: uint256 id = random % tokenIdCounter.current();

The recommendation is to take the modulo of tokenIdCounter.current() without the plus one. Then it is impossible to get an ID for an NFT that does not exist.

The heuristic

First, the direct lesson: calling ownerOf for an ID that does not exist yet reverts the transaction.

More broadly, this is the classic off-by-one error. It is common in complex mathematical formulas, and in loops that run one extra, unintended time.

Off-by-one errors are probably among the most common bugs in smart contracts. Whenever you see a formula, and especially a plus one, write it out on paper or a whiteboard.

Is the one necessary? And if there is no plus one, does the formula need one?

4. Shorten Lock

GG-3, "Shorten lock," comes from Guardian's audit of Bridges Exchange. Users hold locked positions of tokens, each with a stakeUntil time.

In transferLock, the receiver's stakeUntil is set to the sender's.

So a lock can be shortened by transferring from an address with a short lock to an address with a long one.

UserInfo storage to = userInfo[recipient];
// can move to.stakeUntil earlier than it already is
to.stakeUntil = from.stakeUntil;

The receiving address might be locked for two or three years.

The sending address might be locked for a day or two, and now the long lock opens in days, or even sooner.

The impact is serious: it can invalidate the entire locking logic.

The heuristic

Whenever a state variable is assigned a new value, think through every edge case for that piece of state.

This bug looks obvious once pointed out, but others like it are not.

Ask of every assignment like to.stakeUntil:

• Can I set it to zero?
• Can I set it to a very large number?
• Can I set it to something less than its current value?

5. Incorrect Truncation Avoidance

RLQ-4, "Incorrect truncation avoidance," comes from Guardian's audit of Reliquary by Byte Masons. The code is a pure internal helper that computes weights for a reward distribution.

The branches

The helper splits into an if branch and an else if branch. Ignoring precision loss, both compute the same thing.

The split exists to avoid precision loss when the fraction rounds down. But it does the opposite of what it intends.

The if branch runs when the added value is the smaller one, and it puts that smaller value in the numerator.

That exposes the smaller value to truncation. The else if branch does the same with the old value.

// simplified
uint256 total = oldValue + addedValue;
if (addedValue < oldValue) {
    weightNew = addedValue * 1e18 / total; // smaller value on top
    weightOld = 1e18 - weightNew;
} else if (oldValue < addedValue) {
    weightOld = oldValue * 1e18 / total; // smaller value on top
    weightNew = 1e18 - weightOld;
}

Make it obvious with extreme values

To understand logic like this without spending much mental effort, use extreme values. Let the added value be 1 wei and the old value 1 ether.

The if branch runs. The numerator is 1 wei times 1e18, which is 1e18, over a denominator of 1 ether plus 1 wei, so weightNew rounds down to zero.

Put the larger value on top instead. The old value times 1e18, over the same denominator, gives a weightOld just under 1e18 with no rounding to zero.

Subtracting it from 1e18 gives a more accurate weightNew of 1.

The report's description uses the same pair of values, the other way round, and the recommendation is to swap the logic of the two branches.

The larger value should always be in the numerator.

The heuristic

• Do not shy away from high-effort code. Your eyes may gloss over a function like this. Skip it on the first pass, but on later passes plug in values and work through every case and every truncation.
• Check every division. Look for divide-by-zero cases (prevented here), precision loss and rounding down to zero.
• Use a very small and a very large value. Inputs that maximize the gap make a rounding issue black and white. It is not the most common case, but it is the clearest, and the most useful for a proof of concept.

For more on this class, read rounding errors in Solidity.

6. Uncaught Reverts

This GLOBAL-2 comes from Guardian's first audit of GMX V2, on a very early iteration of the code.

Its bug let users delay limit orders until they succeeded, among many other things.

The try/catch

The order handler wraps its order execution in a try/catch.

On a revert, it reads the reason to decide whether to revert, so the order can be tried again, or cancel the order for good.

The crux is the catch clause: it only catches a revert with a reason string.

try this.executeOrderInternal(key) {
} catch Error(string memory reason) {
    // only revert("...") and require(..., "...") land here
    handleOrderError(key, reason);
}
// custom errors and panics are not caught: the whole call reverts

Not every revert has a reason string. Custom errors do not, and neither do panics such as divide by zero, underflow and overflow, which revert with bytes.

None of these reach the catch block, so the top-level transaction reverts. Orders that should have been canceled are retried instead.

The exploit

A malicious user can force a panic or custom error, keeping the order alive.

When price moves in their favor, they twist whichever knob makes it stop reverting, and get a risk-free trade.

The same bug was in the deposit and withdrawal handlers. The report's main example is a limit increase order that exceeds the maximum reserved USD:

1. Ether is at $2,000. At block 100, user A creates a limit increase long for ether at $2,000, with a USD size larger than allowed.
2. The order reverts with an insufficient reserves custom error, but it stays in the order store.
3. Twenty blocks later, ether is at $2,100. User A deposits enough reserves that the order no longer reverts.
4. The order executes using the price from block 100, so user A buys ether at $2,000 while it trades at $2,100.

The heuristics

Write tests early and often. This bug was not found on a first pass.

It surfaced while writing initial edge case tests: one reverted with an underflow panic, and the order was not canceled.

Tests build your understanding of how each function executes, and they catch things like this early.

Once you have seen it, a try/catch on the reason-string selector will always raise alarm bells.

Turn every crux into its most critical form. Once you find the core issue, use every knob you understand in the protocol to make the impact as severe as it can be.

That clearly communicates the exploit's value to the protocol team, and that it must be fixed.

Guardian wrote several proofs of concept for this bug and chose the most critical one: hold a limit order with insufficient reserves, deposit, and execute when favorable.

For how to build one, see how to write a smart contract proof of concept.

Heuristics Checklist

• Where state is overwritten, what can a front-runner or back-runner gain, and who loses?
• Can any token transfer be made to revert, including with a blacklisted address passed as a parameter?
• Is every plus one in a formula or loop bound necessary, and is one missing anywhere?
• For each state assignment, can the value be zero, huge, or lower than before?
• At every division, try a very small and a very large value. Does the result round to zero?
• Does each try/catch handle custom errors and panics, not just reason strings?
• Have you written edge case tests early, and pushed each finding to its most critical impact?

How Guardian Helps

Guardian's smart contract audits pair senior researchers' exploit-driven review with an invariant suite built around the protocol's safety properties.

Findings come with proof-of-concept depth, and the suite stays with your team as a regression harness for every fix.

Frequently Asked Questions

Why read smart contract audit findings?

They show new bug classes and, more importantly, the auditor's mindset. Learning the heuristic behind a finding lets you find similar bugs in your own audits and avoid them in your own code.

How can a blacklisted address break a protocol?

A transfer to a blacklisted address fails. If a critical flow such as a liquidation must transfer to an address the user controls, the user can make it revert at will.

Why do off-by-one errors matter so much?

They are probably among the most common smart contract bugs. A stray plus one can select an ID that does not exist, loop one extra time, or skew a formula.

What does a Solidity catch Error(string) clause miss?

It only catches reverts with a reason string. Custom errors and panics, such as overflow, underflow or divide by zero, are not caught, so the outer transaction reverts.

Sources

• Owen Thurm (YouTube): Learn From My Findings | 3 Findings Breakdown (June 2023)
• Owen Thurm (YouTube): Build Your Auditing Toolkit | Findings Breakdown (May 2023)
• Guardian: Bridges Exchange audit report (GG-3)
• Guardian: Reliquary audit report (RLQ-4)