From 69f484d092a3e63f08a356d9696d0fa4c6f9afb1 Mon Sep 17 00:00:00 2001 From: Timo Pollmeier Date: Thu, 4 Feb 2021 10:36:10 +0100 Subject: [PATCH 1/2] Also create owner WITH clause for single resources The function init_get_iterator2_with will now also use acl_where_owned to generate a WITH clause in case permissions_subject is used in a subquery. The init_result_get_iterator_severity had to be adjusted to not generate the same WITH clause twice. This change fixes an error getting details pages for types use the WITH clauses for permission checks. --- src/manage_acl.c | 86 ++++++++++++++++++++++++------------------------ src/manage_sql.c | 35 +++++++++----------- 2 files changed, 58 insertions(+), 63 deletions(-) diff --git a/src/manage_acl.c b/src/manage_acl.c index 01f036505..091fc83b2 100644 --- a/src/manage_acl.c +++ b/src/manage_acl.c @@ -1011,50 +1011,7 @@ acl_where_owned_user (const char *user_id, const char *user_sql, guint index; if (with) - *with = NULL; - - if (owned == 0) - return g_strdup (" t ()"); - - permission_or = g_string_new (""); - index = 0; - if (permissions == NULL || permissions->len == 0) { - /* Treat filters with no permissions keyword as "any". */ - permission_or = g_string_new ("t ()"); - index = 1; - } - else if (permissions) - for (; index < permissions->len; index++) - { - gchar *permission, *quoted; - permission = (gchar*) g_ptr_array_index (permissions, index); - if (strcasecmp (permission, "any") == 0) - { - g_string_free (permission_or, TRUE); - permission_or = g_string_new ("t ()"); - index = 1; - break; - } - quoted = sql_quote (permission); - if (index == 0) - g_string_append_printf (permission_or, "name = '%s'", quoted); - else - g_string_append_printf (permission_or, " OR name = '%s'", - quoted); - g_free (quoted); - } - - table_trash = get->trash && strcasecmp (type, "task"); - if (resource || (user_id == NULL)) - owned_clause - = g_strdup (" (t ())"); - else if (with) - { - gchar *permission_clause; - - /* Caller supports WITH clause. */ - *with = g_strdup_printf ("WITH permissions_subject" " AS (SELECT * FROM permissions" @@ -1098,6 +1055,49 @@ acl_where_owned_user (const char *user_id, const char *user_sql, user_sql, user_sql, user_sql); + } + + if (owned == 0) + return g_strdup (" t ()"); + + permission_or = g_string_new (""); + index = 0; + if (permissions == NULL || permissions->len == 0) + { + /* Treat filters with no permissions keyword as "any". */ + permission_or = g_string_new ("t ()"); + index = 1; + } + else if (permissions) + for (; index < permissions->len; index++) + { + gchar *permission, *quoted; + permission = (gchar*) g_ptr_array_index (permissions, index); + if (strcasecmp (permission, "any") == 0) + { + g_string_free (permission_or, TRUE); + permission_or = g_string_new ("t ()"); + index = 1; + break; + } + quoted = sql_quote (permission); + if (index == 0) + g_string_append_printf (permission_or, "name = '%s'", quoted); + else + g_string_append_printf (permission_or, " OR name = '%s'", + quoted); + g_free (quoted); + } + + table_trash = get->trash && strcasecmp (type, "task"); + if (resource || (user_id == NULL)) + owned_clause + = g_strdup (" (t ())"); + else if (with) + { + gchar *permission_clause; + + /* Caller supports WITH clause. */ permission_clause = NULL; if (user_id && index) diff --git a/src/manage_sql.c b/src/manage_sql.c index ed86d9cf9..cd55de7f0 100644 --- a/src/manage_sql.c +++ b/src/manage_sql.c @@ -4857,11 +4857,13 @@ init_get_iterator2_with (iterator_t* iterator, const char *type, with_clause = NULL; - if (resource) - /* Ownership test is done above by find function. */ - owned_clause = g_strdup (" t ()"); - else if (assume_permitted) - owned_clause = g_strdup (" t ()"); + if (resource || assume_permitted) + /* Ownership test of single resources is done above by find function + * but acl_where_owned has to be called to generate WITH clause + * in case subqueries depend on it. + */ + owned_clause = acl_where_owned (type, get, 0, owner_filter, resource, + permissions, &with_clause); else owned_clause = acl_where_owned (type, get, owned, owner_filter, resource, permissions, &with_clause); @@ -22202,7 +22204,7 @@ init_result_get_iterator_severity (iterator_t* iterator, const get_data_t *get, gchar *filter; int autofp, apply_overrides, dynamic_severity; gchar *extra_tables, *extra_where, *extra_where_single; - gchar *owned_clause, *with_clause, *with_clauses; + gchar *owned_clause, *with_clause; char *user_id; assert (report); @@ -22360,11 +22362,12 @@ init_result_get_iterator_severity (iterator_t* iterator, const get_data_t *get, user_id = sql_string ("SELECT id FROM users WHERE uuid = '%s';", current_credentials.uuid); - owned_clause = acl_where_owned_for_get ("override", user_id, &with_clause); + // Do not get ACL with_clause as it will be added by init_get_iterator2_with. + owned_clause = acl_where_owned_for_get ("override", user_id, NULL); free (user_id); - with_clauses = g_strdup_printf - ("%s%s" - " valid_overrides" + + with_clause = g_strdup_printf + (" valid_overrides" " AS (SELECT result_nvt, hosts, new_severity, port," " severity, result" " FROM overrides" @@ -22382,17 +22385,9 @@ init_result_get_iterator_severity (iterator_t* iterator, const get_data_t *get, " ORDER BY result DESC, task DESC, port DESC, severity ASC," " creation_time DESC)" " ", - with_clause - /* Skip the leading "WITH" because init_get.. - * below will add it. A bit of a hack, but - * it's the only place that needs this. */ - ? with_clause + 4 - : "", - with_clause ? "," : "", owned_clause, report, report); - g_free (with_clause); g_free (owned_clause); table_order_if_sort_not_specified = 1; @@ -22413,11 +22408,11 @@ init_result_get_iterator_severity (iterator_t* iterator, const get_data_t *get, TRUE, report ? TRUE : FALSE, extra_order, - with_clauses, + with_clause, 1); table_order_if_sort_not_specified = 0; column_array_free (filterable_columns); - g_free (with_clauses); + g_free (with_clause); g_free (extra_tables); g_free (extra_where); g_free (extra_where_single); From 8e746efd2807c2df24ac1fb80242dda8bcfbaf1a Mon Sep 17 00:00:00 2001 From: Timo Pollmeier Date: Thu, 4 Feb 2021 10:48:30 +0100 Subject: [PATCH 2/2] Add CHANGELOG entry for WITH clause fix --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 34c80ad0b..a83dcc658 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Changed ### Fixed +- Also create owner WITH clause for single resources [#1406](https://github.com/greenbone/gvmd/pull/1406) ### Removed