From 61ef09d42e0d37f20e0f39a2916322829ee1cda1 Mon Sep 17 00:00:00 2001 From: Timo Pollmeier Date: Mon, 13 Jul 2020 15:17:02 +0200 Subject: [PATCH 1/3] Quote identifiers in SQL functions using EXECUTE PL/pgSQL functions that build EXECUTE statements with schema, relation or column names passed as parameters should quote them to ensure they are always interpreted as identifiers. --- src/manage_pg.c | 90 +++++++++++++++++++++++++++++++++++++------------ 1 file changed, 68 insertions(+), 22 deletions(-) diff --git a/src/manage_pg.c b/src/manage_pg.c index 9f52aed3b..701e523cc 100644 --- a/src/manage_pg.c +++ b/src/manage_pg.c @@ -332,15 +332,46 @@ manage_create_sql_functions () /* Functions in pl/pgsql. */ + /* Helper function for quoting the individual parts of multi-part + * identifiers like "scap.cpes.id" + */ + sql ("CREATE OR REPLACE FUNCTION quote_ident_split (ident_name text)" + " RETURNS text AS $$" + " DECLARE quoted text := '';" + " BEGIN" + " WITH split AS" + " (SELECT (unnest(string_to_array(ident_name, '.'))) AS part)" + " SELECT string_agg(quote_ident(trim(part, '\"')), '.') FROM split" + " INTO quoted;" + " RETURN quoted;" + " END;" + " $$ LANGUAGE plpgsql;"); + + /* Helper function for quoting comma-separated lists of" + * identifiers like "config.name, config.type" + */ + sql ("CREATE OR REPLACE FUNCTION quote_ident_list (ident_name text)" + " RETURNS text AS $$" + " DECLARE quoted text := '';" + " BEGIN" + " WITH split AS" + " (SELECT (unnest(string_to_array(ident_name, ','))) AS ident)" + " SELECT string_agg(quote_ident_split(trim(ident, ' ')), ', ')" + " FROM split" + " INTO quoted;" + " RETURN quoted;" + " END;" + " $$ LANGUAGE plpgsql;"); + /* Wrapping the "LOCK TABLE ... NOWAIT" like this will prevent * error messages in the PostgreSQL log if the lock is not available. */ sql ("CREATE OR REPLACE FUNCTION try_exclusive_lock (regclass)" " RETURNS integer AS $$" " BEGIN" - " EXECUTE 'LOCK TABLE \"'" - " || $1" - " || '\" IN ACCESS EXCLUSIVE MODE NOWAIT;';" + " EXECUTE 'LOCK TABLE '" + " || quote_ident_split($1::text)" + " || ' IN ACCESS EXCLUSIVE MODE NOWAIT;';" " RETURN 1;" " EXCEPTION WHEN lock_not_available THEN" " RETURN 0;" @@ -420,15 +451,17 @@ manage_create_sql_functions () " WHEN $1 = 'task'" " THEN RETURN (SELECT name FROM tasks WHERE uuid = $2);" " WHEN $3 = " G_STRINGIFY (LOCATION_TABLE) - " THEN EXECUTE 'SELECT name FROM ' || $1 || 's" - " WHERE uuid = $1'" + " THEN EXECUTE 'SELECT name FROM '" + " || quote_ident_split($1 || 's')" + " || ' WHERE uuid = $1'" " INTO execute_name" " USING $2;" " RETURN execute_name;" " WHEN $1 NOT IN ('nvt', 'cpe', 'cve', 'ovaldef', 'cert_bund_adv'," " 'dfn_cert_adv', 'report', 'result', 'user')" - " THEN EXECUTE 'SELECT name FROM ' || $1 || 's_trash" - " WHERE uuid = $1'" + " THEN EXECUTE 'SELECT name FROM '" + " || quote_ident_split ($1 || 's_trash')" + " || ' WHERE uuid = $1'" " INTO execute_name" " USING $2;" " RETURN execute_name;" @@ -650,8 +683,9 @@ manage_create_sql_functions () " IF type = 'user' THEN separator := '_'; END IF;" " candidate := proposed_name || suffix || separator || number::text;" " LOOP" - " EXECUTE 'SELECT count (*) = 0 FROM ' || type || 's" - " WHERE name = $1" + " EXECUTE 'SELECT count (*) = 0 FROM '" + " || quote_ident_split(type || 's')" + " || ' WHERE name = $1" " AND (($2 IS NULL) OR (owner IS NULL) OR (owner = $2))'" " INTO unique_candidate" " USING candidate, owner;" @@ -674,8 +708,9 @@ manage_create_sql_functions () " AND tablename = lower (table_name)" " AND indexname = lower (index_name))" " THEN" - " EXECUTE 'CREATE INDEX ' || index_name" - " || ' ON ' || table_name || ' (' || columns || ');';" + " EXECUTE 'CREATE INDEX ' || quote_ident(index_name)" + " || ' ON ' || quote_ident_split(table_name)" + " || ' (' || quote_ident_list(columns) || ');';" " END IF;" " END;" "$$ LANGUAGE plpgsql;"); @@ -705,27 +740,36 @@ manage_create_sql_functions () " AND ((resource = 0)" /* Super on other_user. */ " OR ((resource_type = ''user'')" - " AND (resource = (SELECT ' || $1 || 's.owner" - " FROM ' || $1 || 's" - " WHERE id = $2)))" + " AND (resource = (SELECT '" + " || quote_ident_split($1 || 's')" + " || '.owner'" + " || ' FROM '" + " || quote_ident_split($1 || 's')" + " || ' WHERE id = $2)))" /* Super on other_user's role. */ " OR ((resource_type = ''role'')" " AND (resource" " IN (SELECT DISTINCT role" " FROM role_users" " WHERE \"user\"" - " = (SELECT ' || $1 || 's.owner" - " FROM ' || $1 || 's" - " WHERE id = $2))))" + " = (SELECT '" + " || quote_ident_split($1 || 's')" + " || '.owner'" + " || ' FROM '" + " || quote_ident_split($1 || 's')" + " || ' WHERE id = $2))))" /* Super on other_user's group. */ " OR ((resource_type = ''group'')" " AND (resource" " IN (SELECT DISTINCT \"group\"" " FROM group_users" " WHERE \"user\"" - " = (SELECT ' || $1 || 's.owner" - " FROM ' || $1 || 's" - " WHERE id = $2)))))" + " = (SELECT '" + " || quote_ident_split($1 || 's')" + " || '.owner'" + " || ' FROM '" + " || quote_ident_split($1 || 's')" + " || ' WHERE id = $2)))))" " AND subject_location = " G_STRINGIFY (LOCATION_TABLE) " AND ((subject_type = ''user''" " AND subject" @@ -809,7 +853,8 @@ manage_create_sql_functions () " END CASE;" " ELSE" " EXECUTE" - " 'SELECT EXISTS (SELECT * FROM ' || $1 || 's" + " 'SELECT EXISTS (SELECT * FROM '" + " || quote_ident_split ($1 || 's') || '" " WHERE id = $2" " AND ((owner IS NULL)" " OR (owner = (SELECT id FROM users" @@ -836,7 +881,8 @@ manage_create_sql_functions () " ret boolean;" " BEGIN" " EXECUTE" - " 'SELECT id FROM ' || $1 || 's WHERE uuid = $2'" + " 'SELECT id FROM ' || quote_ident_split($1 || 's') || '" + " WHERE uuid = $2'" " USING arg_type, arg_uuid" " INTO resource;" " ret = user_owns (arg_type, resource::integer);" From 10465c2dd45375bfd5d51740229580b43dcbf629 Mon Sep 17 00:00:00 2001 From: Timo Pollmeier Date: Mon, 13 Jul 2020 17:19:10 +0200 Subject: [PATCH 2/3] Add SQL quoting fix to CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b06b349aa..3d7429759 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -133,6 +133,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Check number of args to ensure period_offsets is 0 [#1175](https://github.com/greenbone/gvmd/pull/1175) - Fix name handling when creating host assets [#1183](https://github.com/greenbone/gvmd/pull/1183) - Outdated references to "openvassd" have been updated to "openvas" [#1189](https://github.com/greenbone/gvmd/pull/1189) +- Quote identifiers in SQL functions using EXECUTE [#1192](https://github.com/greenbone/gvmd/pull/1192) ### Removed - Remove support for "All SecInfo": removal of "allinfo" for type in get_info [#790](https://github.com/greenbone/gvmd/pull/790) From dd7b7253b3bee61f37fbe69e55ed527b0397f4e0 Mon Sep 17 00:00:00 2001 From: Timo Pollmeier Date: Wed, 15 Jul 2020 14:25:36 +0200 Subject: [PATCH 3/3] Add extra comments for quoting helper functions --- src/manage_pg.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/manage_pg.c b/src/manage_pg.c index f45ceba07..f109775cb 100644 --- a/src/manage_pg.c +++ b/src/manage_pg.c @@ -333,29 +333,36 @@ manage_create_sql_functions () /* Functions in pl/pgsql. */ /* Helper function for quoting the individual parts of multi-part - * identifiers like "scap.cpes.id" + * identifiers like "scap", "cpes" and "id" in "scap.cpes.id" where + * necessary. */ sql ("CREATE OR REPLACE FUNCTION quote_ident_split (ident_name text)" " RETURNS text AS $$" " DECLARE quoted text := '';" " BEGIN" + // Split original dot-separated input into rows " WITH split AS" " (SELECT (unnest(string_to_array(ident_name, '.'))) AS part)" + // For each row trim outer quote marks and quote the result. + // then recombine the rows into a single, dot-separated string again. " SELECT string_agg(quote_ident(trim(part, '\"')), '.') FROM split" " INTO quoted;" " RETURN quoted;" " END;" " $$ LANGUAGE plpgsql;"); - /* Helper function for quoting comma-separated lists of" + /* Helper function for quoting comma-separated lists of * identifiers like "config.name, config.type" */ sql ("CREATE OR REPLACE FUNCTION quote_ident_list (ident_name text)" " RETURNS text AS $$" " DECLARE quoted text := '';" " BEGIN" + // Split original comma-separated input into rows " WITH split AS" " (SELECT (unnest(string_to_array(ident_name, ','))) AS ident)" + // For each row trim outer whitespace and quote the result. + // then recombine the rows into a single, comma-separated string again. " SELECT string_agg(quote_ident_split(trim(ident, ' ')), ', ')" " FROM split" " INTO quoted;"