From f0faee859d27637e70a3e0111e35febd9fb772dd Mon Sep 17 00:00:00 2001 From: Evgeny Kolesnikov Date: Tue, 3 Nov 2020 19:58:23 +0100 Subject: [PATCH] Fix issues uncovered by Coverity Scan, fix field names handling in yamlfilecontent probe --- openscap-1.3.5-coverity1-PR_1617.patch | 162 ++++++++++++++++++ openscap-1.3.5-coverity2-PR_1620.patch | 147 ++++++++++++++++ ...nscap-1.3.5-plug-memory-leak-PR_1616.patch | 0 ...lfilecontent-fix-field-names-PR_1619.patch | 67 ++++++++ openscap.spec | 11 +- 5 files changed, 385 insertions(+), 2 deletions(-) create mode 100644 openscap-1.3.5-coverity1-PR_1617.patch create mode 100644 openscap-1.3.5-coverity2-PR_1620.patch rename openscap-1.3.4-plug-memory-leak.patch => openscap-1.3.5-plug-memory-leak-PR_1616.patch (100%) create mode 100644 openscap-1.3.5-yamlfilecontent-fix-field-names-PR_1619.patch diff --git a/openscap-1.3.5-coverity1-PR_1617.patch b/openscap-1.3.5-coverity1-PR_1617.patch new file mode 100644 index 0000000..ea7edcb --- /dev/null +++ b/openscap-1.3.5-coverity1-PR_1617.patch @@ -0,0 +1,162 @@ +From 0311ac9d8368acd5baac8b7fc6f753bd895ea3fc Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Jan=20=C4=8Cern=C3=BD?= +Date: Tue, 6 Oct 2020 13:32:19 +0200 +Subject: [PATCH 1/2] Fix Coverity warnings + +Addressing multiple Coverity defects similar to this one: +Defect type: CHECKED_RETURN +check_return: Calling "curl_easy_setopt(curl, _curl_opt, _curl_trace)" +without checking return value. This library function may fail and return +an error code. +--- + src/common/oscap_acquire.c | 65 +++++++++++++++++++++++++++++++------- + 1 file changed, 53 insertions(+), 12 deletions(-) + +diff --git a/src/common/oscap_acquire.c b/src/common/oscap_acquire.c +index 666f4f5c9..34a92fa19 100644 +--- a/src/common/oscap_acquire.c ++++ b/src/common/oscap_acquire.c +@@ -326,18 +326,59 @@ char* oscap_acquire_url_download(const char *url, size_t* memory_size) + return NULL; + } + +- struct oscap_buffer* buffer = oscap_buffer_new(); +- +- curl_easy_setopt(curl, CURLOPT_URL, url); +- curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_to_memory_callback); +- curl_easy_setopt(curl, CURLOPT_WRITEDATA, buffer); +- curl_easy_setopt(curl, CURLOPT_ACCEPT_ENCODING, ""); +- curl_easy_setopt(curl, CURLOPT_TRANSFER_ENCODING, true); +- curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, true); +- curl_easy_setopt(curl, CURLOPT_VERBOSE, true); +- curl_easy_setopt(curl, CURLOPT_DEBUGFUNCTION, _curl_trace); +- +- CURLcode res = curl_easy_perform(curl); ++ CURLcode res; ++ ++ res = curl_easy_setopt(curl, CURLOPT_URL, url); ++ if (res != 0) { ++ oscap_seterr(OSCAP_EFAMILY_NET, "Failed to set CURLOPT_URL to '%s': %s", url, curl_easy_strerror(res)); ++ return NULL; ++ } ++ ++ res = curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_to_memory_callback); ++ if (res != 0) { ++ oscap_seterr(OSCAP_EFAMILY_NET, "Failed to set CURLOPT_WRITEFUNCTION to write_to_memory_callback: %s", curl_easy_strerror(res)); ++ return NULL; ++ } ++ ++ res = curl_easy_setopt(curl, CURLOPT_ACCEPT_ENCODING, ""); ++ if (res != 0) { ++ oscap_seterr(OSCAP_EFAMILY_NET, "Failed to set CURLOPT_ACCEPT_ENCODING to an empty string: %s", curl_easy_strerror(res)); ++ return NULL; ++ } ++ ++ res = curl_easy_setopt(curl, CURLOPT_TRANSFER_ENCODING, true); ++ if (res != 0) { ++ oscap_seterr(OSCAP_EFAMILY_NET, "Failed to set CURLOPT_TRANSFER_ENCODING to true: %s", curl_easy_strerror(res)); ++ return NULL; ++ } ++ ++ res = curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, true); ++ if (res != 0) { ++ oscap_seterr(OSCAP_EFAMILY_NET, "Failed to set CURLOPT_FOLLOWLOCATION to true: %s", curl_easy_strerror(res)); ++ return NULL; ++ } ++ ++ res = curl_easy_setopt(curl, CURLOPT_VERBOSE, true); ++ if (res != 0) { ++ oscap_seterr(OSCAP_EFAMILY_NET, "Failed to set CURLOPT_VERBOSE to true: %s", curl_easy_strerror(res)); ++ return NULL; ++ } ++ ++ res = curl_easy_setopt(curl, CURLOPT_DEBUGFUNCTION, _curl_trace); ++ if (res != 0) { ++ oscap_seterr(OSCAP_EFAMILY_NET, "Failed to set CURLOPT_DEBUGFUNCTION to _curl_trace: %s", curl_easy_strerror(res)); ++ return NULL; ++ } ++ ++ struct oscap_buffer *buffer = oscap_buffer_new(); ++ res = curl_easy_setopt(curl, CURLOPT_WRITEDATA, buffer); ++ if (res != 0) { ++ oscap_seterr(OSCAP_EFAMILY_NET, "Failed to set CURLOPT_WRITEDATA as buffer: %s", curl_easy_strerror(res)); ++ oscap_buffer_free(buffer); ++ return NULL; ++ } ++ ++ res = curl_easy_perform(curl); + curl_easy_cleanup(curl); + + if (res != 0) { + +From 34af1348b6ff6e4710aeb6e383b1a50c4751c16e Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Jan=20=C4=8Cern=C3=BD?= +Date: Mon, 26 Oct 2020 11:12:04 +0100 +Subject: [PATCH 2/2] Add curl_easy_cleanup everywhere + +--- + src/common/oscap_acquire.c | 8 ++++++++ + 1 file changed, 8 insertions(+) + +diff --git a/src/common/oscap_acquire.c b/src/common/oscap_acquire.c +index 34a92fa19..cd9bfc36f 100644 +--- a/src/common/oscap_acquire.c ++++ b/src/common/oscap_acquire.c +@@ -330,42 +330,49 @@ char* oscap_acquire_url_download(const char *url, size_t* memory_size) + + res = curl_easy_setopt(curl, CURLOPT_URL, url); + if (res != 0) { ++ curl_easy_cleanup(curl); + oscap_seterr(OSCAP_EFAMILY_NET, "Failed to set CURLOPT_URL to '%s': %s", url, curl_easy_strerror(res)); + return NULL; + } + + res = curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_to_memory_callback); + if (res != 0) { ++ curl_easy_cleanup(curl); + oscap_seterr(OSCAP_EFAMILY_NET, "Failed to set CURLOPT_WRITEFUNCTION to write_to_memory_callback: %s", curl_easy_strerror(res)); + return NULL; + } + + res = curl_easy_setopt(curl, CURLOPT_ACCEPT_ENCODING, ""); + if (res != 0) { ++ curl_easy_cleanup(curl); + oscap_seterr(OSCAP_EFAMILY_NET, "Failed to set CURLOPT_ACCEPT_ENCODING to an empty string: %s", curl_easy_strerror(res)); + return NULL; + } + + res = curl_easy_setopt(curl, CURLOPT_TRANSFER_ENCODING, true); + if (res != 0) { ++ curl_easy_cleanup(curl); + oscap_seterr(OSCAP_EFAMILY_NET, "Failed to set CURLOPT_TRANSFER_ENCODING to true: %s", curl_easy_strerror(res)); + return NULL; + } + + res = curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, true); + if (res != 0) { ++ curl_easy_cleanup(curl); + oscap_seterr(OSCAP_EFAMILY_NET, "Failed to set CURLOPT_FOLLOWLOCATION to true: %s", curl_easy_strerror(res)); + return NULL; + } + + res = curl_easy_setopt(curl, CURLOPT_VERBOSE, true); + if (res != 0) { ++ curl_easy_cleanup(curl); + oscap_seterr(OSCAP_EFAMILY_NET, "Failed to set CURLOPT_VERBOSE to true: %s", curl_easy_strerror(res)); + return NULL; + } + + res = curl_easy_setopt(curl, CURLOPT_DEBUGFUNCTION, _curl_trace); + if (res != 0) { ++ curl_easy_cleanup(curl); + oscap_seterr(OSCAP_EFAMILY_NET, "Failed to set CURLOPT_DEBUGFUNCTION to _curl_trace: %s", curl_easy_strerror(res)); + return NULL; + } +@@ -373,6 +380,7 @@ char* oscap_acquire_url_download(const char *url, size_t* memory_size) + struct oscap_buffer *buffer = oscap_buffer_new(); + res = curl_easy_setopt(curl, CURLOPT_WRITEDATA, buffer); + if (res != 0) { ++ curl_easy_cleanup(curl); + oscap_seterr(OSCAP_EFAMILY_NET, "Failed to set CURLOPT_WRITEDATA as buffer: %s", curl_easy_strerror(res)); + oscap_buffer_free(buffer); + return NULL; diff --git a/openscap-1.3.5-coverity2-PR_1620.patch b/openscap-1.3.5-coverity2-PR_1620.patch new file mode 100644 index 0000000..404ff9c --- /dev/null +++ b/openscap-1.3.5-coverity2-PR_1620.patch @@ -0,0 +1,147 @@ +From 538c70780b49a36a4d2420ef93b87b78817dc14c Mon Sep 17 00:00:00 2001 +From: Evgeny Kolesnikov +Date: Mon, 26 Oct 2020 08:31:53 +0100 +Subject: [PATCH] Covscan fixes + +--- + src/OVAL/probes/fsdev.c | 2 +- + src/OVAL/probes/independent/yamlfilecontent_probe.c | 5 +++-- + src/OVAL/probes/unix/fileextendedattribute_probe.c | 2 +- + src/OVAL/probes/unix/linux/partition_probe.c | 2 +- + src/OVAL/probes/unix/xinetd_probe.c | 7 ++++++- + src/XCCDF/xccdf_session.c | 4 ++-- + utils/oscap-tool.c | 6 +++++- + utils/oscap-xccdf.c | 3 +-- + 8 files changed, 20 insertions(+), 11 deletions(-) + +diff --git a/src/OVAL/probes/fsdev.c b/src/OVAL/probes/fsdev.c +index b2b984441..c82ab620b 100644 +--- a/src/OVAL/probes/fsdev.c ++++ b/src/OVAL/probes/fsdev.c +@@ -219,7 +219,7 @@ static fsdev_t *__fsdev_init(fsdev_t *lfs) + endmntent(fp); + + void *new_ids = realloc(lfs->ids, sizeof(dev_t) * i); +- if (new_ids == NULL) { ++ if (new_ids == NULL && i > 0) { + e = errno; + free(lfs->ids); + free(lfs); +diff --git a/src/OVAL/probes/independent/yamlfilecontent_probe.c b/src/OVAL/probes/independent/yamlfilecontent_probe.c +index 6f18abf83..e7e6cb3f5 100644 +--- a/src/OVAL/probes/independent/yamlfilecontent_probe.c ++++ b/src/OVAL/probes/independent/yamlfilecontent_probe.c +@@ -216,12 +216,13 @@ static int yaml_path_query(const char *filepath, const char *yaml_path_cstr, str + result_error("YAML parser error: %s", parser.problem); + goto cleanup; + } ++ ++ event_type = event.type; ++ + if (yaml_path_filter_event(yaml_path, &parser, &event) == YAML_PATH_FILTER_RESULT_OUT) { + goto next; + } + +- event_type = event.type; +- + if (sequence) { + if (event_type == YAML_SEQUENCE_END_EVENT) { + sequence = false; +diff --git a/src/OVAL/probes/unix/fileextendedattribute_probe.c b/src/OVAL/probes/unix/fileextendedattribute_probe.c +index b442ea540..ee853886a 100644 +--- a/src/OVAL/probes/unix/fileextendedattribute_probe.c ++++ b/src/OVAL/probes/unix/fileextendedattribute_probe.c +@@ -298,7 +298,7 @@ static int file_cb(const char *prefix, const char *p, const char *f, void *ptr, + + // Allocate buffer, '+1' is for trailing '\0' + void *new_xattr_val = realloc(xattr_val, sizeof(char) * (xattr_vallen + 1)); +- if (xattr_val == NULL) { ++ if (new_xattr_val == NULL) { + dE("Failed to allocate memory for xattr_val"); + free(xattr_val); + goto exit; +diff --git a/src/OVAL/probes/unix/linux/partition_probe.c b/src/OVAL/probes/unix/linux/partition_probe.c +index a74c0323a..adb244b04 100644 +--- a/src/OVAL/probes/unix/linux/partition_probe.c ++++ b/src/OVAL/probes/unix/linux/partition_probe.c +@@ -207,7 +207,7 @@ static int collect_item(probe_ctx *ctx, oval_schema_version_t over, struct mnten + mnt_ocnt = add_mnt_opt(&mnt_opts, mnt_ocnt, "move"); + } + +- dD("mnt_ocnt = %d, mnt_opts[mnt_ocnt]=%p", mnt_ocnt, mnt_opts[mnt_ocnt]); ++ dD("mnt_ocnt = %d, mnt_opts[mnt_ocnt]=%p", mnt_ocnt, mnt_opts == NULL ? NULL : mnt_opts[mnt_ocnt]); + + /* + * "Correct" the type (this won't be (hopefully) needed in a later version +diff --git a/src/OVAL/probes/unix/xinetd_probe.c b/src/OVAL/probes/unix/xinetd_probe.c +index 75b12f95b..d61c7d547 100644 +--- a/src/OVAL/probes/unix/xinetd_probe.c ++++ b/src/OVAL/probes/unix/xinetd_probe.c +@@ -566,7 +566,12 @@ static int xiconf_add_cfile(xiconf_t *xiconf, const char *path, int depth) + } + + xifile->depth = depth; +- xiconf->cfile = realloc(xiconf->cfile, sizeof(xiconf_file_t *) * ++xiconf->count); ++ void *cfile = realloc(xiconf->cfile, sizeof(xiconf_file_t *) * ++xiconf->count); ++ if (cfile == NULL) { ++ dE("Failed re-allocate memory for cfile"); ++ return (-1); ++ } ++ xiconf->cfile = cfile; + xiconf->cfile[xiconf->count - 1] = xifile; + + dD("Added new file to the cfile queue: %s; fi=%zu", path, xiconf->count - 1); +diff --git a/src/XCCDF/xccdf_session.c b/src/XCCDF/xccdf_session.c +index 8bd394e2f..f1b837959 100644 +--- a/src/XCCDF/xccdf_session.c ++++ b/src/XCCDF/xccdf_session.c +@@ -286,9 +286,9 @@ static struct oscap_source *xccdf_session_extract_arf_source(struct xccdf_sessio + } + struct tm *tm_mtime = malloc(sizeof(struct tm)); + #ifdef OS_WINDOWS +- tm_mtime = localtime_s(tm_mtime, &file_stat.st_mtime); ++ localtime_s(tm_mtime, &file_stat.st_mtime); + #else +- tm_mtime = localtime_r(&file_stat.st_mtime, tm_mtime); ++ localtime_r(&file_stat.st_mtime, tm_mtime); + #endif + strftime(tailoring_doc_timestamp, max_timestamp_len, + "%Y-%m-%dT%H:%M:%S", tm_mtime); +diff --git a/utils/oscap-tool.c b/utils/oscap-tool.c +index 9bfe52697..660a19047 100644 +--- a/utils/oscap-tool.c ++++ b/utils/oscap-tool.c +@@ -315,7 +315,10 @@ static void getopt_parse_env(struct oscap_module *module, int *argc, char ***arg + opt = oscap_strtok_r(opts, delim, &state); + while (opt != NULL) { + eargc++; +- eargv = realloc(eargv, eargc * sizeof(char *)); ++ void *new_eargv = realloc(eargv, eargc * sizeof(char *)); ++ if (new_eargv == NULL) ++ goto exit; ++ eargv = new_eargv; + eargv[eargc - 1] = strdup(opt); + opt = oscap_strtok_r(NULL, delim, &state); + } +@@ -334,6 +337,7 @@ static void getopt_parse_env(struct oscap_module *module, int *argc, char ***arg + + *argc = nargc; + *argv = nargv; ++exit: + free(opts); + free(eargv); + } +diff --git a/utils/oscap-xccdf.c b/utils/oscap-xccdf.c +index af337b844..0a9ae5270 100644 +--- a/utils/oscap-xccdf.c ++++ b/utils/oscap-xccdf.c +@@ -610,8 +610,7 @@ int app_evaluate_xccdf(const struct oscap_action *action) + + /* syslog message */ + #if defined(HAVE_SYSLOG_H) +- syslog(priority, "Evaluation finished. Return code: %d, Base score %f.", evaluation_result, +- session == NULL ? 0 : xccdf_session_get_base_score(session)); ++ syslog(priority, "Evaluation finished. Return code: %d, Base score %f.", evaluation_result, xccdf_session_get_base_score(session)); + #endif + + xccdf_session_set_xccdf_export(session, action->f_results); diff --git a/openscap-1.3.4-plug-memory-leak.patch b/openscap-1.3.5-plug-memory-leak-PR_1616.patch similarity index 100% rename from openscap-1.3.4-plug-memory-leak.patch rename to openscap-1.3.5-plug-memory-leak-PR_1616.patch diff --git a/openscap-1.3.5-yamlfilecontent-fix-field-names-PR_1619.patch b/openscap-1.3.5-yamlfilecontent-fix-field-names-PR_1619.patch new file mode 100644 index 0000000..7d39e31 --- /dev/null +++ b/openscap-1.3.5-yamlfilecontent-fix-field-names-PR_1619.patch @@ -0,0 +1,67 @@ +diff --git a/src/OVAL/probes/independent/yamlfilecontent_probe.c b/src/OVAL/probes/independent/yamlfilecontent_probe.c +index 6f18abf83..17741a240 100644 +--- a/src/OVAL/probes/independent/yamlfilecontent_probe.c ++++ b/src/OVAL/probes/independent/yamlfilecontent_probe.c +@@ -206,6 +206,7 @@ static int yaml_path_query(const char *filepath, const char *yaml_path_cstr, str + yaml_event_type_t event_type; + bool sequence = false; + bool mapping = false; ++ bool fake_mapping = false; + int index = 0; + char *key = strdup("#"); + +@@ -224,21 +225,39 @@ static int yaml_path_query(const char *filepath, const char *yaml_path_cstr, str + + if (sequence) { + if (event_type == YAML_SEQUENCE_END_EVENT) { +- sequence = false; ++ if (fake_mapping) { ++ fake_mapping = false; ++ if (record && record->itemcount > 0) { ++ oscap_list_add(values, record); ++ } else { ++ // Do not collect empty records ++ oscap_htable_free0(record); ++ } ++ record = NULL; ++ } else { ++ sequence = false; ++ } + } else if (event_type == YAML_SEQUENCE_START_EVENT) { +- result_error("YAML path '%s' points to a multi-dimensional structure (sequence containing another sequence)", yaml_path_cstr); +- goto cleanup; ++ if (mapping || fake_mapping) { ++ result_error("YAML path '%s' points to a multi-dimensional structure (a map or a sequence containing other sequences)", yaml_path_cstr); ++ goto cleanup; ++ } else { ++ fake_mapping = true; ++ record = oscap_htable_new(); ++ } + } + } else { + if (event_type == YAML_SEQUENCE_START_EVENT) { + sequence = true; ++ if (mapping) ++ index++; + } + } + + if (mapping) { + if (event_type == YAML_MAPPING_END_EVENT) { + mapping = false; +- if (record->itemcount > 0) { ++ if (record && record->itemcount > 0) { + oscap_list_add(values, record); + } else { + // Do not collect empty records +@@ -255,6 +274,10 @@ static int yaml_path_query(const char *filepath, const char *yaml_path_cstr, str + result_error("YAML path '%s' points to an invalid structure (map containing another map)", yaml_path_cstr); + goto cleanup; + } ++ if (fake_mapping) { ++ result_error("YAML path '%s' points to a multi-dimensional structure (two-dimensional sequence containing a map)", yaml_path_cstr); ++ goto cleanup; ++ } + mapping = true; + sequence = false; + index = 0; diff --git a/openscap.spec b/openscap.spec index 700af10..55bf867 100644 --- a/openscap.spec +++ b/openscap.spec @@ -1,12 +1,15 @@ Name: openscap Version: 1.3.4 -Release: 1%{?dist} +Release: 2%{?dist} Epoch: 1 Summary: Set of open source libraries enabling integration of the SCAP line of standards License: LGPLv2+ URL: http://www.open-scap.org/ Source0: https://github.com/OpenSCAP/%{name}/releases/download/%{version}/%{name}-%{version}.tar.gz -Patch0: openscap-1.3.4-plug-memory-leak.patch +Patch1: openscap-1.3.5-plug-memory-leak-PR_1616.patch +Patch2: openscap-1.3.5-coverity1-PR_1617.patch +Patch3: openscap-1.3.5-coverity2-PR_1620.patch +Patch4: openscap-1.3.5-yamlfilecontent-fix-field-names-PR_1619.patch BuildRequires: cmake >= 2.6 BuildRequires: gcc BuildRequires: gcc-c++ @@ -198,6 +201,10 @@ pathfix.py -i %{__python3} -p -n $RPM_BUILD_ROOT%{_bindir}/scap-as-rpm %{_mandir}/man8/oscap-podman.8* %changelog +* Tue Nov 03 2020 Evgenii Kolesnikov - 1.3.4-2 +- Fix problems uncovered by the Coverity Scan +- Fix field names handling in yamlfilecontent probe + * Wed Oct 07 2020 Evgenii Kolesnikov - 1:1.3.4-1 - Upgrade to the latest upstream release