Skip to content

Commit

Permalink
Fix a memory leak in glob_for_cachedir()
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ppisar authored and kontura committed Jul 18, 2024
1 parent a6d89d6 commit b245193
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion libdnf/hy-iutil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}
Expand Down

0 comments on commit b245193

Please sign in to comment.