Blob Blame History Raw
From 35643637c2964e9dd1a459fd76076b088219c117 Mon Sep 17 00:00:00 2001
From: Tomas Halman <thalman@redhat.com>
Date: Tue, 28 Jun 2022 11:07:18 +0200
Subject: [PATCH] cli: fix memory handling with new popt library

This patch makes a copy of the string returned by popt so
the string can be safely used after releasing popt context.

Resolves: https://github.com/authselect/authselect/issues/313
---
 src/cli/cli_tool.c |  19 +++++++-
 src/cli/cli_tool.h |   2 +-
 src/cli/main.c     | 109 ++++++++++++++++++++++++++++-----------------
 3 files changed, 86 insertions(+), 44 deletions(-)

diff --git a/src/cli/cli_tool.c b/src/cli/cli_tool.c
index 83bc1ef339bdc5e610c930ccb605946f0096fb1a..7cf0d45ac2ee8a66db788e7f397c311bb220f72d 100644
--- a/src/cli/cli_tool.c
+++ b/src/cli/cli_tool.c
@@ -301,7 +301,7 @@ errno_t cli_tool_popt_ex(struct cli_cmdline *cmdline,
                          void *popt_fn_pvt,
                          const char *fopt_name,
                          const char *fopt_help,
-                         const char **_fopt,
+                         char **_fopt,
                          bool allow_more_free_opts,
                          bool *_opt_set)
 {
@@ -319,6 +319,11 @@ errno_t cli_tool_popt_ex(struct cli_cmdline *cmdline,
     bool opt_set;
     int ret;
 
+    /* Set output parameter _fopt to NULL value if present. */
+    if (_fopt != NULL) {
+        *_fopt = NULL;
+    }
+
     /* Create help option string. We always need to append command name since
      * we use POPT_CONTEXT_KEEP_FIRST. */
     if (fopt_name == NULL) {
@@ -379,7 +384,12 @@ errno_t cli_tool_popt_ex(struct cli_cmdline *cmdline,
             }
         }
 
-        *_fopt = fopt;
+        *_fopt = strdup(fopt);
+        if (*_fopt == NULL) {
+            ERROR("Out of memory!");
+            ret = ENOMEM;
+            goto done;
+        }
     } else if (_fopt == NULL && fopt != NULL) {
         /* Unexpected free argument. */
         fprintf(stderr, _("Unexpected parameter: %s\n\n"), fopt);
@@ -410,6 +420,11 @@ errno_t cli_tool_popt_ex(struct cli_cmdline *cmdline,
 done:
     poptFreeContext(pc);
     free(help);
+    if (ret != EOK && _fopt != NULL) {
+        free(*_fopt);
+        *_fopt = NULL;
+    }
+
     return ret;
 }
 
diff --git a/src/cli/cli_tool.h b/src/cli/cli_tool.h
index a52260fba470d192990b8887771647f16bebd3cb..b3b361c1b6ddd765cdc575f013de3972a1062f77 100644
--- a/src/cli/cli_tool.h
+++ b/src/cli/cli_tool.h
@@ -68,7 +68,7 @@ errno_t cli_tool_popt_ex(struct cli_cmdline *cmdline,
                          void *popt_fn_pvt,
                          const char *fopt_name,
                          const char *fopt_help,
-                         const char **_fopt,
+                         char **_fopt,
                          bool allow_more_free_opts,
                          bool *_opt_set);
 
diff --git a/src/cli/main.c b/src/cli/main.c
index afe10097612a06b78f0aa45738dc4f9cc3a4f9c9..18486b50bc42f9937cc7294c3e5e2b32cafab5e0 100644
--- a/src/cli/main.c
+++ b/src/cli/main.c
@@ -61,15 +61,17 @@ list_max_length(char **list)
 static errno_t
 parse_profile_options(struct cli_cmdline *cmdline,
                       struct poptOption *options,
-                      const char **_profile_id,
+                      char **_profile_id,
                       const char ***_features)
 {
-    const char *profile_id;
+    char *profile_id;
     const char **features;
     bool profile_skipped;
     errno_t ret;
     int i, j;
 
+    *_profile_id = NULL;
+
     ret = cli_tool_popt_ex(cmdline, options, CLI_TOOL_OPT_OPTIONAL,
                            NULL, NULL, "PROFILE-ID", _("Profile identifier."),
                            &profile_id, true, NULL);
@@ -80,6 +82,7 @@ parse_profile_options(struct cli_cmdline *cmdline,
 
     features = malloc_zero_array(const char *, cmdline->argc);
     if (features == NULL) {
+        free(profile_id);
         return ENOMEM;
     }
 
@@ -143,7 +146,7 @@ static errno_t activate(struct cli_cmdline *cmdline)
 {
     struct authselect_profile *profile = NULL;
     const char **features = NULL;
-    const char *profile_id;
+    char *profile_id = NULL;
     char *requirements = NULL;
     char *backup_name = NULL;
     char **maps = NULL;
@@ -232,6 +235,7 @@ done:
     authselect_array_free(maps);
     authselect_profile_free(profile);
     free(features);
+    free(profile_id);
 
     return ret;
 }
@@ -428,7 +432,7 @@ done:
 static errno_t list_features(struct cli_cmdline *cmdline)
 {
     struct authselect_profile *profile;
-    const char *profile_id;
+    char *profile_id;
     char **features;
     errno_t ret;
     int i;
@@ -438,14 +442,14 @@ static errno_t list_features(struct cli_cmdline *cmdline)
                            &profile_id, true, NULL);
     if (ret != EOK) {
         ERROR("Unable to parse command arguments");
-        return ret;
+        goto done;
     }
 
     ret = authselect_profile(profile_id, &profile);
     if (ret != EOK) {
         ERROR("Unable to get profile information [%d]: %s",
               ret, strerror(ret));
-        return ret;
+        goto done;
     }
 
     features = authselect_profile_features(profile);
@@ -453,7 +457,8 @@ static errno_t list_features(struct cli_cmdline *cmdline)
     if (features == NULL) {
         ERROR("Unable to get profile features [%d]: %s",
               ret, strerror(ret));
-        return ENOMEM;
+        ret = ENOMEM;
+        goto done;
     }
 
     for (i = 0; features[i] != NULL; i++) {
@@ -462,13 +467,17 @@ static errno_t list_features(struct cli_cmdline *cmdline)
 
     authselect_array_free(features);
 
-    return EOK;
+    ret = EOK;
+
+done:
+    free(profile_id);
+    return ret;
 }
 
 static errno_t show(struct cli_cmdline *cmdline)
 {
     struct authselect_profile *profile;
-    const char *profile_id;
+    char *profile_id;
     errno_t ret;
 
     ret = cli_tool_popt_ex(cmdline, NULL, CLI_TOOL_OPT_OPTIONAL,
@@ -476,41 +485,47 @@ static errno_t show(struct cli_cmdline *cmdline)
                            &profile_id, false, NULL);
     if (ret != EOK) {
         ERROR("Unable to parse command arguments");
-        return ret;
+        goto done;
     }
 
     ret = authselect_profile(profile_id, &profile);
     if (ret != EOK) {
         ERROR("Unable to get profile information [%d]: %s",
               ret, strerror(ret));
-        return ENOMEM;
+        ret = ENOMEM;
+        goto done;
     }
 
     puts(authselect_profile_description(profile));
 
     authselect_profile_free(profile);
 
-    return EOK;
+    ret = EOK;
+
+done:
+    free(profile_id);
+    return ret;
 }
 
 static errno_t requirements(struct cli_cmdline *cmdline)
 {
-    struct authselect_profile *profile;
-    const char *profile_id;
+    struct authselect_profile *profile = NULL;
+    char *profile_id = NULL;
     const char **features;
-    char *requirements;
+    char *requirements = NULL;
     errno_t ret;
 
     ret = parse_profile_options(cmdline, NULL, &profile_id, &features);
     if (ret != EOK) {
-        return ret;
+        goto done;
     }
 
     ret = authselect_profile(profile_id, &profile);
     if (ret != EOK) {
         ERROR("Unable to get profile information [%d]: %s",
               ret, strerror(ret));
-        return ENOMEM;
+        ret = ENOMEM;
+        goto done;
     }
 
     requirements = authselect_profile_requirements(profile, features);
@@ -528,6 +543,7 @@ static errno_t requirements(struct cli_cmdline *cmdline)
 
 done:
     free(requirements);
+    free(profile_id);
     authselect_profile_free(profile);
 
     return ret;
@@ -536,7 +552,7 @@ done:
 static errno_t test(struct cli_cmdline *cmdline)
 {
     struct authselect_files *files;
-    const char *profile_id;
+    char *profile_id = NULL;
     const char **features;
     const char *content;
     const char *path;
@@ -583,13 +599,13 @@ static errno_t test(struct cli_cmdline *cmdline)
 
     ret = parse_profile_options(cmdline, options, &profile_id, &features);
     if (ret != EOK) {
-        return ret;
+        goto done;
     }
 
     ret = authselect_files(profile_id, features, &files);
     if (ret != EOK) {
         ERROR("Unable to get generated content [%d]: %s", ret, strerror(ret));
-        return ret;
+        goto done;
     }
 
     for (i = 0; generated[i].content_fn != NULL; i++) {
@@ -613,7 +629,9 @@ static errno_t test(struct cli_cmdline *cmdline)
         }
     }
 
-    return EOK;
+done:
+    free(profile_id);
+    return ret;
 }
 
 static errno_t enable(struct cli_cmdline *cmdline)
@@ -622,7 +640,7 @@ static errno_t enable(struct cli_cmdline *cmdline)
     char *backup_name = NULL;
     char *requirements = NULL;
     char *profile_id = NULL;
-    const char *feature;
+    char *feature;
     const char *features[2];
     int backup = 0;
     int quiet = 0;
@@ -693,6 +711,7 @@ static errno_t enable(struct cli_cmdline *cmdline)
 done:
     free(profile_id);
     free(requirements);
+    free(feature);
     authselect_profile_free(profile);
 
     return ret;
@@ -702,7 +721,7 @@ static errno_t disable(struct cli_cmdline *cmdline)
 {
     int backup = 0;
     char *backup_name = NULL;
-    const char *feature;
+    char *feature;
     errno_t ret;
 
     struct poptOption options[] = {
@@ -716,32 +735,34 @@ static errno_t disable(struct cli_cmdline *cmdline)
                            &feature, false, NULL);
     if (ret != EOK) {
         ERROR("Unable to parse command arguments");
-        return ret;
+        goto done;
     }
 
     ret = perform_backup(false, backup, backup_name);
     if (ret != EOK) {
-        return ret;
+        goto done;
     }
 
     ret = authselect_feature_disable(feature);
     if (ret != EOK) {
         CLI_ERROR("Unable to disable feature [%d]: %s\n", ret, strerror(ret));
-        return ret;
+        goto done;
     }
 
-    return EOK;
+done:
+    free(feature);
+    return ret;
 }
 
 static errno_t create(struct cli_cmdline *cmdline)
 {
-    const char *name;
+    char *name;
     const char *base_id = NULL;
     enum authselect_profile_type type = AUTHSELECT_PROFILE_CUSTOM;
     enum authselect_profile_type base_type = AUTHSELECT_PROFILE_ANY;
     int symlink_flags = AUTHSELECT_SYMLINK_NONE;
     const char **symlinks = NULL;
-    char *path;
+    char *path = NULL;
     errno_t ret;
 
     struct poptOption options[] = {
@@ -761,20 +782,22 @@ static errno_t create(struct cli_cmdline *cmdline)
                            &name, false, NULL);
     if (ret != EOK) {
         ERROR("Unable to parse command arguments");
-        return ret;
+        goto done;
     }
 
     ret = authselect_profile_create(name, type, base_id, base_type,
                                     symlink_flags, symlinks, &path);
     if (ret != EOK) {
         CLI_ERROR("Unable to create new profile [%d]: %s\n", ret, strerror(ret));
-        return ret;
+        goto done;
     }
 
     CLI_PRINT("New profile was created at %s\n", path);
+
+done:
     free(path);
-
-    return EOK;
+    free(name);
+    return ret;
 }
 
 static errno_t backup_list(struct cli_cmdline *cmdline)
@@ -855,7 +878,7 @@ done:
 
 static errno_t backup_remove(struct cli_cmdline *cmdline)
 {
-    const char *name;
+    char *name;
     errno_t ret;
 
     ret = cli_tool_popt_ex(cmdline, NULL, CLI_TOOL_OPT_OPTIONAL,
@@ -864,22 +887,24 @@ static errno_t backup_remove(struct cli_cmdline *cmdline)
                            &name, false, NULL);
     if (ret != EOK) {
         ERROR("Unable to parse command arguments");
-        return ret;
+        goto done;
     }
 
     ret = authselect_backup_remove(name);
     if (ret != EOK) {
         CLI_ERROR("Unable to remove backup [%s] [%d]: %s\n",
                   name, ret, strerror(ret));
-        return ret;
+        goto done;
     }
 
-    return EOK;
+done:
+    free(name);
+    return ret;
 }
 
 static errno_t backup_restore(struct cli_cmdline *cmdline)
 {
-    const char *name;
+    char *name;
     errno_t ret;
 
     ret = cli_tool_popt_ex(cmdline, NULL, CLI_TOOL_OPT_OPTIONAL,
@@ -888,17 +913,19 @@ static errno_t backup_restore(struct cli_cmdline *cmdline)
                            &name, false, NULL);
     if (ret != EOK) {
         ERROR("Unable to parse command arguments");
-        return ret;
+        goto done;
     }
 
     ret = authselect_backup_restore(name);
     if (ret != EOK) {
         CLI_ERROR("Unable to restore backup [%s] [%d]: %s\n",
                   name, ret, strerror(ret));
-        return ret;
+        goto done;
     }
 
-    return EOK;
+done:
+    free(name);
+    return ret;
 }
 
 static errno_t uninstall(struct cli_cmdline *cmdline)
-- 
2.36.1