Skip to content

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

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

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

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

Completes PR #2053: adds escapeSqlLiteral to SqlServerIdentifierUtils and applies at remaining DDL/comment/metadata paths; XML/Geography DML values escaped with SQL Server semantics (quote-only doubling).

Verification

mvn -B -pl chat2db-community-plugins/chat2db-community-sqlserver -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: 49, 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 SqlServerIdentifierProcessor (SPI ISQLIdentifierProcessor):
  quoteIdentifier always bracket-quotes with ] doubling, escapeString
  doubles single quotes only (no backslash doubling in T-SQL)
- SqlServerMetaData.getSQLIdentifierProcessor() returns the shared
  INSTANCE; metadata call sites use it (statics use INSTANCE directly)
- builders/DBManager/enums/XML+Geography value processors rewired to
  SqlServerIdentifierProcessor.INSTANCE / static escapeIdentifier
- collation validation moved to public SqlServerSqlGuards
- SqlServerIdentifierUtils removed; tests migrated to
  SqlServerIdentifierProcessorTest (49 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