From b99047c977668af71c814dd4487bfcbee50b713d Mon Sep 17 00:00:00 2001 From: r10nw7fd3 Date: Wed, 27 Mar 2024 09:53:58 +0300 Subject: [PATCH 1/2] fix(clib-package): Double free --- src/common/clib-package.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/common/clib-package.c b/src/common/clib-package.c index 48ad8649..8d29a066 100644 --- a/src/common/clib-package.c +++ b/src/common/clib-package.c @@ -23,10 +23,10 @@ #include "parse-repo/parse-repo.h" #include "parson/parson.h" #include "path-join/path-join.h" +#include "rimraf/rimraf.h" #include "strdup/strdup.h" #include "substr/substr.h" #include "tempdir/tempdir.h" -#include "rimraf/rimraf.h" #include #include #include @@ -324,7 +324,7 @@ static inline list_t *parse_package_deps(JSON_Object *obj) { if (!(dep = clib_package_dependency_new(name, version))) goto loop_cleanup; - list_node_t* dep_node = list_node_new(dep); + list_node_t *dep_node = list_node_new(dep); // note: if we fail to allocate the node itself, // `dep` will never be pushed on the list if (!dep_node) { @@ -711,6 +711,7 @@ clib_package_new_from_slug_with_package_name(const char *slug, int verbose, pkg->version = version; } else { free(version); + version = NULL; } } } else { @@ -724,6 +725,7 @@ clib_package_new_from_slug_with_package_name(const char *slug, int verbose, pkg->author = author; } else { free(author); + author = NULL; } } else { pkg->author = strdup(author); @@ -1544,7 +1546,7 @@ int clib_package_install(clib_package_t *pkg, const char *dir, int verbose) { #ifdef HAVE_PTHREADS // Here there are i-1 threads running. for (int j = 0; j < i; j++) { - fetch_package_file_thread_data_t *data = fetchs[j]; + fetch_package_file_thread_data_t *data = fetchs[j]; int *status; pthread_join(data->thread, (void **)&status); @@ -1566,7 +1568,6 @@ int clib_package_install(clib_package_t *pkg, const char *dir, int verbose) { } #endif - install: if (pkg->configure) { E_FORMAT(&command, "cd %s/%s && %s", dir, pkg->name, pkg->configure); @@ -1597,7 +1598,6 @@ int clib_package_install(clib_package_t *pkg, const char *dir, int verbose) { pthread_mutex_unlock(&lock.mutex); #endif - cleanup: if (pkg_dir) { if (0 != rc) { @@ -1629,7 +1629,8 @@ int clib_package_install(clib_package_t *pkg, const char *dir, int verbose) { #endif if (0 != rc && pkg) { clib_cache_delete_json(pkg->author, pkg->name, pkg->version); - _debug("deleted json cache: %s/%s@%s", pkg->author, pkg->name, pkg->version); + _debug("deleted json cache: %s/%s@%s", pkg->author, pkg->name, + pkg->version); } #ifdef HAVE_PTHREADS pthread_mutex_unlock(&lock.mutex); From 956478d11b9009b37f1b7c99d2f72838716cd5cc Mon Sep 17 00:00:00 2001 From: r10nw7fd3 Date: Wed, 27 Mar 2024 09:59:05 +0300 Subject: [PATCH 2/2] chore(clib-package): Remove redundant nullability checks Since the check is done right after calling `parse_repo_{owner,version}()`, there is no need to do that down into the function. Besides that, if `author` was ever null, a call to `strdup(author)` would invoke undefined behaviour. --- src/common/clib-package.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/common/clib-package.c b/src/common/clib-package.c index 8d29a066..136754e8 100644 --- a/src/common/clib-package.c +++ b/src/common/clib-package.c @@ -704,22 +704,20 @@ clib_package_new_from_slug_with_package_name(const char *slug, int verbose, // force version number if (pkg->version) { - if (version) { - if (0 != strcmp(version, DEFAULT_REPO_VERSION)) { - _debug("forcing version number: %s (%s)", version, pkg->version); - free(pkg->version); - pkg->version = version; - } else { - free(version); - version = NULL; - } + if (0 != strcmp(version, DEFAULT_REPO_VERSION)) { + _debug("forcing version number: %s (%s)", version, pkg->version); + free(pkg->version); + pkg->version = version; + } else { + free(version); + version = NULL; } } else { pkg->version = version; } // force package author (don't know how this could fail) - if (author && pkg->author) { + if (pkg->author) { if (0 != strcmp(author, pkg->author)) { free(pkg->author); pkg->author = author;