fix(oracle): SQL injection, view comment DOT, and pagination column leak

chat2db-community-server/chat2db-community-plugins/chat2db-community-oracle/src/main/java/ai/chat2db/plugin/oracle/OracleMetaData.java

@@ -135,10 +135,7 @@ public String tableDDL(Connection connection, String databaseName, String schema
 
     @Override
     public List<Table> tables(Connection connection, String databaseName, String schemaName, String tableName) {
-        String sql = String.format(SELECT_TABLE_SQL, schemaName);
-        if (StringUtils.isNotBlank(tableName)) {
-            sql = sql + " and A.TABLE_NAME = '" + tableName + "'";
-        }
+        String sql = buildTablesSql(schemaName, tableName);
         return DefaultSQLExecutor.getInstance().execute(connection, sql, resultSet -> {
             List<Table> tables = new ArrayList<>();
             while (resultSet.next()) {
@@ -153,6 +150,18 @@ public List<Table> tables(Connection connection, String databaseName, String sch
         });
     }
 
+    static String buildTablesSql(String schemaName, String tableName) {
+        String sql = String.format(SELECT_TABLE_SQL, escapeSqlLiteral(schemaName));
+        if (StringUtils.isNotBlank(tableName)) {
+            sql = sql + " and A.TABLE_NAME = '" + escapeSqlLiteral(tableName) + "'";
+        }
+        return sql;
+    }
+
+    private static String escapeSqlLiteral(String value) {
+        return StringUtils.replace(value, "'", "''");
+    }
+
 
 
     @Override

chat2db-community-server/chat2db-community-plugins/chat2db-community-oracle/src/main/java/ai/chat2db/plugin/oracle/builder/OracleSqlBuilder.java

@@ -208,24 +208,19 @@ public String buildAlterTable(Table oldTable, Table newTable) {
     public String buildPageLimit(PageLimitRequest request) {
         String sql = request.getSql();
         int offset = request.getOffset();
-        int pageNo = request.getPageNo();
         int pageSize = request.getPageSize();
         int startRow = offset;
         int endRow = offset + pageSize;
         StringBuilder sqlBuilder = new StringBuilder(sql.length() + 120);
+        sqlBuilder.append(SQL_SELECT);
         if (startRow > 0) {
-            sqlBuilder.append(SQL_SELECT);
-        }
-        if (endRow > 0) {
             sqlBuilder.append(SQL_SELECT_TMP_PAGE_ROWNUM_CAHT2DB);
         }
         sqlBuilder.append(SQLConstants.LINE_SEPARATOR);
         sqlBuilder.append(sql);
         sqlBuilder.append(SQLConstants.LINE_SEPARATOR);
-        if (endRow > 0) {
-            sqlBuilder.append(SQL_CLOSE_PAREN_TMP_PAGE_WHERE_ROWNUM_EQUAL);
-            sqlBuilder.append(endRow);
-        }
+        sqlBuilder.append(SQL_CLOSE_PAREN_TMP_PAGE_WHERE_ROWNUM_EQUAL);
+        sqlBuilder.append(endRow);
         if (startRow > 0) {
             sqlBuilder.append(SQL_CLOSE_PAREN_WHERE_CAHT2DB_AUTO_ROW_ID);
             sqlBuilder.append(startRow);
@@ -269,11 +264,11 @@ public String buildCreateView(ModifyView modifyView) {
         createViewSqlBuilder.append(SQLConstants.VIEW_KEYWORD);
         String schemaName = modifyView.getSchemaName();
         if (StringUtils.isNotBlank(schemaName)) {
-            createViewSqlBuilder.append(SQLConstants.DOUBLE_QUOTE).append(schemaName).append(SQLConstants.DOUBLE_QUOTE).append(SQLConstants.DOT);
+            createViewSqlBuilder.append(quoteOracleIdentifier(schemaName)).append(SQLConstants.DOT);
         }
         String viewName = modifyView.getViewName();
         if (StringUtils.isNotBlank(viewName)) {
-            createViewSqlBuilder.append(SQLConstants.DOUBLE_QUOTE).append(viewName).append(SQLConstants.DOUBLE_QUOTE);
+            createViewSqlBuilder.append(quoteOracleIdentifier(viewName));
         } else {
             createViewSqlBuilder.append(UNDEFINED_KEYWORD);
         }
@@ -320,15 +315,30 @@ public String buildCreateView(ModifyView modifyView) {
         String comment = modifyView.getComment();
         if (StringUtils.isNotBlank(comment)) {
             createViewSqlBuilder.append(SQLConstants.LINE_SEPARATOR);
-            createViewSqlBuilder.append(SQL_COMMENT_TABLE)
-                    .append(SQLConstants.DOUBLE_QUOTE).append(schemaName).append(SQLConstants.DOUBLE_QUOTE)
-                    .append(SQLConstants.DOUBLE_QUOTE).append(viewName).append(SQLConstants.DOUBLE_QUOTE)
+            createViewSqlBuilder.append(SQL_COMMENT_TABLE_2);
+            if (StringUtils.isNotBlank(schemaName)) {
+                createViewSqlBuilder.append(quoteOracleIdentifier(schemaName)).append(SQLConstants.DOT);
+            }
+            createViewSqlBuilder.append(quoteOracleIdentifier(viewName))
                     .append(SQLConstants.SQL_IS_LOWER)
-                    .append(comment).append(SQLConstants.SEMICOLON);
+                    .append(quoteStringLiteral(comment))
+                    .append(SQLConstants.SEMICOLON);
         }
         return createViewSqlBuilder.toString();
     }
 
+    private static String quoteOracleIdentifier(String identifier) {
+        return SQLConstants.DOUBLE_QUOTE
+                + identifier.replace(SQLConstants.DOUBLE_QUOTE, SQLConstants.DOUBLE_QUOTE + SQLConstants.DOUBLE_QUOTE)
+                + SQLConstants.DOUBLE_QUOTE;
+    }
+
+    private static String quoteStringLiteral(String value) {
+        return SQLConstants.SINGLE_QUOTE
+                + value.replace(SQLConstants.SINGLE_QUOTE, SQLConstants.SINGLE_QUOTE + SQLConstants.SINGLE_QUOTE)
+                + SQLConstants.SINGLE_QUOTE;
+    }
+
     @Override
     public String buildExplain(String sql) {
         return SQL_EXPLAIN_PLAN_FOR + sql;

chat2db-community-server/chat2db-community-plugins/chat2db-community-oracle/src/test/java/ai/chat2db/plugin/oracle/OracleMetaDataTest.java

@@ -3,9 +3,22 @@
 import org.junit.jupiter.api.Test;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 
 class OracleMetaDataTest {
 
+    @Test
+    void buildTablesSqlEscapesSchemaAndTableFiltersAsLiterals() {
+        String sql = OracleMetaData.buildTablesSql("SCOTT' OR '1'='1", "O'Brien' OR '1'='1");
+
+        assertEquals("SELECT A.OWNER, A.TABLE_NAME, B.COMMENTS FROM ALL_TABLES A "
+                        + "LEFT JOIN ALL_TAB_COMMENTS B ON  A.OWNER = B.OWNER  AND A.TABLE_NAME = B.TABLE_NAME\n"
+                        + "where A.OWNER = 'SCOTT'' OR ''1''=''1'  "
+                        + "and A.TABLE_NAME = 'O''Brien'' OR ''1''=''1'",
+                sql);
+        assertFalse(sql.contains("A.TABLE_NAME = 'O'Brien'"));
+    }
+
     @Test
     void appendRoutineSourceTextPreservesOracleLineTerminators() {
         StringBuilder builder = new StringBuilder("CREATE OR REPLACE ");

chat2db-community-server/chat2db-community-plugins/chat2db-community-oracle/src/test/java/ai/chat2db/plugin/oracle/builder/OracleSqlBuilderTest.java

@@ -1,11 +1,71 @@
 package ai.chat2db.plugin.oracle.builder;
 
+import ai.chat2db.community.domain.api.model.view.ModifyView;
+import ai.chat2db.spi.model.request.PageLimitRequest;
 import org.junit.jupiter.api.Test;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 class OracleSqlBuilderTest {
 
+    @Test
+    void shouldLimitFirstPageWithoutExposingSyntheticRowId() {
+        OracleSqlBuilder builder = new OracleSqlBuilder();
+
+        String sql = builder.buildPageLimit(PageLimitRequest.builder()
+                .sql("SELECT ID, NAME FROM EMPLOYEE")
+                .offset(0)
+                .pageNo(1)
+                .pageSize(10)
+                .build());
+
+        assertEquals("SELECT * FROM ( \nSELECT ID, NAME FROM EMPLOYEE\n ) TMP_PAGE WHERE ROWNUM <= 10", sql);
+        assertFalse(sql.contains("CAHT2DB_AUTO_ROW_ID"));
+    }
+
+    @Test
+    void shouldApplyUpperAndLowerBoundsAfterFirstPage() {
+        OracleSqlBuilder builder = new OracleSqlBuilder();
+
+        String sql = builder.buildPageLimit(PageLimitRequest.builder()
+                .sql("SELECT ID, NAME FROM EMPLOYEE")
+                .offset(10)
+                .pageNo(2)
+                .pageSize(10)
+                .build());
+
+        assertEquals("SELECT * FROM (  SELECT TMP_PAGE.*, ROWNUM CAHT2DB_AUTO_ROW_ID FROM ( \n"
+                        + "SELECT ID, NAME FROM EMPLOYEE\n"
+                        + " ) TMP_PAGE WHERE ROWNUM <= 20 ) WHERE CAHT2DB_AUTO_ROW_ID > 10",
+                sql);
+    }
+
+    @Test
+    void shouldBuildQualifiedAndEscapedViewComment() {
+        OracleSqlBuilder builder = new OracleSqlBuilder();
+        ModifyView view = view("SA\"LES", "ACTIVE\"USERS", "Owner\\team's active users");
+
+        String sql = builder.buildCreateView(view);
+
+        assertTrue(sql.startsWith("CREATE VIEW \"SA\"\"LES\".\"ACTIVE\"\"USERS\""));
+        assertTrue(sql.endsWith(
+                "COMMENT ON TABLE \"SA\"\"LES\".\"ACTIVE\"\"USERS\" is 'Owner\\team''s active users';"));
+        assertFalse(sql.contains("Owner\\\\team"));
+    }
+
+    @Test
+    void shouldBuildUnqualifiedViewCommentWhenSchemaIsMissing() {
+        OracleSqlBuilder builder = new OracleSqlBuilder();
+        ModifyView view = view(null, "ACTIVE_USERS", "Active users");
+
+        String sql = builder.buildCreateView(view);
+
+        assertTrue(sql.endsWith("COMMENT ON TABLE \"ACTIVE_USERS\" is 'Active users';"));
+        assertFalse(sql.contains("\"null\""));
+    }
+
     @Test
     void shouldUseRowidSubQueryWhenLimitingSingleRowDeleteAndUpdate() {
         OracleSqlBuilder builder = new OracleSqlBuilder();
@@ -16,4 +76,13 @@ void shouldUseRowidSubQueryWhenLimitingSingleRowDeleteAndUpdate() {
         assertEquals("UPDATE \"T\" set \"A\" = 1 where rowid in (select rowid from \"T\"" + where + " and rownum = 1)",
                 builder.appendSingleRowLimit("UPDATE", "\"T\"", where, "UPDATE \"T\" set \"A\" = 1" + where));
     }
+
+    private static ModifyView view(String schemaName, String viewName, String comment) {
+        ModifyView view = new ModifyView();
+        view.setSchemaName(schemaName);
+        view.setViewName(viewName);
+        view.setViewBody("SELECT ID FROM EMPLOYEE");
+        view.setComment(comment);
+        return view;
+    }
 }