aiai-bot commented on PR #10327:
URL: https://github.com/apache/seatunnel/pull/10327#issuecomment-3753765499
<!-- code-pr-reviewer -->
## 🔴 Blocking Issues Found
### **1. CRITICAL - SQL Injection in `getDatabaseWithConditionSql()`**
`SqlServerCatalog.java:73-75`
```java
return String.format(getListDatabaseSql() + " where name = '%s'",
databaseName);
```
**Impact**: Database names containing single quotes (e.g., `O'Reilly`) will
cause SQL syntax errors. This is called by
`AbstractJdbcCatalog.databaseExists()`, making it exploitable in production.
**Fix**: Use bracket quoting (consistent with `getListTableSql()`):
```java
return String.format(getListDatabaseSql() + " where name = [%s]",
databaseName);
```
---
### **2. MAJOR - SQL Injection in `getTableWithConditionSql()`**
`SqlServerCatalog.java:78-84`
```java
+ " and TABLE_SCHEMA = '%s' and TABLE_NAME = '%s'"
```
While `databaseName` is escaped via `getListTableSql()`, `schemaName` and
`tableName` are concatenated with single quotes. Schema or table names with
single quotes (e.g., `dbo'`, `my'table`) will break queries.
**Fix**: Escape with brackets:
```java
+ " and TABLE_SCHEMA = [%s] and TABLE_NAME = [%s]"
```
---
### **3. MAJOR - Inconsistency: `getDropTableSql()` Missing Brackets**
`SqlServerCatalog.java:141-143`
Uses `tablePath.getFullName()` (no quotes), while `getExistDataSql()`:174
and `getTruncateTableSql()`:179 use `getFullNameWithQuoted("[", "]")`. Tables
with special characters can be created/truncated but NOT dropped.
**Fix**:
```java
return String.format("DROP TABLE %s", tablePath.getFullNameWithQuoted("[",
"]"));
```
Also update test expectation at `PreviewActionTest.java:366`.
---
### **4. MAJOR - Test Expects Wrong Behavior**
`PreviewActionTest.java:363-367`
DROP_TABLE expects `"DROP TABLE testddatabase.testtable"` (no brackets), but
CREATE_DATABASE expects `"CREATE DATABASE [testddatabase]"` (with brackets).
This test doesn't validate special character handling and will block the
correct fix.
**Fix**: Update expectation:
```java
assertPreviewResult(catalog, Catalog.ActionType.DROP_TABLE,
"DROP TABLE [testddatabase].[testtable]", Optional.empty());
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
