Security Review — PrimeOTC.sol

Client job: leftclaw.services job #1064 — "Security review of PrimeOTC.sol"
Source (pinned, reproducible): GitHub Gist, revision 95e44e0920230d6b0b98721c6cbc5f88a200d40b —
https://gist.githubusercontent.com/ASTRALDEFI/b66dddd65e60b652c6d6d1b0956d0fdc/raw/95e44e0920230d6b0b98721c6cbc5f88a200d40b/PrimeOTC.sol
(re-fetched and byte-diffed against the audited copy immediately before publishing this report — identical.)
Scope: PrimeOTC.sol only, Solidity ^0.8.26, 589 lines, 364 nSLOC, single file, no imports, no build config, no tests, not yet deployed. No commit history exists for this target beyond the gist's own revision above (it is a single-file gist, not a repository) — a local git commit was created solely to run this audit's tooling (4b378109c3a8a7ea9989440e5a11609e15de9acf, not a reproducible pin since it is a local-only artifact; the gist revision above is the actual pin a reader should fetch).
Methodology: x-ray mechanical pre-scan → Phase 0 context building (3 parallel agents) → Phase 1 breadth (6 ethskills checklist agents: general, precision-math, erc20, access-control, dos, assembly) → Phase 2 depth (12 pashov V4 attack agents, blind to Phase 1, one pass) → hybrid reconciliation with a three-axis coverage gate.

Severity counts: 1 High · 1 Low · 3 Informational

Findings

[H-1] Quote-currency payouts in buy()/sell() are not balance-checked — a fee-on-transfer or hook-bearing quote token silently shorts the counterparty and defeats the price-limit guard

Severity: High
Confirmed independently by: 11 of 18 total hunting agents across both phases — Phase 1's erc20 checklist agent (blind), and 5 of Phase 2's 12 pashov agents reaching full-FINDING status independently and blind to each other (economic-security, periphery, first-principles, asymmetry, flow-gap), plus 6 more agents reporting the same root cause as a LEAD. This is the strongest-signal result in the engagement.

Description. The contract's own NatSpec design note 7 states: "Every payout in a fill measures the receiver's balance the same way [as the pull], so a token that starts taking a fee after the offer was listed (some can be switched on by their owner) cannot short a taker: the fill reverts instead." This guarantee is implemented only for the offered token leg of a fill, via _pushChecked (PrimeOTC.sol:555-560), which measures the receiver's balance before and after and reverts on any shortfall (Shortfall). The quote-currency leg (ETH/WETH, USDC, or DGUY) of every fill is paid via the unchecked _push (PrimeOTC.sol:548-550):

• buy(), non-ETH quote branch, pays the maker: PrimeOTC.sol:370 — _push(q, maker, quoteAmount);
• sell() pays the taker via _payQuote → _push: PrimeOTC.sol:407 (_payQuote(quote, msg.sender, quoteAmount);), resolving at PrimeOTC.sol:571-574:

  function _payQuote(Quote quote, address to, uint256 amount) private {
      if (quote == Quote.ETH) _payEthFromWeth(to, amount);
      else _push(_quoteToken(quote), to, amount);
  }
  

• Contrast with the checked token-leg payouts in the same two functions: PrimeOTC.sol:379 (_pushChecked(token, msg.sender, tokenAmount); in buy) and PrimeOTC.sol:406 (_pushChecked(token, maker, tokenAmount); in sell).

WETH and USDC are canonical, well-known, fee-free contracts, so this gap is latent for two of the three quote currencies. DGUY is a third, deployer-named token ("The Delivery Guy") whose transfer behavior this contract never validates and whose source is out of this review's scope — the NatSpec's own KNOWN LIMITS section acknowledges USDC can freeze/blacklist but says nothing about DGUY ever taking a transfer fee, even though note 7 explicitly anticipates exactly that risk class for some token.

Compounding defect — the slippage guard is defeated, not just the balance check. buy()'s maxQuote check (PrimeOTC.sol:359) and sell()'s minQuote check (PrimeOTC.sol:398) both run against the computed quoteAmount, before _push/_payQuote is ever called. If the quote token takes a cut on transfer, the price-limit parameter the taker explicitly set to protect themselves never sees the real, post-fee amount — it cannot catch the shortfall, because the shortfall happens after the check.

Proof of concept. Maker lists a Buy offer: wants 100e18 of token X, escrows 1000e18 DGUY as the quote (quoteTotal = quoteLeft = 1000e18), partialOk = false. At listing time DGUY has no transfer fee, so _pull (checked) escrows exactly 1000e18. DGUY's owner later enables a 2% transfer tax. A taker calls sell(id, 100e18, minQuote = 990e18), expecting at least 990e18 DGUY back. _whole() returns true, quoteAmount = 1000e18; the check 1000e18 < 990e18 is false, so it passes. _record sets quoteLeft = 0, closes the offer as Filled, and the OfferFilled event reports quoteAmount = 1000e18 paid. _payQuote(Quote.DGUY, taker, 1000e18) → _push → DGUY.transfer(taker, 1000e18) — DGUY's fee logic delivers only 980e18 to the taker. The call does not revert (no Shortfall check exists on this path), so the transaction succeeds. The taker gave up 100e18 of token X in full (checked via _pushChecked to the maker) and received 20e18 DGUY less than the minQuote they explicitly required, with no on-chain trace of the shortfall beyond the token's own transfer event. The symmetric case in buy() shorts the maker instead (_push(q, maker, quoteAmount) at line 370 is equally unchecked).

Recommendation. Route every fill payout of a quote currency through the same balance-delta check _pushChecked already performs for the offered token:

function _payQuote(Quote quote, address to, uint256 amount) private {
    if (quote == Quote.ETH) { _payEthFromWeth(to, amount); return; }
    _pushChecked(_quoteToken(quote), to, amount);
}

and change buy()'s line 370 from _push(q, maker, quoteAmount) to _pushChecked(q, maker, quoteAmount). Leave cancel()'s refund path (PrimeOTC.sol:329) on the unchecked _push/_payQuote — that asymmetry is explicitly intentional (note 7: "A cancel does not check the receiver, so a maker can always take their tokens back out, whatever the token does") and must not be changed, or a maker could be permanently unable to cancel a Sell offer in a token that starts rejecting transfers.

[L-1] Unbounded low-level-call return-data copy in _call() enables a gas-griefing DoS via a maliciously crafted offered token

Severity: Low
Confirmed independently by: 3 of 6 Phase 1 agents (assembly, dos, general), each reaching the same root cause by a different route, with escalating exploit framing (Low → Medium → High in the raw agent reports). Not independently found by any Phase 2 agent. Reconciled here to Low — see severity rationale below.

Description. _call (PrimeOTC.sol:562-567) performs (bool ok, bytes memory ret) = token.call(data); with no bound on the size of ret. Solidity's .call() syntax copies the callee's entire return buffer into memory before the function can inspect ret.length, at quadratic memory-expansion cost. Because the offered token in every offer is a maker-chosen, unvetted address (checked only for non-zero and code.length != 0, PrimeOTC.sol:260-261), a token whose transfer/transferFrom is coded to return an oversized payload forces this cost onto every subsequent caller of _pull/_push/_pushChecked against that token — reachable from createOffer, buy, sell, and cancel.

Why Low, not High. The agents' escalating framings (permanent fund lock, zero-capital griefing via a Buy-offer listing) were checked against two constraints that cap real-world impact: (1) for a Sell offer, the only party whose funds get trapped if the listed token starts bombing returns is the maker who chose to list that token themselves — there is no incentive for a rational attacker to trap their own escrow, so this path is self-harm, not third-party harm; (2) for a Buy offer, the malicious token is only ever touched when a taker calls sell() and must already hold that specific, attacker-controlled token to sell it — a plausible but narrower precondition (the taker must be deceived into holding and trying to sell a fake/copycat token, a scenario the contract's own KNOWN LIMITS section already names as a general risk: "a token can copy another's name and ticker"). Independent of the fund-loss framing, the pure gas-griefing mechanism is real and verified: Solidity's default full-returndata copy, combined with EIP-150's 63/64 gas-forwarding rule, means a sufficiently large returned payload can make the caller's copy step run out of gas regardless of how much total gas the caller's transaction supplies — this is a genuine, if narrow-precondition, robustness gap.

Recommendation. Bound the copied return data in _call to the 32 bytes the function actually consumes, using a raw call + capped returndatacopy:

function _call(address token, bytes memory data) private {
    (bool ok,) = token.call(data);
    uint256 size;
    assembly { size := returndatasize() }
    if (size > 32) size = 32;
    bytes memory ret = new bytes(size);
    if (size > 0) assembly { returndatacopy(add(ret, 0x20), 0, size) }
    if (!ok || token.code.length == 0 || (ret.length != 0 && (ret.length < 32 || !abi.decode(ret, (bool))))) {
        revert TransferFailed();
    }
}

[I-1] Maker may fill their own offer (self-trade / wash fill)

Severity: Info
Confirmed independently by: Phase 1 access-control and general agents, and Phase 2 first-principles agent (as a non-filed lead).

_fillable() (PrimeOTC.sol:485-492) does not check msg.sender != o.maker on a public offer (taker == address(0)). A maker can call buy()/sell() against their own listing, net-zero economically (balance-checked legs mean no value is created or destroyed), but the offer closes as Filled with an OfferFilled event and an incremented fills counter indistinguishable on-chain from a genuine third-party trade — which could mislead an off-chain volume/activity display for a token with no other liquidity. No fund-safety impact. Optional hardening: if (msg.sender == o.maker) revert NotYourOffer(); in _fillable, if wash-fill resistance in displayed statistics is desired.

[I-2] A blacklisted Buy-offer maker can still have the offer filled, but can never cancel it — the escrow is stranded permanently

Severity: Info
Found by: Phase 2 trust-gap agent (novel, not found elsewhere).

For a Buy offer, sell() pays the quote currency to the taker (_payQuote(quote, msg.sender, quoteAmount), PrimeOTC.sol:407) and the token to the maker (_pushChecked(token, maker, tokenAmount), PrimeOTC.sol:406) — the maker never receives the quote currency during a fill, only the token. cancel(), however, always refunds the quote currency directly to the maker (_payQuote(quote, maker, quoteRefund), PrimeOTC.sol:329). If the maker's address is later blacklisted on that quote currency (USDC is explicitly named in this contract's own KNOWN LIMITS as capable of this), every sell() fill of that Buy offer continues to succeed normally, but every cancel() attempt — by the maker, or by anyone after expiry under note 9's "anyone may return an expired offer" mechanism — reverts forever, because the refund transfer to the blacklisted maker always fails and the whole transaction rolls back. The escrow and the _open/_openPos entry are stuck permanently. This is a narrower, more specific consequence of the contract's own already-disclosed limitation ("a token whose owner can freeze or blacklist addresses can freeze this contract... Nothing here can change that") rather than a new category of risk — reported at Informational severity to document the precise mechanism (fills keep working; only cancel breaks) for the client's awareness, not as an undisclosed bug.

[I-3] Minor residual observations (no action required)

• Read-only reentrancy via view functions mid-fill (Phase 2 periphery agent): getOffer/openOffers/etc. carry no reentrancy guard and could, in principle, return post-settlement/pre-payout state to a contract reentering during a fill's external calls. The agent could not identify any in-scope consumer of these views that acts on such a read mid-transaction — noted as a theoretical gap, not a demonstrated issue.
• Rounding slack accrues to the last partial-fill taker (Phase 2 boundary agent): the final fill against a multi-partial offer takes the literal remainder rather than a ratio-based amount, so it absorbs the accumulated one-sided rounding earlier partial fillers paid away. This is bounded by the dust minimum and is the documented mechanism by which NatSpec note 5's "exact totals" guarantee holds — a fairness curiosity, not a vulnerability.
• No code.length/behavior validation on weth/usdc/dguy at construction (Phase 2 asymmetry/flow-gap agents): unlike the offered token in createOffer (checked for non-zero and code.length != 0), the three quote-currency addresses are validated only for non-zero in the constructor. A one-time deployer-trust gap, not exploitable by any third party post-deployment.

Invariant Verification

Catalog from the x-ray mechanical pre-scan (invariants.md), each inferred row re-examined against both hunting phases' results.

9 inferred invariants answered: 8 hold, 1 violated (I-6 → H-1), 0 unverified.

Coverage Gate

• Entrypoints: 5 external/public state-changing functions in source (createOffer, cancel, buy, sell, receive), 5 addressed. createOffer and receive examined, no issue. cancel addressed by [I-2]. buy/sell addressed by [H-1].
• Threat catalog rows: 10 rows from Phase 0's threat catalog, all answered — 9 "holds" (with citation to the confirming agent(s)), 1 → [H-1].
• Invariants: 9 inferred rows answered (see table above) — 8 hold, 1 violated, 0 unverified.
• Coverage holes closed this pass: 2 — the return-data-bomb gas-griefing mechanism ([L-1]) and the blacklist/cancel-liveness asymmetry ([I-2]) were not present as distinct rows in Phase 0's threat catalog; both were surfaced fresh by the hunting phases and are now folded into this report's findings.

Reconciliation summary: Phase 1 (6 agents) and Phase 2 (12 agents) ran independently and blind to each other. 1 High finding converged across both phases (11 of 18 agents total). 1 Low finding converged within Phase 1 only (3 of 6 agents). 3 Informational observations, none fund-affecting. Coverage holes closed: 2. Invariants: 9 answered (8 hold, 1 violated, 0 unverified).

Overall Assessment

PrimeOTC is an unusually disciplined single-file contract: no privileged roles of any kind, one global reentrancy lock correctly scoped across all four state-changing functions, checks-effects-interactions honored even where the lock would already cover it, and nearly every concern the client's own design notes call out (token isolation, rounding/dust bounds, ETH/WETH fallback, post-expiry cancel, the reentrancy lock, open-set bookkeeping) traces cleanly to a correctly implemented guard under adversarial review from 18 independent hunting agents across two blind methodologies. The one substantive finding ([H-1]) is a real gap between what the NatSpec promises ("every payout in a fill measures the receiver's balance") and what the code does for the quote-currency leg specifically — a narrow but mechanically clear defect that is cheap to fix and should be fixed before deployment, since it also silently defeats the taker's own slippage protection. No test suite or build tooling exists for this target (single gist file) — every property in this report was verified by manual, multi-agent source review only, not by execution; a Foundry invariant suite asserting I-1, I-4, I-5, and X-1 would be the highest-value pre-deployment addition beyond fixing [H-1].