aere-docs/AERE-SECURITY-FIXES.md
Aere Network dd1410c266 The unpublished line of work joins the sanitized public line
The published line and the local line of this repository had no common
ancestor: the public one carried the hygiene pass (no host names, no internal
paths), the local one carried a month of corrections that never shipped. This
commit ports the local work onto the public line, keeping the public hygiene
wording wherever the two touched the same sentence, and keeping the public
version of AERE-CROSS-CLIENT-DETERMINISM.md entirely.

Carried: LICENSE/LICENSING corrections, VERIFY-POLICY.md,
CITATIONS-UNRESOLVED.md remeasured 2026-08-11, the 'audited' adjective removed
from next to Bouncy Castle, citation paths rewritten to published form, AIP-8,
the QA consolidation report, the second EIP validation pass, fork-height
corrections, the AereSink / threshold-factory correction, the forge test
floor, and the architecture-map updates.
2026-08-15 13:54:42 +03:00

52 lines
6.5 KiB
Markdown

# Aere Network: adversarial-review fixes (pre-deploy hardening)
Source review: `aerenew/docs/AERE-SECURITY-REVIEW-NEW-CONTRACTS.md`
Repo: `aerenew/contracts` (solc 0.8.23, OpenZeppelin v4.9.x). All contracts founder-gated, not deployed.
Guard pattern for L1 copied from `aere-contracts/contracts/pqfinality/AereFinalityCertificateVerifier.sol` (`gateway.code.length == 0` revert).
## Per-finding status
### M1 (MEDIUM) FIXED, with proof
File: `aere-contracts/contracts/erc8004/AereReputationRegistry8004.sol`
- Made the adapter `Ownable` (OZ v4, `@openzeppelin/contracts/access/Ownable.sol`, matching the repo).
- Added `mapping(address => bool) public isAuthorizedFeedbackAuthor`, owner functions `addFeedbackAuthor` / `removeFeedbackAuthor` (with `FeedbackAuthorSet` event and a zero-address guard on add), and a new error `NotAuthorizedFeedbackAuthor(address)`.
- Gated `giveFeedback`: first line now reverts `NotAuthorizedFeedbackAuthor` unless `isAuthorizedFeedbackAuthor[msg.sender]`. The rest of the flow (agent-registered, delta clamp, adapter-is-attestor) is unchanged.
- Design choice (documented in NatSpec): owner-curated author allowlist, mirroring the live AereAIReputation curated-attestor model. Scope is authorization-only: the owner cannot set a reputation value, cannot exceed the `{-1,0,+1}` delta clamp every author is held to, and moves no funds. No re-entrancy, no fund/reputation-value backdoor. Read path (`getScore`, `getStats`, `liveKeyOf`, `canWriteThrough`) untouched.
### L1 (LOW) FIXED (x3), with proof
Added the sibling's codeless-gateway guard to each verify path. New error `VerifierHasNoCode()` in each.
- `aere-contracts/contracts/mpc/AerePQAggregateVerifier.sol` constructor: reverts if `gateway.code.length == 0` (after the existing `ZeroGateway` check).
- `aere-contracts/contracts/depin/AereComputeMarketV3.sol` constructor: reverts if `zkVerifier.code.length == 0` (after the existing `ZeroAddress` check).
- `aere-contracts/contracts/compliance/AereBitstringStatusList.sol` constructor: reverts if `nonRevocationVerifier_ != address(0) && nonRevocationVerifier_.code.length == 0`. The zero address is still allowed (keeps the zk non-revocation path fail-closed OFF until configured), which preserves existing behavior. Verifier is immutable, so the constructor is the correct place.
### L2 (LOW) FIXED, with proof
File: `aere-contracts/contracts/agentic/AereVectorStore.sol`
- Bound each paid-query receipt to an intended provider. Added `address provider` to the `QueryReceipt` struct.
- `paidQuery` now takes `address provider` (validated non-zero via existing `ZeroAddress`) and records it in the receipt. Signature is now `paidQuery(storeId, queryHash, provider, agentId, amount, nonce, deadline, authSig)`. The settlement/forwarding flow (AERE402 rail, fee to sink, delta-measured forward to recipient) is unchanged; `receiptId` formula unchanged.
- `attestRetrieval` priced branch now reverts new error `NotReceiptProvider()` unless `msg.sender == receipt.provider`, so a third party (or the payer) cannot front-run the honest provider and burn the receipt with a junk resultRoot. `getReceipt` view extended to also return `provider`.
### L3 (LOW) DOCUMENTED (accepted risk), not code-changed
File: `aere-contracts/contracts/erc8004/AereAP2MandateVerifier.sol`
- Chose DOCUMENT over CODE-CHANGE, per the task's guidance, because the two candidate code fixes are unsuitable: (a) a per-authorization nonce would break the intended standing-mandate ("sign once, spend repeatedly up to the cap") semantics that the design and tests rely on; (b) "bind consumption to the executor" is already the built-in `SETTLEMENT_EXECUTOR` gate.
- Strengthened the contract-level and `authorizeSpend` NatSpec: the open accounting path (`SETTLEMENT_EXECUTOR == address(0)`) is verification/test-only and MUST NOT be used in production; production MUST set `SETTLEMENT_EXECUTOR` to the AERE402 facilitator so a mandate's cap is only ever consumed alongside a real settlement. No funds at risk (this contract holds none); the vector is cap-exhaustion griefing only.
- The existing test "enforces the settlement-executor gate when one is configured" already proves the mitigation works (a non-executor is rejected, the configured executor succeeds). No new test needed.
## Tests added / changed
- `aere-contracts/test/erc8004-adapters.test.js`: `deployRep` now authorizes `other` as a feedback author (both branches). Two pre-existing tests that relied on the ungated `giveFeedback` were updated to call as the authorized `other` (legitimate behavior change from the M1 gate). Added new test "gates giveFeedback to owner-authorized feedback authors (M1)": unauthorized caller reverts `NotAuthorizedFeedbackAuthor` and writes nothing, owner authorizes -> same call succeeds and moves the live score to 10, owner revokes -> re-blocked, and only the owner may manage the allowlist.
- `aere-contracts/test/AerePQAggregate.test.js`: added "rejects a code-less SP1 gateway at construction (L1 fail-open guard)" (EOA gateway reverts `VerifierHasNoCode`, zero reverts `ZeroGateway`, real mock deploys).
- `aere-contracts/test/AereComputeMarketV3.test.js`: added "construction guards" > "rejects a code-less ZK verifier at construction (L1)" (EOA reverts `VerifierHasNoCode`, zero reverts `ZeroAddress`, real mock deploys).
- `aere-contracts/test/aere-pq-screen.test.js`: added "rejects a non-zero code-less non-revocation verifier at construction, but allows zero (L1)" (EOA reverts `VerifierHasNoCode`; zero deploys and `nonRevocationConfigured()==false`; real mock deploys).
- `aere-contracts/test/aere-vector-store.test.js`: the two `paidQuery` calls updated to pass `provider.address`; the paid-query test extended to assert the receipt is bound to the provider and that a third party (`operator`) and even the payer (`consumer`) are rejected `NotReceiptProvider` with the receipt left unconsumed, then the bound provider consumes it successfully (L2 proof).
## Real re-run results (npx hardhat test <file>, from aerenew/contracts)
- aere-contracts/test/erc8004-adapters.test.js .... 19 passing
- aere-contracts/test/aere-vector-store.test.js ... 8 passing
- aere-contracts/test/AerePQAggregate.test.js ..... 18 passing
- aere-contracts/test/AereComputeMarketV3.test.js . 14 passing
- aere-contracts/test/aere-pq-screen.test.js ...... 13 passing
`npx hardhat compile`: green (compiled successfully, then "Nothing to compile" on re-run).
## Notes
- No contracts outside the findings were touched. No existing guard was weakened. The L1 zero-address behavior for AereBitstringStatusList was intentionally preserved. Prose contains no em-dashes or "--".