From 37a9f0f6a1fc366d2ad7617d0276165669e9011e Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Tue, 10 Aug 2021 14:32:16 +0200 Subject: [PATCH] Resolves: rhbz#1938708 - review of important potential issues detected by static analyzers in ding-libs-0.6.1-47.el9 --- 0004-INI-fix-check-for-error-code.patch | 40 +++++++++++++ ...ILS-suppress-false-positive-warnings.patch | 58 +++++++++++++++++++ ...ress-false-positive-coverity-warning.patch | 28 +++++++++ ding-libs.spec | 8 ++- 4 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 0004-INI-fix-check-for-error-code.patch create mode 100644 0005-PATH_UTILS-suppress-false-positive-warnings.patch create mode 100644 0006-INI-suppress-false-positive-coverity-warning.patch diff --git a/0004-INI-fix-check-for-error-code.patch b/0004-INI-fix-check-for-error-code.patch new file mode 100644 index 0000000..196e3cf --- /dev/null +++ b/0004-INI-fix-check-for-error-code.patch @@ -0,0 +1,40 @@ +From ec6817736968fb4683b9df0bd932c1a86dec0ba8 Mon Sep 17 00:00:00 2001 +From: Alexey Tikhonov +Date: Wed, 4 Aug 2021 19:22:19 +0200 +Subject: [PATCH 4/6] INI: fix check for error code + +In case of fail `asprintf()` returns -1, not 1. + +Fixes following covscan issues: +``` +Error: RESOURCE_LEAK (CWE-772): [#def1] +ding-libs-0.6.1/ini/ini_configmod.c:869: alloc_arg: "asprintf" allocates memory that is stored into "strval". [Note: The source code implementation of the function has been overridden by a builtin model.] +ding-libs-0.6.1/ini/ini_configmod.c:873: leaked_storage: Variable "strval" going out of scope leaks the storage it points to. + # 871| TRACE_ERROR_NUMBER("Asprintf failed.", ret); + # 872| /* The main reason is propbaly memory allocation */ + # 873|-> return ENOMEM; + # 874| } + # 875| +``` + +Reviewed-by: Pawel Polawski +--- + ini/ini_configmod.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/ini/ini_configmod.c b/ini/ini_configmod.c +index da4175c..88a7133 100644 +--- a/ini/ini_configmod.c ++++ b/ini/ini_configmod.c +@@ -867,7 +867,7 @@ int ini_config_add_double_value(struct ini_cfgobj *ini_config, + TRACE_FLOW_ENTRY(); + + ret = asprintf(&strval, "%f", value); +- if (ret == 1) { ++ if (ret == -1) { + TRACE_ERROR_NUMBER("Asprintf failed.", ret); + /* The main reason is propbaly memory allocation */ + return ENOMEM; +-- +2.26.3 + diff --git a/0005-PATH_UTILS-suppress-false-positive-warnings.patch b/0005-PATH_UTILS-suppress-false-positive-warnings.patch new file mode 100644 index 0000000..ae7db76 --- /dev/null +++ b/0005-PATH_UTILS-suppress-false-positive-warnings.patch @@ -0,0 +1,58 @@ +From 82ee1cff9d7401f4381cfa574f8b102625b06a31 Mon Sep 17 00:00:00 2001 +From: Alexey Tikhonov +Date: Thu, 5 Aug 2021 18:02:57 +0200 +Subject: [PATCH 5/6] PATH_UTILS: suppress false positive warnings + +Warnings are false positives: every such `strncpy` is followed +by an explicit check that result is NULL-terminated. + +Reviewed-by: Pawel Polawski +--- + path_utils/path_utils.c | 5 +++++ + 1 file changed, 5 insertions(+) + +diff --git a/path_utils/path_utils.c b/path_utils/path_utils.c +index 61605ab..5203cc9 100644 +--- a/path_utils/path_utils.c ++++ b/path_utils/path_utils.c +@@ -116,6 +116,7 @@ int get_basename(char *base_name, size_t base_name_size, const char *path) + if (!path) return EINVAL; + if (!base_name || base_name_size < 1) return ENOBUFS; + ++ /* coverity[buffer_size_warning : SUPPRESS] */ /* false positive warning */ + strncpy(tmp_path, path, sizeof(tmp_path)); + if (tmp_path[sizeof(tmp_path)-1] != '\0') return ENOBUFS; + strncpy(base_name, basename(tmp_path), base_name_size); +@@ -137,6 +138,7 @@ int get_dirname(char *dir_path, size_t dir_path_size, const char *path) + if (!path) return EINVAL; + if (!dir_path || dir_path_size < 1) return ENOBUFS; + ++ /* coverity[buffer_size_warning : SUPPRESS] */ /* false positive warning */ + strncpy(tmp_path, path, sizeof(tmp_path)); + if (tmp_path[sizeof(tmp_path)-1] != '\0') return ENOBUFS; + strncpy(dir_path, dirname(tmp_path), dir_path_size); +@@ -161,11 +163,13 @@ int get_directory_and_base_name(char *dir_path, size_t dir_path_size, + if (!dir_path || dir_path_size < 1) return ENOBUFS; + if (!base_name || base_name_size < 1) return ENOBUFS; + ++ /* coverity[buffer_size_warning : SUPPRESS] */ /* false positive warning */ + strncpy(tmp_path, path, sizeof(tmp_path)); + if (tmp_path[sizeof(tmp_path)-1] != '\0') return ENOBUFS; + strncpy(base_name, basename(tmp_path), base_name_size); + if (base_name[base_name_size-1] != '\0') return ENOBUFS; + ++ /* coverity[buffer_size_warning : SUPPRESS] */ /* false positive warning */ + strncpy(tmp_path, path, sizeof(tmp_path)); + if (tmp_path[sizeof(tmp_path)-1] != '\0') return ENOBUFS; + strncpy(dir_path, dirname(tmp_path), dir_path_size); +@@ -528,6 +532,7 @@ int find_existing_directory_ancestor(char *ancestor, size_t ancestor_size, const + + if (!ancestor || ancestor_size < 1) return ENOBUFS; + *ancestor = 0; ++ /* coverity[buffer_size_warning : SUPPRESS] */ /* false positive warning */ + strncpy(dir_path, path, sizeof(dir_path)); + if (dir_path[sizeof(dir_path)-1] != '\0') return ENOBUFS; + +-- +2.26.3 + diff --git a/0006-INI-suppress-false-positive-coverity-warning.patch b/0006-INI-suppress-false-positive-coverity-warning.patch new file mode 100644 index 0000000..1055c77 --- /dev/null +++ b/0006-INI-suppress-false-positive-coverity-warning.patch @@ -0,0 +1,28 @@ +From 584dc25f2c31f4d8e5cf7154e0362e4d2504779c Mon Sep 17 00:00:00 2001 +From: Alexey Tikhonov +Date: Thu, 5 Aug 2021 18:48:23 +0200 +Subject: [PATCH 6/6] INI: suppress false positive coverity warning + +`get_str_cfg_array()` returns `char **array` that is composed of pointers +to slices of `copy` so `copy` can't be freed here. + +Reviewed-by: Pawel Polawski +--- + ini/ini_get_array.c | 1 + + 1 file changed, 1 insertion(+) + +diff --git a/ini/ini_get_array.c b/ini/ini_get_array.c +index 30ed423..95d0b05 100644 +--- a/ini/ini_get_array.c ++++ b/ini/ini_get_array.c +@@ -164,6 +164,7 @@ static char **get_str_cfg_array(struct collection_item *item, + /* If count is 0 the copy needs to be freed */ + if (count == 0) free(copy); + TRACE_FLOW_STRING("get_str_cfg_array", "Exit"); ++ /* coverity[leaked_storage : SUPPRESS] */ /* false positive warning */ + return array; + } + +-- +2.26.3 + diff --git a/ding-libs.spec b/ding-libs.spec index f846ca1..2ce428a 100644 --- a/ding-libs.spec +++ b/ding-libs.spec @@ -1,6 +1,6 @@ Name: ding-libs Version: 0.6.1 -Release: 50%{?dist} +Release: 51%{?dist} Summary: "Ding is not GLib" assorted utility libraries License: LGPLv3+ URL: https://pagure.io/SSSD/ding-libs @@ -21,6 +21,9 @@ Patch0: INI-Silence-ini_augment-match-failures.patch Patch1: INI-Remove-definiton-of-TRACE_LEVEL.patch Patch2: INI-Fix-detection-of-error-messages.patch Patch3: TEST-validators_ut_check-Fix-fail-with-new-glibc.patch +Patch4: 0004-INI-fix-check-for-error-code.patch +Patch5: 0005-PATH_UTILS-suppress-false-positive-warnings.patch +Patch6: 0006-INI-suppress-false-positive-coverity-warning.patch ### Dependencies ### # ding-libs is a meta-package that will pull in all of its own @@ -330,6 +333,9 @@ rm -f \ rm -f */doc/html/installdox %changelog +* Tue Aug 10 2021 Alexey Tikhonov - 0.6.1-51 +- Resolves: rhbz#1938708 - review of important potential issues detected by static analyzers in ding-libs-0.6.1-47.el9 + * Mon Aug 09 2021 Mohan Boddu - 0.6.1-50 - Rebuilt for IMA sigs, glibc 2.34, aarch64 flags Related: rhbz#1991688