carlos / platform Public
| Previous finding | Disposition at d16d43c6 | |---|---| | R1-1 — Readiness without established database state | Resolved. Unknown pointers block readiness; off and expired pointers require empty mirrors. | | R1-2 — off waits behind publication | Resolved. Publication holds the slot; off returns 409 immediately. | | R1-3 — Unbounded console pointer reads | Resolved for reads. Pointer reads are streamed and capped. | | R1-4 — Recovery generation deleted after failed synchronization | Resolved for the reported paths. Failed installations preserve loaded and retained generations. | | R1-5 — Rejected-upload test does not establish zero reads | Resolved. The panicking-reader test detects reads before reservation. | | R1-6 — Countries overflow ordering | Resolved. _other remains intact until rendering and sorts last. | | R2-1 — Feed status writes bypass the read cap | Still partly resolved; P2 remains. geoipfeed.go:174 calls Put; release/store.go:408 reads the entire existing object before checking its precondition. An oversized feed.json can exhaust memory. Apply the stat and streamed-hash checks used by PutFrom. | | R2-2 — Timed-out checks never persist failures | Resolved for download timeouts. Persistence receives a separate bounded context. | | R2-3 — Unreadable feed status clears an alert | Resolved. Read and parse failures preserve established feed state. | | R2-4 — Unusable directory prevents “not loaded” alert | Resolved. Pointer state is retained before directory preparation. | | R2-5 — Published alert reasons never change while unhealthy | Resolved. Reason changes update Value and emit an event. | | R3-1 — Console follower keeps serving an expired database | Resolved. Loaded-reader expiry is checked independently of local state and candidate loading. | | R3-2 — Transient status-read failure erases feed history | Resolved. Unknown history is not overwritten. | | R4-1 — Unbounded bucket operations block startup or publication | Resolved for the reported operations. Startup cleanup is asynchronous; maintenance and staging reads have deadlines. | | R4-2 — Restart falsely clears a persisted GeoIP alert | Resolved. Unknown inputs cannot establish recovery. | | R4-3 — Email delivery suppresses active reason changes | Resolved. Successful delivery records the last reason per recipient. | | R4-4 — Region guesses carry another generation’s attribution | Resolved. Location and source are captured under one reader lock. | | R5-1 — Unknown feed state suppresses confirmed expiry | Resolved. Established unhealthy conditions precede the unknown-state guard. | | R6-1 — Failed startup deletion leaves a disabled feed alert firing indefinitely | Resolved. Deletion retries with bounded attempts and capped backoff. | | R7-1 — Unbounded feed edition metadata makes control records unreadable | Resolved. Editions are normalized HTTP dates; oversized serialized pointers are refused. | | R7-2 — Unchanged-edition checks leave the feed reported as “not run yet” | Resolved. Confirmed editions update success history. | | R8-1 — MMDB verification indefinitely holds the publish slot | Resolved for the reported traversal attacks. Verification runs in a child killed at its deadline; commit cleanup releases staging and the slot. | | R9-1 — Malformed MMDB metadata can crash the entire console | Resolved for the reported arithmetic panic. Initial parsing occurs in the verifier child. | | R10-1 — Verifier child has no memory bound | Resolved on Linux for the reported allocation attack. The child requires a 512 MiB RLIMIT_DATA before parsing. | | R10-2 — Retained recovery generations survive GeoLite2 expiry | Resolved for the reported synchronization failure. Unchanged-pointer polls retry synchronization; retained generations receive independent expiry checks. | | R11-1 — Verified database crashes serving processes during lookup | Resolved for the reported stack-exhaustion path. CheckRecords decodes every reachable record through mmdbRecord inside the bounded verifier child before publication. |
New findings:
- P1 — Underscore header spellings bypass stripping and become trusted geo headers in WSGI apps. internal/geoip/answer.go:78
StripAll matches only the hyphenated namespace. Go accepts header names containing underscores, so X_Carlos_Geo_Country: KP, X_Carlos_Geo_Source: forged, and X_Carlos_Hop_Geo: hit;src=forged survive both ingress stripping and setGeo.
Python’s WSGI request handler converts hyphens to underscores and uppercases names. Consequently, these attacker-controlled fields become HTTP_X_CARLOS_GEO_COUNTRY, HTTP_X_CARLOS_GEO_SOURCE, and HTTP_X_CARLOS_HOP_GEO: the same application keys as the platform’s headers. This works when GeoIP is off. With a database loaded, the attacker can also send an unsigned X-Carlos-Hop: 1; the rejected-hop path suppresses legitimate geo headers while preserving the underscore aliases.
Fix: normalize underscores to hyphens when checking whether a name belongs to the reserved namespace, then delete the original header key. Apply this to the whole geo namespace and the internal hop-geo name. Add forgery tests covering mixed separators and a backend that performs WSGI header normalization.
Validation: static review plus an in-memory reproduction using Python’s actual WSGI header conversion. Targeted Go tests could not start because the sandbox refused creation of the build directory; Go runtime verification and cross-compilation remain unverified.
Verdict: revise.