[arrow-adbc] branch main updated: fix(c/driver/postgresql): Prevent SQL Injection in GetTableSchema (#657)

[lidavidm]

This is an automated email from the ASF dual-hosted git repository.

lidavidm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-adbc.git


The following commit(s) were added to refs/heads/main by this push:
     new e0135af  fix(c/driver/postgresql): Prevent SQL Injection in 
GetTableSchema (#657)
e0135af is described below

commit e0135af599ba0f0bab21042c543faa8af54715a1
Author: William Ayd <william....@icloud.com>
AuthorDate: Tue May 9 05:37:19 2023 -0700

    fix(c/driver/postgresql): Prevent SQL Injection in GetTableSchema (#657)
    
    Fixes #643.
---
 c/driver/postgresql/connection.cc      |  2 +-
 c/driver/postgresql/postgresql_test.cc | 36 ++++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/c/driver/postgresql/connection.cc 
b/c/driver/postgresql/connection.cc
index 227c7cb..0513b13 100644
--- a/c/driver/postgresql/connection.cc
+++ b/c/driver/postgresql/connection.cc
@@ -105,7 +105,7 @@ AdbcStatusCode PostgresConnection::GetTableSchema(const 
char* catalog,
     return ADBC_STATUS_INVALID_ARGUMENT;
   }
 
-  int ret = StringBuilderAppend(&query, "%s%s", table_name, 
"'::regclass::oid");
+  int ret = StringBuilderAppend(&query, "%s%s", table, "'::regclass::oid");
   PQfreemem(table);
 
   if (ret != 0) return ADBC_STATUS_INTERNAL;
diff --git a/c/driver/postgresql/postgresql_test.cc 
b/c/driver/postgresql/postgresql_test.cc
index 0765a9c..f86c820 100644
--- a/c/driver/postgresql/postgresql_test.cc
+++ b/c/driver/postgresql/postgresql_test.cc
@@ -30,6 +30,7 @@
 #include "validation/adbc_validation_util.h"
 
 using adbc_validation::IsOkStatus;
+using adbc_validation::IsStatus;
 
 class PostgresQuirks : public adbc_validation::DriverQuirks {
  public:
@@ -97,6 +98,41 @@ class PostgresConnectionTest : public ::testing::Test,
  protected:
   PostgresQuirks quirks_;
 };
+
+TEST_F(PostgresConnectionTest, MetadataGetTableSchemaInjection) {
+  if (!quirks()->supports_bulk_ingest()) {
+    GTEST_SKIP();
+  }
+  ASSERT_THAT(AdbcConnectionNew(&connection, &error), IsOkStatus(&error));
+  ASSERT_THAT(AdbcConnectionInit(&connection, &database, &error), 
IsOkStatus(&error));
+  ASSERT_THAT(quirks()->DropTable(&connection, "bulk_ingest", &error),
+              IsOkStatus(&error));
+  ASSERT_THAT(quirks()->EnsureSampleTable(&connection, "bulk_ingest", &error),
+              IsOkStatus(&error));
+
+  adbc_validation::Handle<ArrowSchema> schema;
+  ASSERT_THAT(AdbcConnectionGetTableSchema(&connection, /*catalog=*/nullptr,
+                                           /*db_schema=*/nullptr,
+                                           "0'::int; DROP TABLE 
bulk_ingest;--",
+                                           &schema.value, &error),
+              IsStatus(ADBC_STATUS_IO, &error));
+
+  ASSERT_THAT(
+      AdbcConnectionGetTableSchema(&connection, /*catalog=*/nullptr,
+                                   /*db_schema=*/"0'::int; DROP TABLE 
bulk_ingest;--",
+                                   "DROP TABLE bulk_ingest;", &schema.value, 
&error),
+      IsStatus(ADBC_STATUS_IO, &error));
+
+  ASSERT_THAT(AdbcConnectionGetTableSchema(&connection, /*catalog=*/nullptr,
+                                           /*db_schema=*/nullptr, 
"bulk_ingest",
+                                           &schema.value, &error),
+              IsOkStatus(&error));
+
+  ASSERT_NO_FATAL_FAILURE(adbc_validation::CompareSchema(
+      &schema.value, {{"int64s", NANOARROW_TYPE_INT64, true},
+                      {"strings", NANOARROW_TYPE_STRING, true}}));
+}
+
 ADBCV_TEST_CONNECTION(PostgresConnectionTest)
 
 class PostgresStatementTest : public ::testing::Test,