fix(elasticsearch): reject URL-special characters in decoded Cloud ID parts

The base64-alphabet check validated the encoded payload, not the decoded
components, so a Cloud ID could inject into the assembled origin. Elastic's own
decoder rejects #@?/ in the host and each UUID, with fixtures named inject-es,
inject-kb and inject-host.

An '@' is the serious one: it turns the UUID into userinfo and hands the origin
to the attacker, so the ApiKey header is sent to their host. The other three
truncate the authority to a bare label.

Guarded on the decoded component before assembly rather than on the finished
string, since checking the assembled URL means re-parsing the parse being
subverted. ':' is rejected too, beyond Elastic's set: the port split takes only
the last colon, so a second one produces a two-colon authority and a bare
Invalid URL -- the same unactionable failure the port fix removed. Both are
no-ops on a real hostname and a hex UUID.

The Kibana UUID is deliberately not checked: we only ever build the
Elasticsearch origin, so rejecting on a component that never reaches our URL
would refuse a Cloud ID that works fine for search.

Author: waleedlatif1

apps/sim/tools/elasticsearch/utils.test.ts

@@ -328,3 +328,131 @@ describe('safeIndexPathSegment', () => {
     expect(safeIndexPathSegment('logs-*', 'index')).toBe('logs-*')
   })
 })
+
+/**
+ * The decoded payloads of Elastic's `inject-es`, `inject-host`, and `inject-kb`
+ * `TestDecodeError` fixtures, re-encoded under an ordinary label.
+ *
+ * Elastic writes those fixtures with a literal `#:` marker before the base64
+ * ("inject-es:#:<payload>"). Elastic's decoder splits the label at the LAST
+ * colon so the marker is discarded, while ours splits at the FIRST — a
+ * deliberate difference pinned by the "takes the payload as everything after
+ * the FIRST colon" test above, which the base64-alphabet check then rejects.
+ * Carrying the marker through would make these tests pass on the wrong error,
+ * so only the decoded payload is reused.
+ */
+const INJECTION_FIXTURES = {
+  injectEs: cloudId(
+    'inject-es',
+    'us-east-1.aws.found.io$cec6f261#attacker.com$c6c2ca6d042249af0cc7d7a9e9625743'
+  ),
+  injectHost: cloudId(
+    'inject-host',
+    'us-east-1.aws.found.io#attacker.com$cec6f261a74bf24ce33bb8811b84294f$c6c2ca6d042249af0cc7d7a9e9625743'
+  ),
+  injectKb: cloudId('inject-kb', `us-east-1.aws.found.io$${ES_UUID}$c6c2ca6d#attacker.com`),
+} as const
+
+describe('parseCloudId rejects URL-special characters in decoded components', () => {
+  it("rejects a '#' in the Elasticsearch UUID (Elastic's inject-es fixture)", () => {
+    expect(() => parseCloudId(INJECTION_FIXTURES.injectEs)).toThrow(/invalid character/)
+  })
+
+  it("rejects a '#' in the parent domain (Elastic's inject-host fixture)", () => {
+    expect(() => parseCloudId(INJECTION_FIXTURES.injectHost)).toThrow(/invalid character/)
+  })
+
+  it.each([
+    ['#', 'truncates the authority into a fragment'],
+    ['@', 'turns the UUID into userinfo and hands the origin to the rest'],
+    ['?', 'truncates the authority into a query'],
+    ['/', 'truncates the authority and turns the rest into a path'],
+  ])('rejects %j in the Elasticsearch UUID, which otherwise %s', (char) => {
+    const id = cloudId('evil', `${PARENT_DN}$${ES_UUID}${char}attacker.com$${KIBANA_UUID}`)
+
+    expect(() => parseCloudId(id)).toThrow(/invalid character/)
+  })
+
+  it.each(['#', '@', '?', '/'])('rejects %j in the parent domain', (char) => {
+    const id = cloudId('evil', `${PARENT_DN}${char}attacker.com$${ES_UUID}$${KIBANA_UUID}`)
+
+    expect(() => parseCloudId(id)).toThrow(/invalid character/)
+  })
+
+  /**
+   * A colon needs a companion to survive: `splitCloudIdPort` right-partitions
+   * the last one as a port, so a single trailing colon is already caught by
+   * the port guard (`:attacker.com` parses to NaN). Two colons leave one
+   * behind in the component, which assembles an authority the URL parser
+   * rejects outright — the case the reject set has to catch.
+   */
+  it('rejects a colon left in the Elasticsearch UUID after the port is split off', () => {
+    const id = cloudId('evil', `${PARENT_DN}$${ES_UUID}:attacker.com:9243$${KIBANA_UUID}`)
+
+    expect(() => parseCloudId(id)).toThrow(/invalid character/)
+  })
+
+  it('rejects a colon left in the parent domain after the port is split off', () => {
+    const id = cloudId('evil', `${PARENT_DN}:attacker.com:9243$${ES_UUID}$${KIBANA_UUID}`)
+
+    expect(() => parseCloudId(id)).toThrow(/invalid character/)
+  })
+
+  it('still catches a single trailing colon through the port guard', () => {
+    const id = cloudId('evil', `${PARENT_DN}$${ES_UUID}:attacker.com$${KIBANA_UUID}`)
+
+    expect(() => parseCloudId(id)).toThrow(/invalid host or port/)
+  })
+
+  it('names the offending component and character in the error', () => {
+    const id = cloudId('evil', `${PARENT_DN}$${ES_UUID}@attacker.com$${KIBANA_UUID}`)
+
+    expect(() => parseCloudId(id)).toThrow(/"@"/)
+    expect(() => parseCloudId(id)).toThrow(new RegExp(ES_UUID))
+  })
+
+  it('checks the decoded component, not the assembled URL', () => {
+    /**
+     * `<es-uuid>@attacker.com` assembles into
+     * `https://<es-uuid>@attacker.com.us-east-1.aws.found.io`, whose origin is
+     * `attacker.com.us-east-1.aws.found.io` — a different host, reached with
+     * the caller's ApiKey or Basic credential attached. An assembled-string
+     * check would have to re-parse that to notice.
+     */
+    const id = cloudId('evil', `${PARENT_DN}$${ES_UUID}@attacker.com$${KIBANA_UUID}`)
+
+    expect(() => parseCloudId(id)).toThrow()
+  })
+
+  it("accepts a '#' in the Kibana UUID, which never reaches an Elasticsearch URL", () => {
+    /**
+     * A deliberate divergence from Elastic's decoder, which validates the
+     * Kibana component because it also builds a Kibana URL. We only ever build
+     * the Elasticsearch origin from words[0] and words[1], so rejecting on
+     * words[2] would refuse a Cloud ID that is perfectly usable for search.
+     */
+    expect(parseCloudId(INJECTION_FIXTURES.injectKb)).toBe(`https://${ES_UUID}.${PARENT_DN}`)
+  })
+
+  it('is a no-op on every legitimate fixture, including the ported ones', () => {
+    expect(parseCloudId(ELASTIC_FIXTURES.customPort)).toBe(
+      `https://${GCP_ES_UUID}.${GCP_PARENT_DN}:9243`
+    )
+    expect(parseCloudId(ELASTIC_FIXTURES.differentEsKbPort)).toBe(
+      `https://${GCP_ES_UUID}.${GCP_PARENT_DN}:9243`
+    )
+    expect(parseCloudId(ELASTIC_FIXTURES.hostAndKbSet)).toBe(
+      `https://${GCP_ES_UUID}.${GCP_PARENT_DN}:9243`
+    )
+    expect(parseCloudId(ELASTIC_FIXTURES.onlyKbSet)).toBe(`https://${GCP_ES_UUID}.${GCP_PARENT_DN}`)
+    expect(parseCloudId(ELASTIC_FIXTURES.extraItems)).toBe(
+      `https://${GCP_ES_UUID}.${GCP_PARENT_DN}`
+    )
+  })
+
+  it('leaves the dots and hyphens a real hostname and hex UUID are made of alone', () => {
+    const id = cloudId('ok', `es-prod.eu-west-1.aws.found.io$${ES_UUID}$${KIBANA_UUID}`)
+
+    expect(parseCloudId(id)).toBe(`https://${ES_UUID}.es-prod.eu-west-1.aws.found.io`)
+  })
+})

apps/sim/tools/elasticsearch/utils.ts

@@ -26,6 +26,56 @@ function splitCloudIdPort(component: string, fallbackPort: number): { name: stri
   }
 }
 
+/**
+ * Characters that must never appear in a decoded Cloud ID component.
+ *
+ * `#@?/` is Elastic's own reject set, applied in `decodeCloudID`
+ * (`beats/libbeat/cloudid/cloudid.go`) to the parent domain and each service
+ * UUID, with the `inject-es`, `inject-host`, and `inject-kb` fixtures pinning
+ * it. Each one lets a decoded component escape the authority it is supposed to
+ * be part of, verified against the WHATWG parser `fetch` uses:
+ *
+ * ```
+ * new URL('https://uuid@attacker.com.host').origin // => 'https://attacker.com.host'
+ * new URL('https://uuid#attacker.com.host').origin // => 'https://uuid'
+ * new URL('https://uuid?x.host').origin            // => 'https://uuid'
+ * new URL('https://uuid/p.host').origin            // => 'https://uuid'
+ * ```
+ *
+ * `@` is the dangerous one: the UUID becomes userinfo and the origin becomes
+ * the attacker's host, which the caller's ApiKey or Basic credential is then
+ * sent to. The other three silently truncate the authority to a bare label.
+ *
+ * `:` is a deliberate addition to Elastic's set. {@link splitCloudIdPort}
+ * right-partitions only the *last* colon, so `a:b:9243` leaves `a:b` as a
+ * component and assembles an authority with two colons that the URL parser
+ * rejects outright — the same unactionable "Invalid URL" this module already
+ * fixes for ported UUIDs. A real parent domain is a hostname and a real
+ * service UUID is hex, so none of these five can occur in a legitimate value.
+ */
+const CLOUD_ID_FORBIDDEN_CHARS = ['#', '@', '?', '/', ':'] as const
+
+/**
+ * Rejects a decoded Cloud ID component that could escape the URL authority.
+ *
+ * Runs on the component **after** its port has been split off and **before**
+ * it is concatenated into the origin. Checking the assembled string instead
+ * would mean re-parsing the very URL whose parse is being subverted.
+ *
+ * @param component - A decoded, port-stripped Cloud ID component.
+ * @param label - Names the component in the error message.
+ * @throws If the component contains any of {@link CLOUD_ID_FORBIDDEN_CHARS}.
+ */
+function assertSafeCloudIdComponent(component: string, label: string): void {
+  for (const char of CLOUD_ID_FORBIDDEN_CHARS) {
+    if (component.includes(char)) {
+      throw new Error(
+        `Cloud ID is not properly formatted (${label} "${component}" contains the invalid character "${char}")`
+      )
+    }
+  }
+}
+
 /**
  * Decodes an Elastic Cloud ID into the Elasticsearch origin it addresses.
  *
@@ -115,6 +165,15 @@ export function parseCloudId(rawCloudId: string): string {
     throw new Error('Cloud ID is not properly formatted (invalid host or port)')
   }
 
+  /**
+   * The Kibana UUID (`words[2]`) is deliberately not checked. Elastic validates
+   * it because it also builds a Kibana URL; we only ever build the
+   * Elasticsearch origin, so rejecting on a component that never reaches it
+   * would refuse a Cloud ID that works fine for search.
+   */
+  assertSafeCloudIdComponent(parentDn, 'host')
+  assertSafeCloudIdComponent(esUuid, 'Elasticsearch UUID')
+
   const origin = `https://${esUuid}.${parentDn}`
   return port === 443 ? origin : `${origin}:${port}`
 }