Merge pull request #1758 from lonvia/check-for-invalid-prefix-and-schema-names

Check more user-supplied strings used as SQL identifier

Author: lonvia

src/options.cpp

@@ -10,6 +10,7 @@
 #include "format.hpp"
 #include "logging.hpp"
 #include "options.hpp"
+#include "pgsql.hpp"
 #include "reprojection.hpp"
 #include "util.hpp"
 #include "version.hpp"
@@ -394,6 +395,7 @@ options_t::options_t(int argc, char *argv[]) : options_t()
             break;
         case 'p':
             prefix = optarg;
+            check_identifier(prefix, "--prefix parameter");
             break;
         case 'd':
             database_options.db = optarg;
@@ -557,9 +559,11 @@ options_t::options_t(int argc, char *argv[]) : options_t()
             break;
         case 215:
             middle_dbschema = optarg;
+            check_identifier(middle_dbschema, "--middle-schema parameter");
             break;
         case 216:
             output_dbschema = optarg;
+            check_identifier(output_dbschema, "--output-pgsql-schema parameter");
             break;
         case 217:
             if (std::strcmp(optarg, "false") == 0) {

src/output-flex.cpp

@@ -983,25 +983,12 @@ int output_flex_t::app_as_geometrycollection()
     return 1;
 }
 
-static void check_name(std::string const &name, char const *in)
-{
-    auto const pos = name.find_first_of("\"',.;$%&/()<>{}=?^*#");
-
-    if (pos == std::string::npos) {
-        return;
-    }
-
-    throw std::runtime_error{
-        "Special characters are not allowed in {} names: '{}'."_format(in,
-                                                                      name)};
-}
-
 flex_table_t &output_flex_t::create_flex_table()
 {
     std::string const table_name =
         luaX_get_table_string(lua_state(), "name", -1, "The table");
 
-    check_name(table_name, "table");
+    check_identifier(table_name, "table names");
 
     auto const it = std::find_if(m_tables->cbegin(), m_tables->cend(),
                                  [&table_name](flex_table_t const &table) {
@@ -1021,7 +1008,7 @@ flex_table_t &output_flex_t::create_flex_table()
     lua_getfield(lua_state(), -1, "schema");
     if (lua_isstring(lua_state(), -1)) {
         std::string const schema = lua_tostring(lua_state(), -1);
-        check_name(schema, "schema");
+        check_identifier(schema, "schema field");
         new_table.set_schema(schema);
     }
     lua_pop(lua_state(), 1);
@@ -1052,7 +1039,7 @@ flex_table_t &output_flex_t::create_flex_table()
     lua_getfield(lua_state(), -1, "data_tablespace");
     if (lua_isstring(lua_state(), -1)) {
         std::string const tablespace = lua_tostring(lua_state(), -1);
-        check_name(tablespace, "data_tablespace");
+        check_identifier(tablespace, "data_tablespace field");
         new_table.set_data_tablespace(tablespace);
     }
     lua_pop(lua_state(), 1);
@@ -1061,7 +1048,7 @@ flex_table_t &output_flex_t::create_flex_table()
     lua_getfield(lua_state(), -1, "index_tablespace");
     if (lua_isstring(lua_state(), -1)) {
         std::string const tablespace = lua_tostring(lua_state(), -1);
-        check_name(tablespace, "index_tablespace");
+        check_identifier(tablespace, "index_tablespace field");
         new_table.set_index_tablespace(tablespace);
     }
     lua_pop(lua_state(), 1);
@@ -1098,7 +1085,7 @@ void output_flex_t::setup_id_columns(flex_table_t *table)
         if (lua_isstring(lua_state(), -1)) {
             std::string const column_name =
                 lua_tolstring(lua_state(), -1, nullptr);
-            check_name(column_name, "column");
+            check_identifier(column_name, "column names");
             auto &column = table->add_column(column_name, "id_type", "");
             column.set_not_null();
         } else if (!lua_isnil(lua_state(), -1)) {
@@ -1111,7 +1098,7 @@ void output_flex_t::setup_id_columns(flex_table_t *table)
 
     std::string const name =
         luaX_get_table_string(lua_state(), "id_column", -2, "The ids field");
-    check_name(name, "column");
+    check_identifier(name, "column names");
 
     auto &column = table->add_column(name, "id_num", "");
     column.set_not_null();
@@ -1143,7 +1130,7 @@ void output_flex_t::setup_flex_table_columns(flex_table_t *table)
                                                        "Column entry", "text");
         char const *const name =
             luaX_get_table_string(lua_state(), "column", -2, "Column entry");
-        check_name(name, "column");
+        check_identifier(name, "column names");
         char const *const sql_type = luaX_get_table_string(
             lua_state(), "sql_type", -3, "Column entry", "");
 

src/pgsql.cpp

@@ -197,6 +197,18 @@ std::string qualified_name(std::string const &schema, std::string const &name)
     return result;
 }
 
+void check_identifier(std::string const &name, char const *in)
+{
+    auto const pos = name.find_first_of("\"',.;$%&/()<>{}=?^*#");
+
+    if (pos == std::string::npos) {
+        return;
+    }
+
+    throw std::runtime_error{
+        "Special characters are not allowed in {}: '{}'."_format(in, name)};
+}
+
 std::map<std::string, std::string>
 get_postgresql_settings(pg_conn_t const &db_connection)
 {

src/pgsql.hpp

@@ -247,6 +247,19 @@ std::string tablespace_clause(std::string const &name);
  */
 std::string qualified_name(std::string const &schema, std::string const &name);
 
+/**
+ * Check that the string confirms to the identifier syntax we accept.
+ * Throws a runtime exception if an invalid character is found.
+ *
+ * Note that PostgreSQL accepts any character in a quoted identifier.
+ * This function checks for some characters that are problematic in the
+ * internal functions that create SQL statements.
+ *
+ * \param name  Identifier to check.
+ * \param in    Name of the identifier. Only used to create a human-readable error.
+ */
+void check_identifier(std::string const &name, char const *in);
+
 struct postgis_version
 {
     int major;

src/table.cpp

@@ -107,11 +107,14 @@ void table_t::start(std::string const &conninfo, std::string const &table_space)
 
         //first with the regular columns
         for (auto const &column : m_columns) {
+            check_identifier(column.name, "column names");
+            check_identifier(column.type_name, "column types");
             sql += R"("{}" {},)"_format(column.name, column.type_name);
         }
 
         //then with the hstore columns
         for (auto const &hcolumn : m_hstore_columns) {
+            check_identifier(hcolumn, "column names");
             sql += R"("{}" hstore,)"_format(hcolumn);
         }