14 KiB
RED TEAM REPORT: base/core Unhook (commit 4256313)
Scope: luxfi/indexer@main commit 4256313 — replace hanzoai/base/core with chi/v5 + stdlib
Date: 2026-04-12
Reviewer: Red (adversarial)
Target: 10 files, 37 handlers converted, explorer/ package
Summary
7 findings: 0 critical, 1 high, 3 medium, 2 low, 1 info.
No SQL injection, no SSRF, no stop-ship. One high-severity issue (GraphQL mutation-over-GET) requires a one-line fix. Three medium issues affect API compatibility and operational safety.
Findings
[HIGH] H-01: GraphQL HandleFunc Accepts All HTTP Methods (Mutation-Over-GET / CSRF)
Description: RegisterRoutes uses r.HandleFunc("/graphql", ...) and
r.HandleFunc("/local/graphql", ...). Chi's HandleFunc accepts every HTTP
method (GET, POST, PUT, DELETE, PATCH, OPTIONS). The handler branches on
r.Method == http.MethodGet vs everything-else. Any non-GET method (PUT,
DELETE, PATCH) proxies to the G-Chain endpoint or executes local GraphQL,
identical to POST.
This means a mutation can be triggered via GET-with-body on some HTTP clients, or via PUT/DELETE which some CORS preflight policies treat differently.
The old code used v2.Any(...) which is semantically identical, so this is not
a regression from the commit itself. However, the refactor was an opportunity to
tighten this and it was missed.
Location: explorer/explorer.go:219-220
Attack Complexity: Low
Exploitability: Craft a cross-origin PUT request to /v1/explorer/graphql
with a GraphQL mutation body. If CORS allows PUT but not POST (misconfigured
upstream proxy), the mutation executes. Also, some WAFs/CDNs treat GET+body as
passthrough.
Impact: Mutation execution via unexpected HTTP methods. The federated
endpoint proxies to G-Chain, so any G-Chain mutation schema is exposed.
Detectability: Not caught by current tests (tests only exercise GET and POST).
Fix Hint: Replace r.HandleFunc with explicit r.Get + r.Post for both
GraphQL routes. Return 405 for other methods.
[MEDIUM] M-01: Error Response Shape Change — Missing status Field
Description: The old base/core ApiError serialized as:
{"status": 404, "message": "Block not found.", "data": {}}
The new notFoundError / badRequestError helpers serialize as:
{"message": "block not found", "data": {}}
Two breaking differences:
- The
statusfield is absent from the new response body. - The message is not sentence-cased (no capital, no trailing period). The old
code used
inflector.Sentenize()which capitalizes and appends a period.
Any frontend that reads response.body.status to determine the error type
(rather than the HTTP status code) will break.
Location: explorer/explorer.go:258-271
Attack Complexity: N/A (compatibility issue)
Impact: Frontend breakage for error handling code that parses the JSON body.
Fix Hint: Add "status": code to the error map. Capitalize and period-terminate
the message string to match old behavior, or document the shape change and
coordinate with frontend teams.
[MEDIUM] M-02: No Panic Recovery Middleware
Description: The old base/core router included built-in panic recovery.
The new RegisterRoutes creates a bare chi.NewRouter() with no middleware.
If any handler panics (nil pointer dereference in scanMaps, unexpected type
assertion in formatBlock, etc.), the entire goroutine crashes and the HTTP
connection drops without a response.
The outer mux (in cmd/explorer/main.go) also uses http.NewServeMux() with
no recovery middleware. A single panic in any handler kills the connection and
may leave the client hanging.
Location: explorer/explorer.go:169
Attack Complexity: Medium — requires triggering a code path that panics
Impact: DoS via connection drop. Repeated panics degrade service for all
clients on the same listener.
Detectability: Would show as a goroutine crash in logs but no structured
error response.
Fix Hint: Add r.Use(middleware.Recoverer) as the first middleware in
RegisterRoutes, or apply it at the outer mux level.
[MEDIUM] M-03: No Graceful Drain for Service.Close() In-Flight Requests
Description: Service.Close() stops the notification worker and closes DB
connections immediately. If called while HTTP handlers are mid-flight (e.g.,
during a queryTxListCtx with a 5-second context timeout), the DB handle
becomes invalid and queries fail with sql: database is closed.
The cmd/explorer/main.go calls srv.Shutdown(context.Background()) which
drains HTTP connections, but Service.Close() is not coordinated with this.
If the shutdown sequence is: (1) cancel context, (2) srv.Shutdown(), (3)
Service.Close() -- that's correct. But Service.Close() is not documented
as requiring this order, and the standalone server mode doesn't call it at all
(the indexer goroutines just exit on context cancellation).
Location: explorer/explorer.go:108-114
Attack Complexity: Low (race during shutdown)
Impact: Request errors during shutdown window. Not exploitable by an
external attacker, but affects reliability.
Fix Hint: Document the shutdown ordering requirement. Optionally, add a
sync.WaitGroup to Service that handlers increment/decrement, and have
Close() wait for it to drain before closing DB connections.
[LOW] L-01: Chi URLParam Passes NUL Bytes to hexToBytes
Description: Chi URL-decodes %00 to a literal NUL byte (0x00) before
returning it from chi.URLParam(). This NUL byte reaches hexToBytes() which
calls hex.DecodeString(). The hex decoder silently truncates at the invalid
byte, returning a partial decode.
The truncated byte slice is then used as a parameterized SQL query argument. This is safe (no injection possible) but produces incorrect query results -- the query matches a shorter prefix instead of the intended address.
Example: /addresses/0xdeadbeef%00pwned decodes to URLParam = "0xdeadbeef\x00pwned",
hexToBytes returns []byte{0xde, 0xad, 0xbe, 0xef} (truncated at NUL).
The query WHERE hash = ? matches 0xdeadbeef instead of returning 404.
Location: explorer/handlers.go (all chi.URLParam call sites),
explorer/format.go:67-71
Attack Complexity: Low
Impact: Incorrect query results. An attacker could discover address prefixes
by probing with NUL-terminated strings. Not a data leak (the address data is
public) but violates the principle of exact-match lookups.
Fix Hint: Validate chi.URLParam results against hexAddrPattern or
hexHashPattern regex before passing to hexToBytes. Return 400 for invalid
format. The standalone server already does this (isValidHexAddr).
[LOW] L-02: Deleted collections.go — Watchlist/Tags/Custom-ABI Endpoints Gone
Description: collections.go was deleted. It defined four Base collections:
explorer_watchlist, explorer_address_tags, explorer_tx_tags,
explorer_custom_abis. These were user-facing features that allowed registered
users to:
- Maintain a watchlist of addresses
- Tag addresses and transactions
- Upload custom ABIs for unverified contracts
The commit message says "not used by explorer; chain data read from indexer SQLite, not Base DB". This is correct for chain data, but these collections stored user data, not chain data. If any frontend calls these endpoints, those features are now broken.
Verification: Grepped luxfi/explore (frontend repo not available locally).
The endpoints were Base-native CRUD on /api/collections/explorer_watchlist/records
etc., not on /v1/explorer/*. Since the explorer binary (cmd/explorer/main.go)
uses StandaloneServer (not the Service+chi path), these collections were only
available when the explorer ran inside a Base instance. The standalone binary
never had them.
Location: explorer/collections.go (deleted)
Impact: If any deployment ran the old Base+plugin mode and users created
watchlist/tag data, that data pathway is gone. Feature regression, not a
security issue.
Fix Hint: If watchlist/tags are needed, implement them as standalone endpoints
backed by SQLite (not Base collections). Otherwise, document the removal.
[INFO] I-01: writeJSON Does Not Handle Encode Errors
Description: writeJSON calls json.NewEncoder(w).Encode(v) after
w.WriteHeader(code). If Encode fails (e.g., the value contains a channel
or function — impossible in practice for the current handlers), the HTTP status
code is already written and cannot be changed. The error is silently discarded.
This is a theoretical concern. All current callers pass map[string]any or
struct values that are guaranteed JSON-serializable.
Location: explorer/explorer.go:245-249
Impact: None in practice.
Fix Hint: Log encode errors if you want defense-in-depth.
Blue's Flagged Vectors — Disposition
Vector 1: Chi path-param with 0x-prefixed hex + URL-special chars
Status: PARTIALLY CONFIRMED (L-01 above)
%2F(slash): Chi does NOT decode it in path params. Returns literal%2F. Safe.%23(#): Chi decodes to#.hexToBytestruncates at#. Same prefix-match issue as NUL. Low.%00(NUL): Chi decodes to\x00.hexToBytestruncates. Low.- SQL injection: Impossible. All values pass through
hexToBytes→[]byte→ parameterized query. No string interpolation of user input into SQL.
Vector 2: HandleFunc method dispatch on GraphQL
Status: CONFIRMED (H-01 above)
HandleFuncaccepts all methods. PUT/DELETE/PATCH reach the handler.- Handler treats all non-GET as "execute query" (same as POST).
- The old
v2.Any(...)had the same behavior, so not a regression, but the refactor should have tightened it.
Vector 3: Error JSON shape mismatch
Status: CONFIRMED (M-01 above)
- Old:
{"status": 404, "message": "Block not found.", "data": {}} - New:
{"message": "block not found", "data": {}} - Missing
statusfield and different message casing.
Vector 4: External callers of removed Register(core.App, Config)
Status: CLEAR
- Grepped entire
luxfi/indexerrepo: zero calls toexplorer.Register(orexplorer.MustRegister(. cmd/explorer/main.gousesNewStandaloneServer, not the Service path.- The Service path is only used in tests. No external breakage.
Vector 5: Deleted collections.go
Status: CONFIRMED LOW (L-02 above)
- Watchlist/tags/custom-ABIs were Base-native CRUD, not explorer API routes.
- Only available in Base+plugin mode (not standalone binary mode).
- Standalone binary (the production path) never had these features.
- If no Base+plugin deployment exists, this is a no-op deletion.
Additional Checks — Disposition
Middleware order
- Recoverer: ABSENT (M-02). No panic recovery in
RegisterRoutesor the outer mux. - CORS: Not handled in
RegisterRoutes. Expected to be handled by the ingress layer (hanzoai/ingress). Acceptable if the deployment guarantees ingress-level CORS. - Auth: The old Base plugin had Base's auth middleware. The new Service has none. This is intentional (the explorer serves public chain data), but if any handler needs auth in the future, it must be added explicitly.
SSE handler / Flusher / goroutine leak
- Not applicable to this commit. The Service handlers do not include SSE. SSE/WebSocket is in
standalone.go(therealtimeHandlermethod onStandaloneServer), which was not changed by this commit.
Graceful shutdown
Service.Close()exists but is not coordinated with HTTP drain (M-03).cmd/explorer/main.gohas signal handling +srv.Shutdown()+ context cancellation. Adequate for the standalone path.
GraphQL query-depth/complexity limiting
- The old Base+plugin mode did not have query-depth limiting either. The local GraphQL handler is a stub (returns
__typenameonly). The federated handler is a reverse proxy to G-Chain, which must enforce its own limits. No regression.
Response Content-Type
writeJSONexplicitly setsapplication/json. Correct.writeHTMLexplicitly setstext/html; charset=utf-8. Correct.
HTTP status codes
writeJSONtakes explicit code parameter. Callers pass correct codes. Correct.notFoundErrorreturns 404,badRequestErrorreturns 400. Correct.
Build verification
go build ./explorer/succeeds.go vet ./explorer/succeeds.- Refactored handler tests pass (GraphQL, cross-chain search, blocks, transactions).
- Pre-existing standalone test failures (DEX routes, concentration endpoint) are not caused by this commit.
Blue Handoff
What Blue got right
- Clean 1:1 handler conversion. All 18
PathValue→chi.URLParammappings verified. - Parameterized SQL throughout. No string interpolation of user input into queries.
writeJSONsets Content-Type explicitly. Good.- LimitReader on webhook body parsing preserved from old code.
- SSRF protection in notification worker preserved (isInternalURL, redirect blocking, DNS rebinding re-check at delivery time).
- Service lifecycle (NewService/Close) is clean. Constructor does setup, Close does teardown.
- Test conversion from
newTestEventtohttptest.NewServeris correct and more realistic.
What Blue missed
- GraphQL
HandleFuncshould beGet+Post(H-01). - Error response body shape changed (M-01) —
statusfield dropped, message casing changed. - No panic recovery middleware (M-02).
- No input validation on chi.URLParam results before hexToBytes (L-01) — standalone server validates, Service handlers do not.
Fix priority for Blue
- H-01: Replace
r.HandleFuncwithr.Get+r.Postfor GraphQL routes (1 line each). - M-01: Add
"status": codeto error response maps, or document the breaking change. - M-02: Add
r.Use(middleware.Recoverer)inRegisterRoutes. - L-01: Add hex validation to URLParam results (optional but recommended for parity with standalone).
Re-review scope
After fixes: re-test GraphQL routes with PUT/DELETE to confirm 405, and verify error response JSON shape matches old format.
RED COMPLETE. Findings ready for Blue. Total: 0 critical, 1 high, 3 medium, 2 low, 1 info Top 3 for Blue to fix:
- GraphQL HandleFunc accepts all methods (H-01)
- Error response missing
statusfield (M-01) - No panic recovery middleware (M-02)
Re-review needed: yes — GraphQL method restriction, error shape Recommendation: fix-then-ship