Skip to content

fix(oscar): escape SQL identifiers and literals in metadata/DDL paths (#1914)#2202

Open
HandSonic wants to merge 5 commits into
OtterMind:mainfrom
HandSonic:fix/sqli2-oscar
Open

fix(oscar): escape SQL identifiers and literals in metadata/DDL paths (#1914)#2202
HandSonic wants to merge 5 commits into
OtterMind:mainfrom
HandSonic:fix/sqli2-oscar

Conversation

@HandSonic

Copy link
Copy Markdown
Contributor

Part of the SQL-injection hardening tracked in #1914. Prior art: #2052 (Oracle), #2053 (SQL Server), and the wave-1 batch (#2172-#2177).

These are second-order injection paths: values such as table/schema/view/index names originate from the connected database's own metadata, so exploitation requires a maliciously named object in a target database.

What changed (9 sites)

OscarMetaData 5 literal sites escaped; DEFAULT/unit/sort-order whitelists accept legitimate quoted defaults; identifier processor routes through quoteIdentifier with doubling.

Verification

mvn -B -pl chat2db-community-plugins/chat2db-community-oscar -f chat2db-community-server/pom.xml -Dmaven.test.skip=false -DskipTests=false -Dsurefire.includes=**/*Test.java -Dsurefire.failIfNoSpecifiedTests=false -Dmaven.test.failure.ignore=false test

Result: Tests run: 14, Failures: 0, Errors: 0, Skipped: 0.

This branch also passed a two-lens adversarial review (escape-correctness/coverage + regression/test-efficacy); all blocking findings were fixed and re-tested before submission.

…tainer review (OtterMind#1914)

- strengthen OscarIdentifierProcessor: shared INSTANCE, escapeString with
  single-quote doubling, static escapeIdentifier for quoted-template content
- OscarMetaData call sites use getSQLIdentifierProcessor().escapeString(...)
- non-escapable validation moved to OscarSqlGuards (default expressions,
  length units, sort orders)
- OscarSqlEscapes removed; tests migrated (15 green)
…e for DDL paths (OtterMind#1914)

- quoteIdentifier(String) is conditional again: null/blank pass through,
  valid non-keyword identifiers return unquoted, everything else wraps
  with embedded-quote doubling; versioned overload delegates to it
- new quoteIdentifierAlways(String) carries the old always-quote
  SqlEscapes semantics (null -> null, else wrap with doubling)
- quoteIdentifierIgnoreCase keeps its SPI always-quote, case-preserving
  meaning by delegating to quoteIdentifierAlways, so DDL-generation
  sites (OscarUtils/builers/enums) keep emitting quoted identifiers
- tests cover conditional and always behaviors incl. null passthrough
  (15 green)
…gex alert (OtterMind#1914)

'([^']|'')*' -> '[^']*(?:''[^']*)*' (same language, no backtracking)
…eDoS) (OtterMind#1914)

Regexes for quoted literals and function-call defaults are replaced by
O(n) character scanners; numeric/keyword patterns (simple classes) stay.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Review

Development

Successfully merging this pull request may close these issues.

3 participants