fix(vault): verify Aave proxy contract on-chain to block indexer poisoning#2129
Conversation
🔐 Commit Signature Verification✅ All 3 commit(s) passed verification
Summary
Required key type: Last verified: 2026-07-24 08:38 UTC |
|
@greptile review |
Greptile SummaryThis PR adds on-chain verification for Aave proxy addresses. The main changes are:
Confidence Score: 4/5The position risk-read path needs a security fix before merging.
services/vault/src/applications/aave/services/positionService.ts and services/vault/src/applications/aave/hooks/useAaveUserPosition.ts
|
| Filename | Overview |
|---|---|
| services/vault/src/applications/aave/services/assertProxyMatchesOnChain.ts | Adds cached on-chain proxy resolution, mismatch detection, and fail-closed error handling. |
| services/vault/src/applications/aave/services/positionService.ts | Verifies the proxy but continues to perform risk reads against the indexer-derived Spoke address. |
| services/vault/src/applications/aave/services/positionTransactions.ts | Re-verifies the proxy before full-repay sizing and resolves the Core Spoke through the configured adapter. |
| services/vault/src/applications/aave/hooks/useAaveUserPosition.ts | Adds the configured adapter to position loading and its query key. |
| services/vault/src/applications/aave/hooks/useRepayTransaction.ts | Maps proxy mismatches to centralized integrity-warning copy. |
Sequence Diagram
sequenceDiagram
participant UI as Loans UI
participant IDX as Indexer
participant PS as Position service
participant AD as Configured adapter
participant SP as Supplied Spoke
UI->>IDX: Fetch position and config
IDX-->>UI: proxyContract and coreSpokeAddress
UI->>PS: Load live position
PS->>AD: getPosition(user)
AD-->>PS: On-chain proxy
PS->>PS: Verify proxy equality
PS->>SP: Read debt and account data
SP-->>PS: Risk figures
PS-->>UI: Display position
Reviews (1): Last reviewed commit: "fix(vault): verify Aave proxy contract o..." | Re-trigger Greptile
Greptile SummaryThis PR verifies indexer-provided Aave proxy addresses against on-chain state. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Sequence DiagramsequenceDiagram
participant UI as Loans UI
participant Indexer as GraphQL indexer
participant Guard as Proxy guard
participant Adapter as Pinned Aave adapter
participant Spoke as Aave Spoke
UI->>Indexer: Fetch position and proxy
UI->>Guard: Verify adapter, user, and indexed proxy
Guard->>Adapter: getPosition(user)
Adapter-->>Guard: On-chain proxy
alt Proxy matches
Guard-->>UI: Return verified proxy
UI->>Spoke: Read position or debt
else Mismatch, missing position, or RPC failure
Guard-->>UI: Throw and fail closed
end
Reviews (1): Last reviewed commit: "fix(vault): verify Aave proxy contract o..." | Re-trigger Greptile |
jonybur
left a comment
There was a problem hiding this comment.
Solid F8 hardening. Verified the two things this needed to get right: the guard resolves the on-chain proxy via getPosition against the env-pinned adapter (never the indexer's), and it fails closed on both the missing-position case and RPC failures (the .catch also deletes the cache entry so a transient failure doesn't poison future reads). The position service applies the check at the chokepoint AND overwrites position.proxyContract with the verified value so downstream consumers can't accidentally re-read the indexer one; repayFull re-verifies at the value-site as an independent check. The Spoke read is safe too, though the PR doesn't advertise it: config.coreSpokeAddress is itself resolved on-chain in fetchConfig from the same pinned adapter. Case-insensitive compare is correct for checksummed-vs-lowercase inputs, and the per-user cache dedupe keeps the extra RPC at one call. Tests cover the match, mismatch, missing-position, RPC-failure, and repay-amount-byte-identity cases.
bd7daa2 to
d907e0c
Compare
Verify the Aave proxy contract on-chain (F8)
Closes https://github.com/babylonlabs-io/baby-auditor/issues/37
What this fixes
When you click "Repay all" on a loan, the app needs to know exactly how much debt to clear. It reads that debt amount from your
proxyContract— the address that holds your position on-chain — and that proxy address comes from an off-chain indexer (GraphQL). Until now we trusted the indexer's proxy as-is and sized the repayment straight from it.That's a problem. If the indexer is compromised (or just wrong), it can return a different proxy — one with less debt. "Repay all" would then quote against the wrong debt, so you'd under-repay (your real debt stays, and you can't withdraw your collateral until it's cleared). Money can't be redirected — the transaction always settles against your own connected wallet — but the amount you sign for could be wrong.
What we did
Before "Repay all" reads the debt, we now cross-check the indexer's proxy against the blockchain and refuse to proceed if they don't match ("fail closed"). The blockchain is the source of truth.
Concretely:
assertProxyMatchesOnChain, that reads the real proxy from the on-chain Aave adapter (getPosition) and throws a clearProxyMismatchErrorif the indexer's value disagrees (case-insensitive compare). It caches the on-chain lookup per user so we don't repeat the call.repayAll— the one path that sizes a signed transaction from the proxy's debt — right before the debt quote, and read the debt against the verified proxy.This mirrors two guards already in the codebase that follow the same "check the indexer against the chain, fail closed" pattern (
assertReserveMatchesOnChainfor the token, and the reorder-flow membership check for vault IDs). We deliberately reuse the same style so reviewers and future maintainers see one consistent approach.Approaches we considered
Why we chose Option B (repay-only)
The first revision of this PR did A + B. On review we scoped it down to B only, and removed the read/display-path check, because:
useBorrowTransaction) and, more fundamentally, Aave enforces the real health factor on-chain — so a spoofed displayed number can't push a real borrow past the protocol's limit. Repay all is the one place that sizes a signed value from the proxy, and that's exactly what Option B guards.getPositionRPC blip — or normal indexer/chain lag on a freshly-deployed proxy — into a fail-closed blank Loans page (the query is polled every ~30s). That's a bad trade on a path that only shows numbers.So the fix is deliberately minimal: one on-chain cross-check at the single site that funds a signed transaction.
How it behaves
Tests
assertProxyMatchesOnChainunit tests: match → returns the on-chain address; mismatch (attacker's healthier proxy) → throws; checksummed-vs-lowercase → passes; on-chain says "no position" while the indexer supplied one → throws (fail closed); RPC failure → propagates and doesn't poison the cache.repayAlltests: verifies the proxy on-chain before quoting debt; mismatch → throws before reading debt and builds no transaction; the debt quote (which sizes the approval) keys off the verified proxy, not the indexer's.ProxyMismatchErrorsurfaces as the integrity copy and the action returns without submitting.Full Aave suite passes, typecheck and lint clean.
Note
Rebased onto
mainto pick up the repay stale-RPC hardening (#2122), which renamedrepayFull→repayAlland moved the debt read toAaveProxy.getPositionReserveTotalDebt; the F8 check is applied on top of that new structure.