From d74dc2ecc82956a74cdd16a511f3bb33f7cc0ebb Mon Sep 17 00:00:00 2001 From: Timo Pollmeier Date: Wed, 30 Jun 2021 17:22:25 +0200 Subject: [PATCH 1/8] Fix sending prefs for whole, growing VT families If a VT family is growing and all VTs are selected, vt_single elements are used instead of a vt_group one again so preferences can be set for the VTs. --- src/manage.c | 20 +++----------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/src/manage.c b/src/manage.c index e89bdf0b8..d0ff6a219 100644 --- a/src/manage.c +++ b/src/manage.c @@ -2483,7 +2483,7 @@ launch_osp_openvas_task (task_t task, target_t target, const char *scan_id, gchar *clean_hosts, *clean_exclude_hosts, *clean_finished_hosts_str; int alive_test, reverse_lookup_only, reverse_lookup_unify; osp_target_t *osp_target; - GSList *osp_targets, *vts, *vt_groups; + GSList *osp_targets, *vts; GHashTable *vts_hash_table; osp_credential_t *ssh_credential, *smb_credential, *esxi_credential; osp_credential_t *snmp_credential; @@ -2624,7 +2624,6 @@ launch_osp_openvas_task (task_t task, target_t target, const char *scan_id, /* Setup vulnerability tests (without preferences) */ vts = NULL; - vt_groups = NULL; vts_hash_table = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, /* Value is freed in vts list. */ @@ -2634,18 +2633,7 @@ launch_osp_openvas_task (task_t task, target_t target, const char *scan_id, while (next (&families)) { const char *family = family_iterator_name (&families); - if (family && config_family_entire_and_growing (config, family)) - { - gchar *filter; - osp_vt_group_t *vt_group; - - filter = g_strdup_printf ("family=%s", family); - vt_group = osp_vt_group_new (filter); - g_free (filter); - - vt_groups = g_slist_prepend (vt_groups, vt_group); - } - else if (family) + if (family) { iterator_t nvts; init_nvt_iterator (&nvts, 0, config, family, NULL, 1, NULL); @@ -2723,13 +2711,12 @@ launch_osp_openvas_task (task_t task, target_t target, const char *scan_id, g_slist_free_full (osp_targets, (GDestroyNotify) osp_target_free); // Credentials are freed with target g_slist_free_full (vts, (GDestroyNotify) osp_vt_single_free); - g_slist_free_full (vt_groups, (GDestroyNotify) osp_vt_group_free); g_hash_table_destroy (scanner_options); return -1; } start_scan_opts.targets = osp_targets; - start_scan_opts.vt_groups = vt_groups; + start_scan_opts.vt_groups = NULL; start_scan_opts.vts = vts; start_scan_opts.scanner_params = scanner_options; start_scan_opts.scan_id = scan_id; @@ -2742,7 +2729,6 @@ launch_osp_openvas_task (task_t task, target_t target, const char *scan_id, g_slist_free_full (osp_targets, (GDestroyNotify) osp_target_free); // Credentials are freed with target g_slist_free_full (vts, (GDestroyNotify) osp_vt_single_free); - g_slist_free_full (vt_groups, (GDestroyNotify) osp_vt_group_free); g_hash_table_destroy (scanner_options); return ret; } From 42999808356c79fd92cc7ce6f34cfe044467f1e4 Mon Sep 17 00:00:00 2001 From: Timo Pollmeier Date: Wed, 30 Jun 2021 17:37:09 +0200 Subject: [PATCH 2/8] Add CHANGELOG entry for VT preferences fix --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f28b10fa..af6619d60 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Deprecated ### Removed ### Fixed +- Fix sending prefs for whole, growing VT families [#1603](https://github.com/greenbone/gvmd/pull/1603) [Unreleased]: https://github.com/greenbone/gvmd/compare/v21.4.2...gvmd-21.04 From 51367b505399da0f6e101d1aaf6820f23e0ecaee Mon Sep 17 00:00:00 2001 From: Timo Pollmeier Date: Tue, 6 Jul 2021 12:25:32 +0200 Subject: [PATCH 3/8] Fix memory errors in modify_permission The string "subject_where_old" was not freed when returning if check_permission_args failed. The string "name" was still used after being freed. --- src/manage_sql.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/manage_sql.c b/src/manage_sql.c index 5beddfbc0..cb156235f 100644 --- a/src/manage_sql.c +++ b/src/manage_sql.c @@ -43631,6 +43631,7 @@ modify_permission (const char *permission_id, const char *name_arg, free (new_resource_id); free (existing_subject_type); free (new_subject_id); + g_free (subject_where_old); sql_rollback (); return ret; } @@ -43683,7 +43684,6 @@ modify_permission (const char *permission_id, const char *name_arg, || (resource_id == NULL)); quoted_name = sql_quote (name); - g_free (name); sql ("UPDATE permissions SET" " name = '%s'," @@ -43767,6 +43767,7 @@ modify_permission (const char *permission_id, const char *name_arg, free (new_resource_id); free (existing_subject_type); free (new_subject_id); + g_free (name); free (old_name); free (old_resource_type); g_free (subject_where); From fb06bc01450cf251d88da70ddf47958ffe77c1d7 Mon Sep 17 00:00:00 2001 From: Timo Pollmeier Date: Tue, 6 Jul 2021 12:28:37 +0200 Subject: [PATCH 4/8] Add CHANGELOG entry for modify_permission fix --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f28b10fa..98c5a44d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -119,6 +119,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Deprecated ### Removed ### Fixed +- Fix memory errors in modify_permission [#1613](https://github.com/greenbone/gvmd/pull/1613) [Unreleased]: https://github.com/greenbone/gvmd/compare/v20.8.2...gvmd-20.08 From 6d1797a866f89c7efe19edb04c5cc99edb5002c7 Mon Sep 17 00:00:00 2001 From: Timo Pollmeier Date: Tue, 6 Jul 2021 16:32:05 +0200 Subject: [PATCH 5/8] Use less report cache SQL when adding results When multiple results are added when handling an OSP get_scan response or a host in a CVE scan, only one SQL statement each is run to update the report and owner of the results and to update the end times of the report_counts cache of the report. This addresses AP-1495. --- src/manage.c | 7 +++++- src/manage.h | 3 +++ src/manage_sql.c | 64 +++++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 70 insertions(+), 4 deletions(-) diff --git a/src/manage.c b/src/manage.c index e89bdf0b8..2c5266b03 100644 --- a/src/manage.c +++ b/src/manage.c @@ -3061,9 +3061,11 @@ cve_scan_host (task_t task, report_t report, gvm_host_t *gvm_host) { iterator_t prognosis; int prognosis_report_host, start_time; + GArray *results; /* Add report_host with prognosis results and host details. */ + results = g_array_new (TRUE, TRUE, sizeof (result_t)); start_time = time (NULL); prognosis_report_host = 0; init_host_prognosis_iterator (&prognosis, report_host); @@ -3136,12 +3138,15 @@ cve_scan_host (task_t task, report_t report, gvm_host_t *gvm_host) result = make_cve_result (task, ip, cve, severity, desc); g_free (desc); - report_add_result (report, result); + g_array_append_val (results, result); g_string_free (locations, TRUE); } cleanup_iterator (&prognosis); + report_add_results_array (report, results); + g_array_free (results, TRUE); + if (prognosis_report_host) { /* Complete the report_host. */ diff --git a/src/manage.h b/src/manage.h index ec9770c9f..1b3d99f77 100644 --- a/src/manage.h +++ b/src/manage.h @@ -1256,6 +1256,9 @@ create_report (array_t*, const char *, const char *, const char *, const char *, void report_add_result (report_t, result_t); +void +report_add_results_array (report_t, GArray *); + char* report_uuid (report_t); diff --git a/src/manage_sql.c b/src/manage_sql.c index cb156235f..834166a1e 100644 --- a/src/manage_sql.c +++ b/src/manage_sql.c @@ -21004,6 +21004,57 @@ report_add_result (report_t report, result_t result) report, report); } +/** + * @brief Add results from an array to a report. + */ +void +report_add_results_array (report_t report, GArray *results) +{ + GString *array_sql; + int index; + + if (report == 0 || results == NULL || results->len == 0) + return; + + array_sql = g_string_new ("("); + for (index = 0; index < results->len; index++) + { + result_t result; + result = g_array_index (results, result_t, index); + + if (index) + g_string_append (array_sql, ", "); + g_string_append_printf (array_sql, "%llu", result); + } + g_string_append_c (array_sql, ')'); + + sql ("UPDATE results SET report = %llu," + " owner = (SELECT reports.owner" + " FROM reports WHERE id = %llu)" + " WHERE id IN %s;", + report, report, array_sql->str); + + for (index = 0; index < results->len; index++) + { + result_t result; + result = g_array_index (results, result_t, index); + + // TODO: Use array to insert multiple results at once + report_add_result_for_buffer (report, result); + } + + sql ("UPDATE report_counts" + " SET end_time = (SELECT coalesce(min(overrides.end_time), 0)" + " FROM overrides, results" + " WHERE overrides.nvt = results.nvt" + " AND results.report = %llu" + " AND overrides.end_time >= m_now ())" + " WHERE report = %llu AND override = 1;", + report, report); + + g_string_free (array_sql, TRUE); +} + /** * @brief Filter columns for report iterator. */ @@ -28742,6 +28793,7 @@ parse_osp_report (task_t task, report_t report, const char *report_xml) char *defs_file = NULL; time_t start_time, end_time; gboolean has_results = FALSE; + GArray *results_array; assert (task); assert (report); @@ -28755,6 +28807,7 @@ parse_osp_report (task_t task, report_t report, const char *report_xml) sql_begin_immediate (); /* Set the report's start and end times. */ + results_array = g_array_new (TRUE, TRUE, sizeof (result_t)); start_time = 0; str = entity_attribute (entity, "start_time"); if (str) @@ -28876,7 +28929,7 @@ parse_osp_report (task_t task, report_t report, const char *report_xml) severity_str ?: severity, qod_int, path); - report_add_result (report, result); + g_array_append_val (results_array, result); } g_free (nvt_id); g_free (desc); @@ -28885,11 +28938,16 @@ parse_osp_report (task_t task, report_t report, const char *report_xml) } if (has_results) - sql ("UPDATE reports SET modification_time = m_now() WHERE id = %llu;", - report); + { + sql ("UPDATE reports SET modification_time = m_now() WHERE id = %llu;", + report); + report_add_results_array (report, results_array); + } + end_parse_osp_report: sql_commit (); + g_array_free (results_array, TRUE); g_free (defs_file); free_entity (entity); } From fde56664cb6817be2d42d092284a525037ddc2b3 Mon Sep 17 00:00:00 2001 From: Timo Pollmeier Date: Tue, 6 Jul 2021 16:37:20 +0200 Subject: [PATCH 6/8] Add CHANGELOG entry for report cache SQL reduction --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d2d9c8bc..bab1f22b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## [21.4.3] (Unreleased) ### Added ### Changed +- Use less report cache SQL when adding results [#1618](https://github.com/greenbone/gvmd/pull/1618) + ### Deprecated ### Removed ### Fixed From 8fe759662f97c32bcb42382d2aeb59615f88fc13 Mon Sep 17 00:00:00 2001 From: Timo Pollmeier Date: Tue, 6 Jul 2021 16:40:00 +0200 Subject: [PATCH 7/8] Add missing param doc for report_add_results_array --- src/manage_sql.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/manage_sql.c b/src/manage_sql.c index 834166a1e..4963c94e5 100644 --- a/src/manage_sql.c +++ b/src/manage_sql.c @@ -21006,6 +21006,9 @@ report_add_result (report_t report, result_t result) /** * @brief Add results from an array to a report. + * + * @param[in] report The report to add the results to. + * @param[in] results GArray containing the row ids of the results to add. */ void report_add_results_array (report_t report, GArray *results) From a3516ce24eae6a6f91f0c05399d7f1678c977886 Mon Sep 17 00:00:00 2001 From: Timo Pollmeier Date: Wed, 7 Jul 2021 12:17:18 +0200 Subject: [PATCH 8/8] Remove TODO comment in report_add_results_array --- src/manage_sql.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/manage_sql.c b/src/manage_sql.c index 4963c94e5..4ebbb7063 100644 --- a/src/manage_sql.c +++ b/src/manage_sql.c @@ -21042,7 +21042,6 @@ report_add_results_array (report_t report, GArray *results) result_t result; result = g_array_index (results, result_t, index); - // TODO: Use array to insert multiple results at once report_add_result_for_buffer (report, result); }