openpanel: ClickHouse SQL injection via unescaped profile-filter column identifier in cohort query (cohort.service.ts profileColumnAccess)

Summary

profileColumnAccess(name) (@openpanel/db cohort.service.ts) escapes only the profiles.properties.<key> branch; any other name is returned verbatim as a column identifier with NO allowlist, then spliced unescaped into argMax(...), cityHash64(...) and comparison clauses. A project-read member reaches it via cohort.preview, injecting attacker-authored SQL in identifier context. This is a DISTINCT sink from GHSA-hwf2-2v2h-7892, whose fixed function profileColumnSql enforces the PROFILE_COLUMNS allowlist.

Root Cause

cohort.service.ts:308-317: for a non-properties name, profileColumnAccess does return normalizedName verbatim. It is spliced into argMax(${columnAccess}, …) (:462), cityHash64(${referencedColumns.join(', ')}) (:338) and comparison clauses (:468/478/523/527/532). The filter VALUE is sqlstring.escaped but the COLUMN identifier is not. GHSA-hwf2-2v2h-7892's fixed function filter-where.service.ts:188 profileColumnSql enforces if (PROFILE_COLUMNS.has(withoutPrefix)) return withoutPrefix; return null; — this cohort function has no equivalent.

Impact

Attacker-authored SQL expression / correlated subquery in identifier context. A subquery the attacker writes in the HAVING/argMax position is not constrained by the outer project_id scope → cross-tenant ClickHouse read.

Proof of Concept

Cohort preview with a property filter:

{ name: "id, (SELECT groupArray(email) FROM profiles WHERE project_id='victim')", operator: "isNotNull", value: [] }

isNotNull with empty value skips the empty-value early return and emits (${columnAccess} IS NOT NULL AND ${columnAccess} != '') — injection needs no filter value.

Attack Chain

1. Entry: tRPC cohort.preview (cohort.ts:210, protectedProcedure, query). Guard: enforceAccess requires READ on top-level projectId (trpc.ts:96-111, needsWrite=false for query); zChartEventFilter.name = z.string(). Bypass proof: any project-read member; name unconstrained, no allowlist, no isKnownEventField filter applied to cohort property names.
2. Flow: preview → countCohort/computeCohort → computePropertyBasedCohort (:560) → buildPropertyBasedCohortQuery (:352) → chQuery (:569).
3. Check: identifier escaping/allowlist. Guard: none in profileColumnAccess. Bypass proof: returns normalizedName raw (cohort.service.ts:316); value escaped, column NOT.
4. Sink: chQuery(buildPropertyBasedCohortQuery(...)) (:569).
5. Impact: cross-tenant ClickHouse read via attacker subquery in argMax/cityHash64/HAVING (outer project_id WHERE does not constrain it).

Bypass Evidence

HEAD read confirms profileColumnAccess returns the name verbatim for the non-properties branch with no allowlist, spliced unescaped into identifier positions. Distinct function/file/entry from GHSA-hwf2-2v2h-7892's fixed profileColumnSql (which HAS the PROFILE_COLUMNS allowlist on HEAD).

Affected Versions

Rolling deploy; vulnerable on HEAD 61a9245; fix unreleased.

Suggested Fix

Apply the same PROFILE_COLUMNS allowlist used by profileColumnSql to profileColumnAccess, rejecting any non-allowlisted column identifier.

--- Reported by zx (Jace) — GitHub: @manus-use