Skip to content

security: validate keyspace and datacenter names to prevent CQL injection (GHSA-p64p-96pw-mxwv)#2472

Merged
erichare merged 1 commit into
mainfrom
security/ghsa-p64p-96pw-mxwv
May 18, 2026
Merged

security: validate keyspace and datacenter names to prevent CQL injection (GHSA-p64p-96pw-mxwv)#2472
erichare merged 1 commit into
mainfrom
security/ghsa-p64p-96pw-mxwv

Conversation

@erichare
Copy link
Copy Markdown
Contributor

Fix for GHSA-p64p-96pw-mxwv.

Originally opened as stargate/data-api-ghsa-p64p-96pw-mxwv#1 in the private advisory fork; re-opened here on a branch in the main repo per team request.

Summary

Two unrelated CQL-injection sinks share a common root cause: user-supplied identifiers are interpolated into CQL via String.format without validation or escaping.

  1. dropKeyspace / dropNamespace — the request name is passed directly into DROP KEYSPACE IF EXISTS "%s". The analogous Create resolvers already validate via NamingRules.KEYSPACE.checkRule(name); the Drop resolvers do not. A double-quote character breaks out of the quoted identifier.
  2. NetworkTopologyStrategy replication mapCreateNamespaceKeyspaceCommandResolver.networkTopologyStrategyMap interpolates user-supplied datacenter-name keys via ", '%s': %d" with no validation. A single-quote in the key breaks out of the literal and lets an attacker inject CQL into the WITH REPLICATION = {…} clause.

Changes

  • DropKeyspaceCommandResolver, DropNamespaceCommandResolver — apply NamingRules.KEYSPACE.checkRule(name), mirroring the Create path.
  • CreateNamespaceKeyspaceCommandResolver — validate each NetworkTopologyStrategy datacenter-name key against an allowlist ([A-Za-z0-9_-]+, max 48 chars). The character set accepts realistic cloud DC names like us-east-1 while rejecting any character that could break the single-quoted CQL literal (notably ', \, whitespace).
  • New error code SchemaException.Code.UNSUPPORTED_REPLICATION_DATA_CENTER_NAME with template in errors.yaml.

Integer replication-factor values are unaffected — Jackson type-coercion rejects non-numeric input and %d only formats numerics.

Test plan

  • New unit tests in DropKeyspaceCommandResolverTest:
    • rejects double-quote injection payload
    • rejects empty name
    • rejects hyphen / special chars
    • same protection on the deprecated dropNamespace alias
  • New unit tests in CreateKeyspaceCommandResolverTest:
    • rejects single-quote injection payload in DC-name key
    • rejects DC name with whitespace
    • rejects DC name over the 48-char limit
    • accepts hyphenated cloud-style DC name (us-east-1) — regression guard for legitimate users
  • ./mvnw test -Dtest='DropKeyspaceCommandResolverTest,CreateKeyspaceCommandResolverTest' — 21 / 21 pass.
  • CI: full test + integration-test suite on this branch.
  • Manual smoke: createKeyspace + dropKeyspace happy path against a real cluster.
  • Confirm vulnerable_version_range on the advisory before publishing the GHSA (currently <= 1.0.x as a placeholder; current pom.xml is 1.0.47-SNAPSHOT).

Notes for reviewers

  • The DC allowlist intentionally includes - so Astra / AWS-region style DC names (dc-westus2, us-east-1) keep working. If you'd prefer the stricter \w+ rule used for keyspace identifiers, flag it and I can tighten.
  • I did not change DropKeyspaceOperation.DROP_KEYSPACE_CQL itself; the validated identifier is still interpolated. The fix relies on the allowlist being CQL-safe. Switching to a QueryBuilder API or driver-side CqlIdentifier quoting would be a larger refactor and is out of scope for this patch.

…tion (GHSA-p64p-96pw-mxwv)

Two unrelated CQL injection sinks shared a common root cause:
user-supplied identifiers were interpolated into CQL via String.format
without validation or escaping.

1. dropKeyspace / dropNamespace passed the request `name` directly into
   `DROP KEYSPACE IF EXISTS "%s"`. The analogous Create resolvers already
   apply `NamingRules.KEYSPACE.checkRule(name)`; this commit applies the
   same validation in both Drop resolvers, mirroring the Create path.

2. NetworkTopologyStrategy replication-map construction in
   CreateNamespaceKeyspaceCommandResolver interpolated user-supplied
   datacenter-name keys via `", '%s': %d"` with no validation. A crafted
   key could break out of the single-quoted CQL literal and inject
   additional CQL into the `WITH REPLICATION = {...}` clause of
   `CREATE KEYSPACE`. This commit adds an allowlist validation
   (`[A-Za-z0-9_-]+`, max 48 chars) that accepts realistic cloud DC names
   (e.g. `us-east-1`) while rejecting any character that could break the
   literal.

Adds a new `UNSUPPORTED_REPLICATION_DATA_CENTER_NAME` SchemaException
code with a corresponding template in `errors.yaml`, and unit tests for
both the drop-validation and DC-name-validation paths.
@erichare erichare requested a review from a team as a code owner May 18, 2026 16:27
@erichare erichare requested a review from tatu-at-datastax May 18, 2026 16:29
@github-actions
Copy link
Copy Markdown
Contributor

📈 Unit Test Coverage Delta vs Main Branch

Metric Value
Main Branch 51.62%
This PR 51.65%
Delta 🟢 +0.03%
✅ Coverage improved!

@github-actions
Copy link
Copy Markdown
Contributor

Unit Test Coverage Report

Overall Project 51.65% 🍏
Files changed 100% 🍏

File Coverage
CreateNamespaceKeyspaceCommandResolver.java 100% 🍏
SchemaException.java 100% 🍏
DropKeyspaceCommandResolver.java 86.67% 🍏
DropNamespaceCommandResolver.java 86.67% 🍏

Copy link
Copy Markdown
Contributor

@tatu-at-datastax tatu-at-datastax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-actions
Copy link
Copy Markdown
Contributor

📈 Integration Test Coverage Delta vs Main Branch (dse69-it)

Metric Value
Main Branch 72.78%
This PR 72.79%
Delta 🟢 +0.00%
✅ Coverage improved!

@github-actions
Copy link
Copy Markdown
Contributor

Integration Test Coverage Report (dse69-it)

Overall Project 72.79% -0.01% 🍏
Files changed 85.9% 🍏

File Coverage
DropKeyspaceCommandResolver.java 100% 🍏
DropNamespaceCommandResolver.java 100% 🍏
SchemaException.java 100% 🍏
CreateNamespaceKeyspaceCommandResolver.java 88.79% -9.48% 🍏

@github-actions
Copy link
Copy Markdown
Contributor

📈 Integration Test Coverage Delta vs Main Branch (hcd-it)

Metric Value
Main Branch 74.21%
This PR 74.21%
Delta 🟢 +0.00%
✅ Coverage improved!

@github-actions
Copy link
Copy Markdown
Contributor

Integration Test Coverage Report (hcd-it)

Overall Project 74.21% -0.01% 🍏
Files changed 85.9% 🍏

File Coverage
DropKeyspaceCommandResolver.java 100% 🍏
DropNamespaceCommandResolver.java 100% 🍏
SchemaException.java 100% 🍏
CreateNamespaceKeyspaceCommandResolver.java 88.79% -9.48% 🍏

@erichare erichare merged commit f034b3b into main May 18, 2026
3 checks passed
@erichare erichare deleted the security/ghsa-p64p-96pw-mxwv branch May 18, 2026 16:55
erichare added a commit that referenced this pull request May 19, 2026
…up to #2472)

PR #2472 closed the GHSA-p64p-96pw-mxwv injection vectors with input
validation. This follow-up addresses the root cause for the keyspace-name
sinks: replace the String.format CQL interpolation in CreateKeyspaceOperation
and DropKeyspaceOperation with the driver's SchemaBuilder using
CqlIdentifier.fromInternal(name). CqlIdentifier doubles embedded quote
characters, so the keyspace identifier is well-formed regardless of upstream
validation.

Driver-gap finding for datacenter names: SchemaBuilder.withNetworkTopologyStrategy(Map)
does NOT escape map keys (see OptionsUtils.extractOptionValue in
java-driver-query-builder 4.19.0-preview1 — a hostile DC name with a
single quote produces broken CQL). The DC-name allowlist applied by the
resolver is therefore the actual security control for the replication map,
not just defense-in-depth. The resolver comment is updated to flag this so
the allowlist is not removed in a later cleanup.

Changes:
- CreateKeyspaceOperation: take (name, strategy, strategyOptions) and build
  the SimpleStatement via SchemaBuilder.createKeyspace(CqlIdentifier).
- DropKeyspaceOperation: build via SchemaBuilder.dropKeyspace(CqlIdentifier).
- CreateNamespaceKeyspaceCommandResolver: drop the dead getReplicationMap
  string-building; keep DC-name validation (now wired via
  validateStrategyOptions). Updated comment documents the driver-gap rationale.
- New operation-level tests pin the SchemaBuilder behaviour: identifier
  escaping for keyspace names, fallback to SimpleStrategy, hyphenated DC
  name support, and a regression-pinning test that documents the driver's
  unescaped-map-key behaviour.
- Existing resolver tests adjusted to the new record fields and existing
  injection-rejection tests retained.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants