aere-docs/AERE-SECURITY-REVIEW-NEW-CONTRACTS.md

# Adversarial Security Review: 17 pre-deployment contracts (Aere Network)

Scope: the new Solidity contracts under `aerenew/contracts/contracts/` listed in the task. All are founder-gated and NOT yet on mainnet. This is a report-only review (no fixes applied). Precompile/gateway calls (Falcon `0x0AE1`, SP1 gateway `0x9ca4…0628`) are mocked in tests; the real accept/reject branch was read directly in each contract.

Confidence tags: CONFIRMED (verified in source, and where relevant in tests) vs PLAUSIBLE (reasoned, not exercised). Severity reflects impact-if-triggered times likelihood.

## Headline

No CRITICAL and no HIGH issue was found. The fund-flow contracts (AereComputeMarketV3, AereDestinationSettler, AereAccountMigrator, AereVectorStore) are structurally sound: correct checks-effects-interactions, reentrancy guards on every external-call path, per-asset solvency accounting, no double-pay, no pay-without-proof, and correctly fail-closed Falcon/SP1 consumption. The most material finding is a MEDIUM reputation-integrity gap in the ERC-8004 reputation adapter. The remaining issues are LOW (deployment-conditional fail-open, and two griefing vectors) or INFO.

## Findings table (most severe first)

| ID | Severity | Contract | Function | Issue | Confidence |
|----|----------|----------|----------|-------|------------|
| M1 | MEDIUM | AereReputationRegistry8004 | `giveFeedback` | No caller gate. Once the adapter is an authorized attestor, ANY address can inflate/tank any registered agent's reputation, defeating the live contract's curated-attestor model and its evidence-dedup. | CONFIRMED |
| L1 | LOW | AerePQAggregateVerifier (+ AereComputeMarketV3, AereBitstringStatusList) | constructor / verify paths | SP1 gateway / verifier address is not asserted to have code. `verifyProof` returns void, so a codeless gateway makes the call succeed silently (no extcodesize check) = fail-open. Sibling AereFinalityCertificateVerifier guards this; these do not. | CONFIRMED (code); PLAUSIBLE (needs misdeploy) |
| L2 | LOW | AereVectorStore | `attestRetrieval` | A paid query receipt is not bound to a specific provider. Any address can consume a victim's receipt with a junk `resultRoot`, burning the honest provider's one-shot attestation and wasting the payment. | CONFIRMED |
| L3 | LOW | AereAP2MandateVerifier | `authorizeSpend` | In the open accounting path (`SETTLEMENT_EXECUTOR == 0`), the Falcon signature is a standing authorization that anyone can replay to consume a mandate's cap without a real settlement (cap-exhaustion griefing). | CONFIRMED |
| I1 | INFO | AereDestinationSettler | `fill` | A fee-on-transfer / rebasing output token delivers less than the recorded `deliveredAmount`; the recipient can receive under the DECLARED output even though `deliveredAmount >= declared`. | PLAUSIBLE |
| I2 | INFO | AereBitstringStatusList | `publishStatusRoot` / `verifyNonRevocation` | The status root is controller-attested (self-reported); a dishonest issuer can publish a root that lies about its own revocations. Documented `[MEASURE]`; `getWord` allows independent recomputation. | CONFIRMED (by design) |
| I3 | INFO | AereVectorStore | `paidQuery` | Uses raw `IERC20.transfer` with a bool check; a no-return (USDT-style) settlement token makes paidQuery revert (fail-closed, but a compat foot-gun; deposit side is delta-measured and robust). | PLAUSIBLE |
| I4 | INFO | AereRecoveryRegistry | `getRecord` | Reverts with `UnknownOperator(recordId)` on a bad record id (wrong error label; cosmetic). | CONFIRMED |
| I5 | INFO | AereValidationRegistry8004 | `requestValidation` | Permissionless request creation allows storage-spam; harmless (no funds, validator opts in to respond). | CONFIRMED |

## Per-finding detail

### M1 (MEDIUM) — AereReputationRegistry8004.giveFeedback: permissionless reputation writes / sybil

`giveFeedback(uint256 agentId, int8 delta, bytes32 evidenceHash, string evidenceURI)` (lines 103-114) has no `msg.sender` restriction. It checks only: agent registered, `delta in {-1,0,1}`, and that the adapter itself is an attestor on the live `AereAIReputation`. It then forwards `REPUTATION.attest(operator, agentKey, delta, evidenceHash, evidenceURI)` with the adapter as the on-chain attestor.

The live `AereAIReputation` (read at `agentic/AereAIReputation.sol`) deliberately gates `attest` to a curated, immutable attestor set and adds anti-abuse: no self-attest and dedup by `(attestor, evidenceHash)`. Routing through the adapter collapses all callers into ONE attestor identity, so:
- the `(attestor, evidenceHash)` dedup is defeated because an attacker supplies a fresh `evidenceHash` per call;
- `positiveCount` / `disputeCount` can be driven arbitrarily by any address.

`scoreOf = positiveCount*10 - ... - disputeCount*20` clamped to `[0,10000]`. So an attacker can push any registered agent to `MAX_SCORE` (self-inflate a scam agent) or to `0` (tank a competitor). The contract's own NatSpec notes reputation gates economic routing (e.g. "AERE402 require >= 3000 before routing"), so manipulation has downstream financial consequence.

Exploit: attacker calls `giveFeedback(victimAgentId, -1, keccak(random_i), "")` in a loop (or `+1` to inflate a malicious agent). The test `erc8004-adapters.test.js` demonstrates the enabling behavior directly: `repAdapter.connect(other).giveFeedback(...)` succeeds from a non-Foundation, non-operator signer (lines 292, 306-308).

Condition / mitigation: this bites only when the adapter is registered as an attestor on the reputation instance. The currently-live `AereAIReputation` attestor set is immutable and does not include the adapter, so write-through requires deploying a NEW reputation instance listing the adapter (the documented, founder-gated intended path, `deployRep(true)` in the test). Since that IS the intended production posture for write-through, the flaw is real.

Fix direction: gate `giveFeedback` to an authorized feedback-submitter set (or a per-interaction proof, e.g. an AERE402 settlement receipt binding the caller to a real interaction), so the adapter cannot act as an open reputation firehose. If permissionless feedback is intended, the score must not be treated as sybil-resistant by any downstream gate.

### L1 (LOW) — Verifier constructors do not assert the gateway has code (fail-open if misdeployed)

`ISP1Verifier.verifyProof(...)` is declared `external view` with NO return value (`zkverify/ISP1Verifier.sol`). For an external call to a void-returning function, Solidity omits the `extcodesize` check, so a call to a codeless address SUCCEEDS silently. A verifier that relies on "verifyProof reverts on an invalid proof" therefore fails OPEN if its gateway address has no code.

`AereFinalityCertificateVerifier` guards exactly this: its constructor reverts `VerifierHasNoCode` when `gateway.code.length == 0` (line 199, with a NatSpec comment naming the silent-accept risk). The following do NOT:
- `AerePQAggregateVerifier` constructor (lines 187-192): checks only `gateway != 0` and `vkey != 0`. A codeless gateway makes `verifyAggregate` accept every committee authorization = post-quantum auth bypass for any account bound via `AerePQAggregateModule`.
- `AereComputeMarketV3` constructor (lines 213-218): checks only `zkVerifier != 0`. A codeless `ZK_VERIFIER` makes `submitResultZK` pay the provider with NO valid proof (fund loss). Highest impact of the three.
- `AereBitstringStatusList.verifyNonRevocation` (lines 304-308): `try NON_REVOCATION_VERIFIER.verifyProof {...} return true`. A non-zero-but-codeless verifier returns true for any proof (a revoked credential proves "not revoked").

Severity is LOW because the gateway is an IMMUTABLE constructor argument set by the founder-gated deployer, and the correct value (`0x9ca4…0628`) has code; the fail-open only occurs on deployer error. Impact-if-triggered is severe (fund loss / auth bypass), so this is a should-fix consistency gap rather than a runtime vulnerability.

Fix direction: add the same `if (gateway.code.length == 0) revert` guard used by `AereFinalityCertificateVerifier` to `AerePQAggregateVerifier`, `AereComputeMarketV3`, and `AereBitstringStatusList` (or require a successful known-answer call at construction).

### L2 (LOW) — AereVectorStore: paid-query receipt not bound to the attesting provider

`paidQuery` mints a receipt keyed by `keccak(this, storeId, queryHash, agentId, nonce)` and forwards the net payment to `paymentRecipient`. `attestRetrieval` (lines 344-381) then consumes the receipt with only these checks: exists, not consumed, `storeId`/`queryHash` match, `amount >= queryPrice`. There is NO check that `msg.sender` is any particular provider.

Because `receiptId`, `storeId` and `queryHash` are all public (derivable from the `QueryPaid` event / inputs), any address can front-run the honest retrieval provider and call `attestRetrieval` with a garbage `resultRoot`, flipping `consumed = true`. The honest provider then reverts `ReceiptConsumed` and cannot attest, so the payer paid but obtains no trustworthy attestation (must pay again).

No funds are stolen (the payment already reached `paymentRecipient`), so this is griefing / service-denial, LOW. Fix direction: bind the receipt to an intended provider (recorded at paidQuery or asserted via signature), or record attestations per-provider rather than one-shot per receipt.

### L3 (LOW) — AereAP2MandateVerifier: replayable cap-consumption in the open path

`authorizeSpend` records cumulative spend under `mandateHash` and enforces the cap. The `mandateHash` does NOT include `amount`, so a valid `falconSig` is a standing authorization: each `authorizeSpend` call consumes `amount` from the cap. When `SETTLEMENT_EXECUTOR == address(0)` (the "open accounting path", used in the test at lines 160-163 and 491-492 where one `sig` authorizes two spends), the call is permissionless. An attacker who observes the public `falconSig` can replay `authorizeSpend` to exhaust the mandate's remaining cap without any real settlement, blocking the agent's legitimate spends under that mandate.

The design already contains the mitigation: setting `SETTLEMENT_EXECUTOR` to the AERE402 facilitator makes `authorizeSpend` callable only alongside real settlement (verified by the gated test at lines 542-560). Severity LOW: griefing only, no custody, and preventable by the intended production config. Fix direction: never deploy production with `SETTLEMENT_EXECUTOR == 0`; document the open path as test-only, or add a per-authorization nonce.

### INFO items
- I1: `AereDestinationSettler.fill` records `outputAmount = deliveredAmount` but transfers solver -> recipient directly; a fee-on-transfer token shorts the recipient below the declared output. The user chose the output token, and origin-side repayment is against the locked input, so no protocol fund loss; worth documenting as a supported-token constraint.
- I2: `AereBitstringStatusList` status roots are controller-attested (self-reported); a dishonest issuer can misrepresent its own revocation state. Explicitly marked `[MEASURE]`; `getWord` exposes raw bits for independent recomputation.
- I3: `AereVectorStore.paidQuery` uses raw `transfer` with a bool check; a no-return ERC-20 would make it revert (fail-closed compat foot-gun). Deposit side is delta-measured, so no accounting drift.
- I4: `AereRecoveryRegistry.getRecord` reverts with `UnknownOperator` on a bad record id (mislabeled error, cosmetic).
- I5: `AereValidationRegistry8004.requestValidation` is permissionless (storage spam only; harmless).

## Per-contract verdict

| Contract | Verdict |
|----------|---------|
| pqfinality/AerePQAttestationKeyRegistry.sol | CLEAN. Append-only enforced (rotation flags predecessor, never overwrites); validators register only their own key (`msg.sender == validator`); owner curates membership but cannot forge keys; deterministic root. |
| pqfinality/AereFinalityCertificateVerifier.sol | CLEAN. Correctly fail-closed; binds root/size/domain/quorum to the live registry; `ceil(2N/3)` threshold math verified (N=7->5, 9->6, 4->3); includes the `VerifierHasNoCode` guard the siblings lack. |
| mpc/AerePQAggregateVerifier.sol | ISSUE (L1). Verify logic is fail-closed and correct; constructor missing the codeless-gateway guard. |
| modular/AerePQAggregateModule.sol | CLEAN. Per-account binding via singleton `msg.sender`; fail-closed try/catch decode; delegates to the fail-closed verifier. |
| compliance/AereTrustRegistry.sol | CLEAN. Owner-gated (trusted list operator by design); append-only status/scope history; terminal revocation truly terminal (no transition out of Revoked); fail-closed accreditation and PQC views. |
| compliance/AereVerifiableCredential.sol | CLEAN. Digest binds chainId+contract+all fields (no replay); anchor verifies full path fail-closed; `isValid` re-checks live accreditation/validity/revocation. |
| compliance/AereBitstringStatusList.sol | CLEAN (I2 by-design trust caveat; L1 non-revocation path shares the codeless-verifier gap). Revocation monotonic/terminal; suspension reversible; verifyNonRevocation binds chain/contract/list/purpose + freshness. |
| intents/AereDestinationSettler.sol | CLEAN (I1 note). No custody (solver->recipient direct), exact output-match, at-most-once fill, CEI + nonReentrant, first-filler-wins with no loss to the loser (AlreadyFilled before transfer). |
| depin/AereComputeMarketV3.sol | CLEAN (L1 ZK-verifier deploy note). No double-pay (single terminal status per job), no pay-without-proof (settle after `verifyProof`), per-asset solvency tracked, fee-on-transfer rejected at deposit, Falcon settlement fail-closed, native+ERC20 liability accounting correct (no double-count on native jobs). |
| agentic/AereVectorStore.sol | ISSUE (L2). paidQuery settlement is CEI + nonReentrant and correctly gated; commit is append-only + fail-closed Falcon; the gap is receipt-to-provider binding. |
| pqc/AereAccountMigrator.sol | CLEAN. Pure conduit: caller is always `from`, caller-chosen `destination` is always `to`; no owner/sweep; strict native handling (StrayNative); atomic (SafeERC20 reverts roll back); nonReentrant. |
| oracle/AereRandomnessBeaconV2.sol | CLEAN. Real EIP-2537 BLS12-381 pairing verification, fail-closed end to end (empty/short/wrong-length returns -> false), on-curve + subgroup (precompile-enforced) checks; no path returns true without pairing equality; honest replacement of a stub verifier. |
| AereRecoveryRegistry.sol | CLEAN (I4 cosmetic). Append-only, per-operator monotonic nonce bound into the signed record (no replay), Falcon verification fail-closed, no admin/owner. |
| erc8004/AereIdentityRegistry8004.sol | CLEAN. Metadata binding gated to the live Falcon-root controller; domain uniqueness enforced; no cross-agent privilege. |
| erc8004/AereReputationRegistry8004.sol | ISSUE (M1). Read path is a faithful pass-through; the write path lacks caller authorization. |
| erc8004/AereValidationRegistry8004.sol | CLEAN (I5 spam note). Challenge binds request tuple + response (non-replayable), respond-once, PQC verification fail-closed, active-key re-check at response time. |
| erc8004/AereAP2MandateVerifier.sol | ISSUE (L3, open-path only). mandateHash binds chain+contract+fields; cap overflow-safe; window + active-key + Falcon checks fail-closed; the gap is the open accounting path being replayable. |

## Coverage-gap notes (what the tests do NOT exercise)
- No test drives any verifier with a codeless gateway (L1) - both `AerePQAggregate.test.js` and `AerePQFinalityCertificate.test.js` only use `MockSp1Verifier`, which has code, so the missing-guard divergence is invisible to the suite.
- `erc8004-adapters.test.js` exercises `giveFeedback` from an arbitrary signer as EXPECTED behavior (M1), so the missing caller gate is treated as a feature, not tested as a risk.
- The AP2 open-path signature replay (L3) is exercised as intended (one sig, two spends) without a test asserting that an unrelated caller should be blocked in the open path.
- No test covers `attestRetrieval` being called by an address other than the intended provider (L2).