Merge pull request #3980 from lonvia/security-smells

Improve SQL query assembly

lib-sql/functions/placex_triggers.sql

@@ -672,7 +672,7 @@ CREATE OR REPLACE FUNCTION placex_insert()
   AS $$
 DECLARE
   postcode TEXT;
-  result BOOLEAN;
+  result INT;
   is_area BOOLEAN;
   country_code VARCHAR(2);
   diameter FLOAT;
@@ -777,11 +777,12 @@ BEGIN
 
 
    -- add to tables for special search
-   -- Note: won't work on initial import because the classtype tables
-   -- do not yet exist. It won't hurt either.
   classtable := 'place_classtype_' || NEW.class || '_' || NEW.type;
-  SELECT count(*)>0 FROM pg_tables WHERE tablename = classtable and schemaname = current_schema() INTO result;
+  SELECT count(*) INTO result
-  IF result THEN
+    FROM pg_tables
+    WHERE classtable NOT SIMILAR TO '%\W%'
+          AND tablename = classtable and schemaname = current_schema();
+  IF result > 0 THEN
     EXECUTE 'INSERT INTO ' || classtable::regclass || ' (place_id, centroid) VALUES ($1,$2)' 
     USING NEW.place_id, NEW.centroid;
   END IF;
@@ -1337,6 +1338,7 @@ CREATE OR REPLACE FUNCTION placex_delete()
   AS $$
 DECLARE
   b BOOLEAN;
+  result INT;
   classtable TEXT;
 BEGIN
   -- RAISE WARNING 'placex_delete % %',OLD.osm_type,OLD.osm_id;
@@ -1395,8 +1397,12 @@ BEGIN
 
   -- remove from tables for special search
   classtable := 'place_classtype_' || OLD.class || '_' || OLD.type;
-  SELECT count(*)>0 FROM pg_tables WHERE tablename = classtable and schemaname = current_schema() INTO b;
+  SELECT count(*) INTO result
-  IF b THEN
+    FROM pg_tables
+    WHERE classtable NOT SIMILAR TO '%\W%'
+          AND tablename = classtable and schemaname = current_schema();
+
+  IF result > 0 THEN
     EXECUTE 'DELETE FROM ' || classtable::regclass || ' WHERE place_id = $1' USING OLD.place_id;
   END IF;
 

src/nominatim_db/config.py

@@ -2,7 +2,7 @@
 #
 # This file is part of Nominatim. (https://nominatim.org)
 #
-# Copyright (C) 2025 by the Nominatim developer community.
+# Copyright (C) 2026 by the Nominatim developer community.
 # For a full list of authors see the git log.
 """
 Nominatim configuration accessor.
@@ -12,6 +12,7 @@ import importlib.util
 import logging
 import os
 import sys
+import re
 from pathlib import Path
 import json
 import yaml
@@ -80,6 +81,10 @@ class Configuration:
         self.lib_dir = _LibDirs()
         self._private_plugins: Dict[str, object] = {}
 
+        if re.fullmatch(r'[\w-]+', self.DATABASE_WEBUSER) is None:
+            raise UsageError("Misconfigured DATABASE_WEBUSER. "
+                             "Only alphnumberic characters, - and _ are allowed.")
+
     def set_libdirs(self, **kwargs: StrPath) -> None:
         """ Set paths to library functions and data.
         """

src/nominatim_db/db/sql_preprocessor.py

@@ -2,12 +2,13 @@
 #
 # This file is part of Nominatim. (https://nominatim.org)
 #
-# Copyright (C) 2024 by the Nominatim developer community.
+# Copyright (C) 2026 by the Nominatim developer community.
 # For a full list of authors see the git log.
 """
 Preprocessing of SQL files.
 """
 from typing import Set, Dict, Any, cast
+import re
 
 import jinja2
 
@@ -34,7 +35,9 @@ def _get_tables(conn: Connection) -> Set[str]:
     with conn.cursor() as cur:
         cur.execute("SELECT tablename FROM pg_tables WHERE schemaname = 'public'")
 
-        return set((row[0] for row in list(cur)))
+        # paranoia check: make sure we don't get table names that cause
+        # an SQL injection later
+        return {row[0] for row in list(cur) if re.fullmatch(r'\w+', row[0])}
 
 
 def _get_middle_db_format(conn: Connection, tables: Set[str]) -> str:

src/nominatim_db/tokenizer/icu_tokenizer.py

@@ -2,7 +2,7 @@
 #
 # This file is part of Nominatim. (https://nominatim.org)
 #
-# Copyright (C) 2025 by the Nominatim developer community.
+# Copyright (C) 2026 by the Nominatim developer community.
 # For a full list of authors see the git log.
 """
 Tokenizer implementing normalisation as used before Nominatim 4 but using
@@ -294,13 +294,12 @@ class ICUTokenizer(AbstractTokenizer):
         with connect(self.dsn) as conn:
             drop_tables(conn, 'word')
             with conn.cursor() as cur:
-                cur.execute(f"ALTER TABLE {old} RENAME TO word")
+                cur.execute(pysql.SQL("ALTER TABLE {} RENAME TO word")
-                for idx in ('word_token', 'word_id'):
+                                 .format(pysql.Identifier(old)))
-                    cur.execute(f"""ALTER INDEX idx_{old}_{idx}
+                for idx in ['word_token', 'word_id'] + [n[0] for n in WORD_TYPES]:
-                                      RENAME TO idx_word_{idx}""")
+                    cur.execute(pysql.SQL("ALTER INDEX {} RENAME TO {}")
-                for name, _ in WORD_TYPES:
+                                     .format(pysql.Identifier(f"idx_{old}_{idx}"),
-                    cur.execute(f"""ALTER INDEX idx_{old}_{name}
+                                             pysql.Identifier(f"idx_word_{idx}")))
-                                    RENAME TO idx_word_{name}""")
             conn.commit()
 
 

test/python/config/test_config.py

@@ -2,7 +2,7 @@
 #
 # This file is part of Nominatim. (https://nominatim.org)
 #
-# Copyright (C) 2025 by the Nominatim developer community.
+# Copyright (C) 2026 by the Nominatim developer community.
 # For a full list of authors see the git log.
 """
 Test for loading dotenv configuration.
@@ -68,13 +68,13 @@ def test_prefer_os_environ_over_project_setting(make_config, monkeypatch, tmp_pa
 
 def test_prefer_os_environ_can_unset_project_setting(make_config, monkeypatch, tmp_path):
     envfile = tmp_path / '.env'
-    envfile.write_text('NOMINATIM_DATABASE_WEBUSER=apache\n', encoding='utf-8')
+    envfile.write_text('NOMINATIM_OSM2PGSQL_BINARY=osm2pgsql\n', encoding='utf-8')
 
-    monkeypatch.setenv('NOMINATIM_DATABASE_WEBUSER', '')
+    monkeypatch.setenv('NOMINATIM_OSM2PGSQL_BINARY', '')
 
     config = make_config(tmp_path)
 
-    assert config.DATABASE_WEBUSER == ''
+    assert config.OSM2PGSQL_BINARY == ''
 
 
 def test_get_os_env_add_defaults(make_config, monkeypatch):

test/python/conftest.py

@@ -60,7 +60,7 @@ def temp_db(monkeypatch):
 
     with psycopg.connect(dbname='postgres', autocommit=True) as conn:
         with conn.cursor() as cur:
-            cur.execute('DROP DATABASE IF EXISTS {}'.format(name))
+            cur.execute(pysql.SQL('DROP DATABASE IF EXISTS') + pysql.Identifier(name))
 
 
 @pytest.fixture
@@ -104,7 +104,9 @@ def table_factory(temp_db_conn):
     """
     def mk_table(name, definition='id INT', content=None):
         with psycopg.ClientCursor(temp_db_conn) as cur:
-            cur.execute('CREATE TABLE {} ({})'.format(name, definition))
+            cur.execute(pysql.SQL("CREATE TABLE {} ({})")
+                             .format(pysql.Identifier(name),
+                                     pysql.SQL(definition)))
             if content:
                 sql = pysql.SQL("INSERT INTO {} VALUES ({})")\
                            .format(pysql.Identifier(name),

test/python/tools/test_database_import.py

@@ -2,7 +2,7 @@
 #
 # This file is part of Nominatim. (https://nominatim.org)
 #
-# Copyright (C) 2025 by the Nominatim developer community.
+# Copyright (C) 2026 by the Nominatim developer community.
 # For a full list of authors see the git log.
 """
 Tests for functions to import a new database.
@@ -25,12 +25,14 @@ class TestDatabaseSetup:
     def setup_nonexistant_db(self):
         with psycopg.connect(dbname='postgres', autocommit=True) as conn:
             with conn.cursor() as cur:
-                cur.execute(f'DROP DATABASE IF EXISTS {self.DBNAME}')
+                cur.execute(pysql.SQL('DROP DATABASE IF EXISTS ')
+                            + pysql.Identifier(self.DBNAME))
 
             yield True
 
             with conn.cursor() as cur:
-                cur.execute(f'DROP DATABASE IF EXISTS {self.DBNAME}')
+                cur.execute(pysql.SQL('DROP DATABASE IF EXISTS ')
+                            + pysql.Identifier(self.DBNAME))
 
     @pytest.fixture
     def cursor(self):
@@ -62,7 +64,7 @@ class TestDatabaseSetup:
     def test_create_db_missing_ro_user(self):
         with pytest.raises(UsageError, match='Missing read-only user.'):
             database_import.setup_database_skeleton(f'dbname={self.DBNAME}',
-                                                    rouser='sdfwkjkjgdugu2;jgsafkljas;')
+                                                    rouser='sdfwkjkjgdugu2jgsafkljas')
 
     def test_setup_extensions_old_postgis(self, monkeypatch):
         monkeypatch.setattr(database_import, 'POSTGIS_REQUIRED_VERSION', (50, 50))