PrimeOTC.sol — Security Review (Job 1091, fifth review)
Target: PrimeOTC.sol (single file, 758 lines, Solidity =0.8.26, cancun), not deployed.
Source: https://gist.githubusercontent.com/ASTRALDEFI/a7441c5d943ec371ab2fb4ba2dd0a718/raw/12adb64d66a5b42b1057be1717d3780b334af2b0/PrimeOTC.sol
Pin: gist revision 12adb64d66a5b42b1057be1717d3780b334af2b0; sha256 ab43ecac9b0d119424a6d0f3384cdc877daec76e81dc01a239245adfe9faeb08 (36,490 bytes) — verified identical to the client's stated hash. (Gist raw URL, not a git repo; no commit hash applies.)
Chain: Base (8453), WETH9 predeploy / USDC / DGUY quotes.
Severity counts: 0 Critical · 0 High · 0 Medium · 3 Low · 6 Informational
Method and honesty note
Phases run: (0) manual protocol map / access-control inventory / threat catalog by the lead auditor; (1) breadth — one sonnet agent applying the local ethskills checklists (erc20, dos, precision-math, assembly, general, chain-specific); (2) depth — two blind sonnet attack agents (accounting/rounding/price-limit; ETH/WETH/gas/token behaviour); (3) reconciliation by the lead, plus executable verification (Foundry, below). This was a condensed run of the v3 pipeline: the formal x-ray scripts and the 12-agent pashov fan-out were not run — scope is one ~480-nSLOC file, so three independent agents plus manual review and tests were used instead. No Foundry fork test against real Base WETH9 was run (a faithful WETH9 copy was used).
Verification performed (Foundry, solc 0.8.26, cancun)
_mulGtfuzzed (256 runs, random uint256 quadruples incl. extremes) against an independent 128-bit-limb 512-bit multiply: exact. Code uses standard mulmod method;memory-safeannotation is accurate (no memory touched).- Burn-all-gas maker (Sell offer, ETH quote) with taker paying excess: payout falls back to WETH and the buy succeeds. Binary search of minimum gas then sweep of 40 higher gas limits: every gas limit that succeeds delivers the maker's 1 ETH as WETH; none succeeds with a wrong outcome. Min gas ≈ 255k (below it:
LowGas/OOG revert).
Answers to the client's specific questions
- Is
_mulGtexact and memory safe for all inputs? Yes (PrimeOTC.sol:625-639; fuzz-verified; no memory access). - Are both limit directions right? Yes. Buy (
:465) reverts iffquote·asked > maxQuote·filled, i.e. quote/filled > maxQuote/asked. Sell (:511) reverts iffminQuote·filled > quote·asked, i.e. quote/filled < minQuote/asked.askedis the originaltokenAmount; it is reassigned only after the check (:466,:512). - Can the clamp pay more / receive less per token than the limit, bypass partialOk, note-6 minimums or note-5 totals? No. A clamped fill is
whole(takes exactlytokenLeft/quoteLeft), so partialOk is irrelevant (it consumes everything) and the remainder minimums were already enforced by earlier_checkPartcalls. Partial-fill rounding (buy up, sell down) never leaves the remainder priced worse per token for the next taker, so a clamped fill after someone else's front-run still meets the per-token limit. - Does moving the token push ahead of payment in
buyopen a path? No. State is recorded first (_record), the lock is held, and any later revert unwinds the push. Token-hook recipients cannot re-enter. - Is 75,000 enough for the WETH fallback? Yes by hand accounting (≈60k worst, ≈76k left after a receiver burns the full 100k at the 213,187 floor) and by the executed test above against a WETH9 clone. ~12k slack; see I-1.
- Is each change complete and adds nothing new? Yes, apart from the points below (L-2 notes that "every ETH payout is the last step" is not literally true in
buy).
Findings
L-1 — Cancel is unchecked, so a fee-on-send token can draw on other offers' escrow; note 7 wording overstates the protection
Severity: Low
430: if (tokenRefund != 0) _push(IERC20(token), maker, tokenRefund);
Note 7 states a token that starts taking a fee "cannot … draw on other offers' escrow", true for fills (_pushChecked, :694-702) but not cancel, which deliberately skips both checks. Scenario: makers A and B each escrow 100 of token T; T's owner switches on a 10% fee charged on top of sends; A cancels, the contract's balance falls by 110, the extra 10 comes from B's escrow; B's cancel later reverts/short-changes. Impact is confined to offers in that one token and is an accepted trade-off (a maker can always exit). Fix: amend note 7 to say cancel is exempt (or cap by a per-token escrow sum and accept the lock-in risk).
L-2 — buy with ETH and excess msg.value: the refund is not the last payout; a gas-burning maker raises the taker's required gas
Severity: Low
483: _sendEth(maker, quoteAmount);
484: if (msg.value > quoteAmount) _sendEth(msg.sender, msg.value - quoteAmount);
750: if (gasleft() < ETH_GAS_FLOOR) revert LowGas();
A maker contract that burns its 100k and then needs the WETH fallback (~160k total) leaves too little for the second _sendEth's fresh 213,187 floor, so a taker with a hand-set gas limit and excess value reverts LowGas (measured minimum ≈255k in the test). No funds at risk; wallet estimation finds a working limit. Fix: refund the excess before paying the maker (so the maker payout is last), or require msg.value == quoteAmount for ETH quotes. Note 8's "last thing its function does" should be qualified.
L-3 — Minimum floor is meaningless for 18-decimal ETH
Severity: Low (deployment-parameter dependent)
302: uint256 private constant MIN_FLOOR = 1e6;
317: if (minEth < MIN_FLOOR || minUsdc < MIN_FLOOR || minDguy < MIN_FLOOR) revert BadAmount();
1e6 wei is worth nothing, so the constructor accepts an ETH minimum that gives no dust protection and makes note 5/6's "one millionth of the smallest part" bound economically empty; dust-offer spam is cheap on Base. Fix: assert real minimums in the deploy script (e.g. ≥1e12 wei for ETH) or use per-currency floors.
I-1 — Gas constants are tuned to today's Base gas schedule
Floor math (:288-300) holds with ≈12k slack; an EIP-7702-delegated receiver (+2,600 cold access) or a repricing erodes it. Consider WETH_FALLBACK_GAS = 90_000 (unused gas is refunded) and a fork test against Base WETH9.
I-2 — A blacklisted payee (USDC/DGUY) makes fills revert
_pushChecked(q, maker, …) (:480) and :521. Documented under KNOWN LIMITS; takers lose only gas.
I-3 — Front-run partial fill can make a pending partial fill revert (TooSmall/PriceLimit)
_size :614-620, _checkPart :645-650. Costs the front-runner a real fill ≥ minimum; take-the-rest (note 13) mitigates for whole fills.
I-4 — Forced ETH, donated tokens and stray WETH are unrecoverable
receive() :329-331 refuses non-WETH ETH but selfdestruct/coinbase bypass it. Harmless: no logic reads address(this).balance; per-offer accounting is unaffected. By design (note 2).
I-5 — Token compatibility edges
_call (:707-717) accepts no-return or exactly-1; tokens returning false/other words on success revert TransferFailed. Rebasing tokens break _pull's exact check (documented).
I-6 — Fake/lying tokens harm only counterparties of that token
A maker can list a Buy offer in a token whose balanceOf/transfer lie; a taker "delivering" nothing takes the escrowed quote. The maker chose the token; stated under KNOWN LIMITS. USDC/DGUY/WETH escrow of other offers cannot be reached since every payout is capped by the offer's own recorded amounts and the lock blocks reentry.
Coverage (entrypoints / threats / invariants)
Entrypoints: createOffer, cancel, buy, sell, receive, 7 views — all examined. Threat rows examined and holding: reentrancy (transient lock, no early returns), cross-offer escrow solvency, rounding extraction, price-limit exactness, partialOk/minimum bypass, _open swap-and-pop (incl. last==id), expiry boundary consistency (:414 vs :606), returndata bombs, scratch-space mload(0) reads, fee-on-transfer both directions, msg.value routing, ETH-recipient refusal/gas burn, self-fill, uint128 casts. Invariants verified: sum of payouts ≤ escrow per offer; final fill takes exactly both remainders; tokenLeft==0 ⇔ Filled; quoteLeft>0 whenever tokenLeft>0 on open offers (via _checkPart).