Security Review — SeriesCode.sol (ArtBlocks ABX)

Repo: https://github.com/ArtBlocks/abx
Commit: 1e3061e945a9c32b4d14af51a49448ee6f9b4837 (default-branch HEAD at audit time; verified reachable with git ls-remote)
Scope: contracts/src/tokens/SeriesCode.sol (329 lines). Directly composed mixins were read for context: AbxErc721Base, MaxInvocations, ExternalMinter, Paused, SeedSourceExtension, CreatorToken, ConfigurableParams, AbxParamsLib (seed, hooks), SeriesCodeFactory, AbxSeedSource.
Threat model (per client): the creator/owner is trusted by design. Findings target unintentional bugs, footguns, over-broad trust, and trust in third parties.

Severity counts: 0 Critical · 0 High · 1 Medium · 4 Low · 4 Informational

Method note: This was run as a single-contract engagement. Instead of the full multi-agent fan-out, the audit was a manual, adversarial line-by-line review of the one contract plus its inherited mixins, driven by the protocol map below. No code was executed, and forge coverage was not run (policy). Dependency files outside the scope were read only to resolve how SeriesCode wires them.

Summary

SeriesCode is a thin composition layer over well-factored mixins. The state-machine ordering in _mintNext is correct: the cursor is advanced before the external seed call, and the cap floor reads the cursor rather than totalSupply. The initializer is locked on the implementation, and creator-token enrollment is placed last in initialize. I found no way for an unprivileged attacker to steal funds, over-mint past the cap, re-initialize a clone, or rewrite a settled seed. The findings are about the trust the owner holds over collectors, and about the weak randomness of the reference seed source.

Access-control map (SeriesCode surface)

Findings

[M-1] Owner-set transfer hook and validator can freeze every holder's transfers, and the hook can also brick minting

Severity: Medium
Confidence: [85]
Location: SeriesCode.sol:320-326 (_afterTokenTransfer → _notifyTransferHook), SeriesCode.sol:310-316 (_beforeTokenTransfer → _validateTransfer); ConfigurableParams.sol:225; CreatorToken.sol:68, CreatorToken.sol:212; AbxParamsLib.sol:478, AbxParamsLib.sol:502.

function _afterTokenTransfer(address from, address to, uint256 id) internal override(...) {
    super._afterTokenTransfer(from, to, id);
    _notifyTransferHook(id, from, to);          // external call, reverts bubble
}

_notifyTransferHook makes an unconditional external call to an owner-chosen address on every mint, transfer and burn. A revert vetoes the movement.

Exploit path (trusted-owner abuse or compromise):

1. Owner calls setParamHooks(0, 0, H) where H.onTokenTransfer reverts, selectively (e.g. on a marketplace operator) or always.
2. Every transfer of every token reverts. Because the hook also runs on from == 0, all mints revert too, including the owner's.
3. If the owner called lockParamHooks() first, or later renounces ownership, this is permanent. The code documents that lockParamHooks freezes addresses only, so a locked proxy hook can be upgraded to revert later.
4. Independently, setTransferValidator(V) with any V that passes the probe in _requireHasCode makes validateTransfer revert, which freezes transfers. The owner can do this at any time, and the validator can never be locked or removed.

The code is explicit that this is an accepted trust assumption. It is still the largest trust opening for buyers: a collector's token has no guaranteed transferability, and a compromised owner key can freeze or hold the whole collection hostage (for example, to force a ransom or to disable secondary sales except via an owner-approved path).
Recommendation: (a) Make hook failures non-fatal for mints and burns, or wrap the hook call in try/catch with a gas cap, so a bad hook cannot block minting. (b) Offer a permanent "no hook / no validator changes" lock for projects that want a hard promise (a one-way lockTransferValidator). (c) Surface paramHooks() and getTransferValidator() prominently to buyers. (d) Consider a timelock on re-pointing either.

[L-1] setSeedSource can be changed at any time and cannot be locked, so a "random" collection can be switched to a curated or grindable source mid-drop

Severity: Low
Confidence: [90]
Location: SeedSourceExtension.sol:34-37, SeedSourceExtension.sol:52-62; SeriesCode.sol:238-241

function setSeedSource(address source) external onlyOwner {
    SeedSourceStorage.layout().seedSource = source;

Only the settled seeds are frozen. The source that will assign seeds to all future mints can be swapped between sales. The owner (or a compromised key) can point it at a source returning chosen values for favoured recipients, or clear it so tokens mint with no seed and are later schema-assigned. The seed governance schema is frozen by latchSeedAssignment, but the source is not.
Related: IAbxSeedSource.seed() is called with the new token id reserved but not yet minted (SeriesCode.sol:236-248), so a source that calls back into view functions such as ownerOf(tokenId) reverts, and a source that reverts blocks every non-owner mint until the owner swaps it (an owner-recoverable DoS only while the owner exists).
Recommendation: Add lockSeedSource() (one-way) consistent with lockParamHooks, and document that buyers should check it.

[L-2] Reference AbxSeedSource randomness can be ground by a contract minter (rarity sniping)

Severity: Low (documented, but it is the default path)
Confidence: [90]
Location: seed/AbxSeedSource.sol (seed), invoked from SeriesCode.sol:239-240

keccak256(abi.encodePacked(msg.sender, tokenId, block.prevrandao, blockhash(block.number - 1), block.timestamp))

All inputs are known inside the minting transaction, and AbxFixedPriceMinter.purchase* (line 190: IAbxSequentialMint(token).mint(to)) is callable by contracts. A buyer wrapper calls purchaseTo(wrapper), reads the token's seed param, renders/evaluates it, and reverts unless it is rare. The only cost is gas. The source's own NatSpec discloses this; it is flagged because SeriesCode's seed promise ("the token draws one") reads as fairer than it is for any project where rare outcomes carry price premiums.
Recommendation: Keep the warning prominent in the factory and SDK defaults, or offer a commit/reveal source (seed from a later block) as the recommended option for priced scarcity.

[L-3] _mint instead of _safeMint can strand tokens in non-receiver contracts

Severity: Low
Confidence: [95]
Location: SeriesCode.sol:248

purchaseTo, mint(to) and mintMany(to, ...) deliver to any to, including contracts that cannot transfer ERC-721s. With burning off by default there is no recovery. The decision is documented and deliberate; the footgun is real for buyers using purchaseTo with a wrong or contract address.
Recommendation: Keep, but have the minter UI/SDK warn when to is a contract without onERC721Received, or offer an opt-in safe variant outside the sale path.

[L-4] The single minter can mint the entire remaining supply; mintMany has no per-call or per-minter bound

Severity: Low
Confidence: [85]
Location: SeriesCode.sol:205-216, SeriesCode.sol:253-257

function mintMany(address to, uint256 count) external { _requireMintAuth(); for (...) _mintNext(to); }

mint/mintMany give the authorized minter the same capability as the owner (minus pause bypass): it can mint all remaining tokens to any address in one call, for free, bypassing whatever pricing the minter contract was meant to enforce. Any bug or upgradeability in the minter contract therefore translates directly into a supply drain, and the cap is the only bound. mintMany is also an unbounded loop that can run out of gas (self-limiting).
Recommendation: Expose only mint(to) to the minter and keep mintMany owner-only, or give the minter a per-call/total allowance. Document that the minter address is fully trusted with the whole remaining supply.

[I-1] Owner mint bypasses paused, and renouncing while paused permanently ends minting

Severity: Info
Confidence: [90]
Location: SeriesCode.sol:253-257; Paused.sol:34
owner() skips the pause check by design. If the owner renounces (or loses the key) while paused == true, nobody can ever unpause and the remaining supply is unmintable. Consider refusing renounceOwnership while paused, or documenting it.

[I-2] Deterministic deployment with an unguarded salt allows address front-running

Severity: Info
Confidence: [75]
Location: SeriesCodeFactory.sol:48-56, SeriesCodeFactory.sol:78-82
The clone address depends only on the implementation, salt and factory, not on params. With a salt whose leading 20 bytes are zero (_guardSalt accepts it), anyone seeing the intended address can deploy first at that address with different init params (owner, validator, seed source). Harmless when the address is read after deployment, but dangerous for integrations that pre-commit to a predicted address. Prefer salts prefixed with the deployer address, and mix keccak(params) into the salt in the SDK.

[I-3] initialize is callable by anyone on a clone that was not initialized in the same transaction

Severity: Info
Confidence: [90]
Location: SeriesCode.sol:150
The factory initializes atomically (SeriesCodeFactory.sol:68-73), so this is not exploitable through the supported path. A hand-rolled LibClone.clone without an initializing call would be takeable by a front-runner. The implementation itself is protected (_disableInitializers, SeriesCode.sol:127-130).

[I-4] Seed from the source is stored without validation; a source returning bytes32(0) marks the seed "set"

Severity: Info
Confidence: [60]
Location: SeriesCode.sol:238-241
_setTokenParam(tokenId, "seed", value, false, source) persists whatever the source returns, including zero, and isSet becomes true so it can never be redrawn. Zero is a plausible "uninitialized" return from a buggy custom source. A non-zero check (or a documented sentinel) would avoid permanently settling a degenerate seed.

Verified non-issues (checked, no vulnerability)

• Cap vs. reentrancy: _mintNext writes nextTokenId before the seed call (SeriesCode.sol:235-237); a reentrant mint sees the advanced cursor, so nextTokenId <= maxInvocations holds. _maxInvocationsFloor reads the cursor, so lowering the cap mid-frame cannot be bypassed.
• Settled seed: AbxParamsLib.setTokenParam (line ~270) refuses a second seed write unless a seed schema exists; latchSeedAssignment closes the schema-declaration reentrancy window before the external call.
• Creator-token enrolment: _initCreatorToken runs last in initialize, after all caps are written, so the validator callback cannot see a half-initialized clone.
• Multicall: every subcall delegatecalls with the original msg.sender and re-runs its own gate; msg.value is rejected by Solady; no ERC-2771.
• Validator probe: a documented misconfiguration guard, not a security boundary.