From 823e8c7a7d1fba5d08764e3734cc946c1fbad2f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= Date: Wed, 17 Jul 2024 09:30:59 +0200 Subject: [PATCH] Fix a memory leak in glob_for_cachedir() Covscan complains: Error: RESOURCE_LEAK (CWE-772): [#def1] [important] libdnf-0.73.1/libdnf/hy-iutil.cpp:100:5: alloc_arg: "wordexp" allocates memory that is stored into "word_vector.we_wordv". libdnf-0.73.1/libdnf/hy-iutil.cpp:102:9: leaked_storage: Variable "word_vector" going out of scope leaks the storage "word_vector.we_wordv" points to. # 100| if (wordexp(p, &word_vector, 0)) { # 101| g_free(p); # 102|-> return ret; # 103| } # 104| for (guint i = 0; i < word_vector.we_wordc; ++i) { The issue is that Covscan model thinks that word_vector should be freed after failing wordexp(). glibc's manual does not explain whether it is or isn't necessary. However, POSIX manual mentions that the memory is valid on WRDE_NOSPACE (not enough memory) error. Reading glibc sources confirms that wordexp() on any error except of WRDE_NOSPACE cleans up and returns original, intact word_vector. Therefore I recognize the missing wordfree() call as an error and this patch fixed it. --- libdnf/hy-iutil.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libdnf/hy-iutil.cpp b/libdnf/hy-iutil.cpp index 43314c60b..4848c9f75 100644 --- a/libdnf/hy-iutil.cpp +++ b/libdnf/hy-iutil.cpp @@ -89,7 +89,7 @@ glob_for_cachedir(char *path) if (!g_str_has_suffix(path, "XXXXXX")) return ret; - wordexp_t word_vector; + wordexp_t word_vector = {0}; char *p = g_strdup(path); const int len = strlen(p); struct stat s; @@ -98,6 +98,7 @@ glob_for_cachedir(char *path) p[len-6] = '*'; p[len-5] = '\0'; if (wordexp(p, &word_vector, 0)) { + wordfree(&word_vector); g_free(p); return ret; }