Skip to content

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

Open
HandSonic wants to merge 6 commits into
OtterMind:mainfrom
HandSonic:fix/sqli-db2
Open

fix(db2): escape SQL identifiers and literals in metadata/DDL paths (#1914)#2173
HandSonic wants to merge 6 commits into
OtterMind:mainfrom
HandSonic:fix/sqli-db2

Conversation

@HandSonic

@HandSonic HandSonic commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes the DB2 plugin portion of #1914 (34 SQL-injection findings across database plugins). All identifier/literal interpolation in the DB2 plugin's metadata, DDL-builder, and export paths now goes through a new Db2SqlEscapes helper (single-quote doubling for string literals, double-quote doubling + wrapping for delimited identifiers), and free-form inputs that cannot be safely escaped (column default values, length units, index sort direction, fallback column types) are validated against strict whitelists.

Changed paths:

  • DB2MetaData: tableDDL (db2look token call), indexes, view, function, procedure, getMetaDataName
  • DB2DBManager: exportTable/Views/Procedures/Triggers, connectDatabase (SET SCHEMA), dropTable, copyTable, truncateTable
  • DB2SqlBuilder: buildCreateTable/buildAlterTable/buildCreateSchema incl. comments, plus new buildDropTable/buildTruncateTable overrides (SPI defaults interpolated raw names)
  • DB2ColumnTypeEnum: column name escaping; strict DEFAULT value pattern; length-unit whitelist; safe fallback for sized types like VARCHAR(10) that miss the exact enum map (previously delegated to raw SPI buildDefaultColumn — the one exploitable path found in adversarial review)
  • DB2IndexTypeEnum: index/constraint names and comments escaped; ASC/DESC exact-match validation

Two defense-in-depth measures beyond plain escaping:

  • tableDDL rejects schema/table names containing ", because the names are embedded in a double-quoted db2look option string (-td "%s" -t "%s") where quote-doubling is not a valid neutralization.
  • All regex validators are anchored with \A/\z (Java $ matches before a trailing newline).

Trade-offs

  • The strict DEFAULT pattern (\A[A-Za-z0-9_+/: \t.-]+\z) rejects quoted string literals such as 'abc'; string defaults remain available via the existing EMPTY_STRING / NULL keywords. This was an explicit reviewer request to prevent literal-context toggling inside enclosing CREATE/ALTER statements.
  • The fallback column-type whitelist accepts only LETTERS, LETTERS(n), LETTERS(n,n); anything else throws IllegalArgumentException instead of being interpolated.
  • truncateTable emits TRUNCATE TABLE "x" without IMMEDIATE (matches the SPI shape; DB2 LUW may require IMMEDIATE in some contexts — functional nit, not a security issue).

Residual risk

  • SPI IColumnBuilder.buildDefaultColumn still concatenates name/type raw; the DB2 plugin no longer calls it, but other plugins still do. Out of scope for this PR (plugin-level fixes only, to keep the per-plugin PRs independent); tracked separately.
  • buildPageLimit / buildExplain concatenate the user's own SQL by design (SQL-editor flow), unchanged.
  • The db2look option string is parsed inside IBM's SYSPROC.DB2LK_GENERATE_DDL; JDBC-level injection is closed (single quotes doubled) and " is rejected up front, but the procedure's internal option parsing is not auditable here.

Test evidence

  • Db2SqlEscapesTest: 23/23 pass on a clean build (mvn -pl chat2db-community-plugins/chat2db-community-db2 test), covering every changed path with real attack strings: x'; DROP TABLE t; --, bad"name, 0; DROP TABLE t, OCTETS) DROP TABLE t, 1), (evil INT DEFAULT (0, x' OR '1'='1, trailing-newline bypasses (0\n, VARCHAR(10)\n, OCTETS\n, ASC\n), db2look option injection (x" -t "y), and sized-type fallback (VARCHAR(10)).
  • Two independent adversarial review rounds (including live throwaway attack probes against compiled classes, 13 payloads) ended in PASS; every probe was either contained inside a quoted identifier/literal or rejected with IllegalArgumentException.

@HandSonic
HandSonic requested a review from openai0229 as a code owner July 26, 2026 09:48
@openai0229 openai0229 moved this to In Review in Chat2DB Community Jul 26, 2026
@HandSonic
HandSonic marked this pull request as draft July 26, 2026 09:56
@HandSonic

Copy link
Copy Markdown
Contributor Author

Converting to draft: final adversarial review pass is still in progress for this branch. Will mark ready for review once it completes. (process note: PR was opened prematurely by automation)

@openai0229 openai0229 moved this from In Review to In Progress in Chat2DB Community Jul 26, 2026
…aths (OtterMind#1914)

- buildCreateColumnSql fallback: validate columnType against a strict
  type-shape whitelist and escape the column name as a delimited
  identifier instead of delegating to the raw SPI buildDefaultColumn
- tableDDL: reject schema/table names containing double quotes, which
  would otherwise break out of the db2look -td/-t option string
- add regression tests for both paths plus SET SCHEMA, dropTable and
  copyTable escaping composition
…t validation (OtterMind#1914)

- DB2SqlBuilder: override buildDropTable/buildTruncateTable so qualified
  table names are quoted via Db2SqlEscapes instead of the SPI identity
- DB2DBManager: override truncateTable with the same quoting
- DB2ColumnTypeEnum.getByType: strip size suffix so VARCHAR(10) resolves
  to the enum and reaches the validated fallback instead of being skipped
- DB2ColumnTypeEnum: drop parens/comma from DEFAULT_VALUE_PATTERN to block
  column-definition breakout via crafted default values
- add regression tests for all four paths
@HandSonic
HandSonic marked this pull request as ready for review July 26, 2026 10:28
@openai0229 openai0229 moved this from In Progress to In Review in Chat2DB Community Jul 26, 2026
…ayloads (OtterMind#1914)

- DEFAULT_VALUE_PATTERN: drop single-quote from the allowed class so
  crafted defaults cannot toggle string-literal context inside the
  enclosing CREATE/ALTER statement
- anchor DEFAULT_VALUE_PATTERN, UNIT_PATTERN and
  FALLBACK_COLUMN_TYPE_PATTERN with \A/\z instead of ^/$ so a trailing
  newline can no longer smuggle input past validation
- validateAscOrDesc needs no anchor change: it uses exact string
  equality, which is already absolute
- add rejection tests for quote/comma/paren breakout defaults,
  trailing-newline bypasses, and GET_DDL_TOKEN option-string quoting
@HandSonic
HandSonic marked this pull request as draft July 26, 2026 10:57
@openai0229 openai0229 moved this from In Review to In Progress in Chat2DB Community Jul 26, 2026
@HandSonic
HandSonic marked this pull request as ready for review July 26, 2026 11:33
@openai0229 openai0229 moved this from In Progress to In Review in Chat2DB Community Jul 26, 2026
…er review (OtterMind#1914)

- new Db2IdentifierProcessor (SPI ISQLIdentifierProcessor): quoteIdentifier
  with double-quote doubling, escapeString with single-quote doubling
- DB2MetaData overrides getSQLIdentifierProcessor(); metadata call sites use it
- builders/managers/enums use Db2IdentifierProcessor.INSTANCE
- non-escapable validation moved to Db2SqlGuards (default values, length
  units, fallback column types, index sort directions)
- Db2SqlEscapes removed; tests migrated (23 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