Skip to content

fix(kingbase): escape SQL identifiers and literals in metadata/DDL paths (#1914) - #2197

Open
HandSonic wants to merge 3 commits into
OtterMind:mainfrom
HandSonic:fix/sqli2-kingbase
Open

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

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 (59 sites)

New KingBaseSqlEscapes applied across MetaData/DBManager(search_path)/SqlBuilder/type enums; index access methods whitelist-validated; identifier processor quote-doubling.

Verification

mvn -B -pl chat2db-community-plugins/chat2db-community-kingbase -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: 15, 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.

… per maintainer review (OtterMind#1914)

- strengthen KingBaseSQLIdentifierProcessor (SPI ISQLIdentifierProcessor):
  quoteIdentifier always double-quotes with embedded-quote doubling,
  escapeString doubles single quotes; INSTANCE for call sites without
  MetaData access
- KingBaseMetaData returns the processor from getSQLIdentifierProcessor();
  metadata call sites use it (KINGBASE_SQL_IDENTIFIER_PROCESSOR removed)
- DBManager/builder/enums use KingBaseSQLIdentifierProcessor.INSTANCE
- non-escapable validation moved to KingBaseSqlGuards (safe expressions,
  index methods)
- KingBaseSqlEscapes removed; tests migrated (15 green)
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.

2 participants