From 7ab83f97e1cbefb78ece17232185bdd2985f0bbe Mon Sep 17 00:00:00 2001

From: Alexey Tikhonov <atikhono@redhat.com>

Date: Fri, 18 Jun 2021 13:17:19 +0200

Subject: [PATCH] TOOLS: replace system() with execvp() to avoid execution of

 user supplied command

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

:relnote: A flaw was found in SSSD, where the sssctl command was

vulnerable to shell command injection via the logs-fetch and

cache-expire subcommands. This flaw allows an attacker to trick

the root user into running a specially crafted sssctl command,

such as via sudo, to gain root access. The highest threat from this

vulnerability is to confidentiality, integrity, as well as system

availability.

This patch fixes a flaw by replacing system() with execvp().

:fixes: CVE-2021-3621

Reviewed-by: Pavel Březina <pbrezina@redhat.com>

---

 src/tools/sssctl/sssctl.c      | 39 ++++++++++++++++-------

 src/tools/sssctl/sssctl.h      |  2 +-

 src/tools/sssctl/sssctl_data.c | 57 +++++++++++-----------------------

 src/tools/sssctl/sssctl_logs.c | 32 +++++++++++++++----

 4 files changed, 73 insertions(+), 57 deletions(-)

diff --git a/src/tools/sssctl/sssctl.c b/src/tools/sssctl/sssctl.c

index 2997dbf96..8adaf3091 100644

--- a/src/tools/sssctl/sssctl.c

+++ b/src/tools/sssctl/sssctl.c

@@ -97,22 +97,36 @@ sssctl_prompt(const char *message,

     return SSSCTL_PROMPT_ERROR;

 }

-errno_t sssctl_run_command(const char *command)

+errno_t sssctl_run_command(const char *const argv[])

 {

     int ret;

+    int wstatus;

-    DEBUG(SSSDBG_TRACE_FUNC, "Running %s\n", command);

+    DEBUG(SSSDBG_TRACE_FUNC, "Running '%s'\n", argv[0]);

-    ret = system(command);

+    ret = fork();

     if (ret == -1) {

-        DEBUG(SSSDBG_CRIT_FAILURE, "Unable to execute %s\n", command);

         ERROR("Error while executing external command\n");

         return EFAULT;

-    } else if (WEXITSTATUS(ret) != 0) {

-        DEBUG(SSSDBG_CRIT_FAILURE, "Command %s failed with [%d]\n",

-              command, WEXITSTATUS(ret));

+    }

+

+    if (ret == 0) {

+        /* cast is safe - see

+        https://pubs.opengroup.org/onlinepubs/9699919799/functions/exec.html

+        "The statement about argv[] and envp[] being constants ... "

+        */

+        execvp(argv[0], discard_const_p(char * const, argv));

         ERROR("Error while executing external command\n");

-        return EIO;

+        _exit(1);

+    } else {

+        if (waitpid(ret, &wstatus, 0) == -1) {

+            ERROR("Error while executing external command '%s'\n", argv[0]);

+            return EFAULT;

+        } else if (WEXITSTATUS(wstatus) != 0) {

+            ERROR("Command '%s' failed with [%d]\n",

+                  argv[0], WEXITSTATUS(wstatus));

+            return EIO;

+        }

     }

     return EOK;

@@ -132,11 +146,14 @@ static errno_t sssctl_manage_service(enum sssctl_svc_action action)

 #elif defined(HAVE_SERVICE)

     switch (action) {

     case SSSCTL_SVC_START:

-        return sssctl_run_command(SERVICE_PATH" sssd start");

+        return sssctl_run_command(

+                      (const char *[]){SERVICE_PATH, "sssd", "start", NULL});

     case SSSCTL_SVC_STOP:

-        return sssctl_run_command(SERVICE_PATH" sssd stop");

+        return sssctl_run_command(

+                      (const char *[]){SERVICE_PATH, "sssd", "stop", NULL});

     case SSSCTL_SVC_RESTART:

-        return sssctl_run_command(SERVICE_PATH" sssd restart");

+        return sssctl_run_command(

+                      (const char *[]){SERVICE_PATH, "sssd", "restart", NULL});

     }

 #endif

diff --git a/src/tools/sssctl/sssctl.h b/src/tools/sssctl/sssctl.h

index 0115b2457..599ef6519 100644

--- a/src/tools/sssctl/sssctl.h

+++ b/src/tools/sssctl/sssctl.h

@@ -47,7 +47,7 @@ enum sssctl_prompt_result

 sssctl_prompt(const char *message,

               enum sssctl_prompt_result defval);

-errno_t sssctl_run_command(const char *command);

+errno_t sssctl_run_command(const char *const argv[]); /* argv[0] - command */

 bool sssctl_start_sssd(bool force);

 bool sssctl_stop_sssd(bool force);

 bool sssctl_restart_sssd(bool force);

diff --git a/src/tools/sssctl/sssctl_data.c b/src/tools/sssctl/sssctl_data.c

index 8d79b977f..bf2291341 100644

--- a/src/tools/sssctl/sssctl_data.c

+++ b/src/tools/sssctl/sssctl_data.c

@@ -105,15 +105,15 @@ static errno_t sssctl_backup(bool force)

         }

     }

-    ret = sssctl_run_command("sss_override user-export "

-                             SSS_BACKUP_USER_OVERRIDES);

+    ret = sssctl_run_command((const char *[]){"sss_override", "user-export",

+                                              SSS_BACKUP_USER_OVERRIDES, NULL});

     if (ret != EOK) {

         ERROR("Unable to export user overrides\n");

         return ret;

     }

-    ret = sssctl_run_command("sss_override group-export "

-                             SSS_BACKUP_GROUP_OVERRIDES);

+    ret = sssctl_run_command((const char *[]){"sss_override", "group-export",

+                                              SSS_BACKUP_GROUP_OVERRIDES, NULL});

     if (ret != EOK) {

         ERROR("Unable to export group overrides\n");

         return ret;

@@ -158,8 +158,8 @@ static errno_t sssctl_restore(bool force_start, bool force_restart)

     }

     if (sssctl_backup_file_exists(SSS_BACKUP_USER_OVERRIDES)) {

-        ret = sssctl_run_command("sss_override user-import "

-                                 SSS_BACKUP_USER_OVERRIDES);

+        ret = sssctl_run_command((const char *[]){"sss_override", "user-import",

+                                                  SSS_BACKUP_USER_OVERRIDES, NULL});

         if (ret != EOK) {

             ERROR("Unable to import user overrides\n");

             return ret;

@@ -167,8 +167,8 @@ static errno_t sssctl_restore(bool force_start, bool force_restart)

     }

     if (sssctl_backup_file_exists(SSS_BACKUP_USER_OVERRIDES)) {

-        ret = sssctl_run_command("sss_override group-import "

-                                 SSS_BACKUP_GROUP_OVERRIDES);

+        ret = sssctl_run_command((const char *[]){"sss_override", "group-import",

+                                                  SSS_BACKUP_GROUP_OVERRIDES, NULL});

         if (ret != EOK) {

             ERROR("Unable to import group overrides\n");

             return ret;

@@ -296,40 +296,19 @@ errno_t sssctl_cache_expire(struct sss_cmdline *cmdline,

                             void *pvt)

 {

     errno_t ret;

-    char *cmd_args = NULL;

-    const char *cachecmd = SSS_CACHE;

-    char *cmd = NULL;

-    int i;

-

-    if (cmdline->argc == 0) {

-        ret = sssctl_run_command(cachecmd);

-        goto done;

-    }

-    cmd_args = talloc_strdup(tool_ctx, "");

-    if (cmd_args == NULL) {

-        ret = ENOMEM;

-        goto done;

+    const char **args = talloc_array_size(tool_ctx,

+                                          sizeof(char *),

+                                          cmdline->argc + 2);

+    if (!args) {

+        return ENOMEM;

     }

+    memcpy(&args[1], cmdline->argv, sizeof(char *) * cmdline->argc);

+    args[0] = SSS_CACHE;

+    args[cmdline->argc + 1] = NULL;

-    for (i = 0; i < cmdline->argc; i++) {

-        cmd_args = talloc_strdup_append(cmd_args, cmdline->argv[i]);

-        if (i != cmdline->argc - 1) {

-            cmd_args = talloc_strdup_append(cmd_args, " ");

-        }

-    }

-

-    cmd = talloc_asprintf(tool_ctx, "%s %s", cachecmd, cmd_args);

-    if (cmd == NULL) {

-        ret = ENOMEM;

-        goto done;

-    }

-

-    ret = sssctl_run_command(cmd);

-

-done:

-    talloc_free(cmd_args);

-    talloc_free(cmd);

+    ret = sssctl_run_command(args);

+    talloc_free(args);

     return ret;

 }

diff --git a/src/tools/sssctl/sssctl_logs.c b/src/tools/sssctl/sssctl_logs.c

index 9ff2be05b..ebb2c4571 100644

--- a/src/tools/sssctl/sssctl_logs.c

+++ b/src/tools/sssctl/sssctl_logs.c

@@ -31,6 +31,7 @@

 #include <ldb.h>

 #include <popt.h>

 #include <stdio.h>

+#include <glob.h>

 #include "util/util.h"

 #include "tools/common/sss_process.h"

@@ -230,6 +231,7 @@ errno_t sssctl_logs_remove(struct sss_cmdline *cmdline,

 {

     struct sssctl_logs_opts opts = {0};

     errno_t ret;

+    glob_t globbuf;

     /* Parse command line. */

     struct poptOption options[] = {

@@ -253,8 +255,20 @@ errno_t sssctl_logs_remove(struct sss_cmdline *cmdline,

         sss_signal(SIGHUP);

     } else {

+        globbuf.gl_offs = 4;

+        ret = glob(LOG_PATH"/*.log", GLOB_ERR|GLOB_DOOFFS, NULL, &globbuf);

+        if (ret != 0) {

+            DEBUG(SSSDBG_CRIT_FAILURE, "Unable to expand log files list\n");

+            return ret;

+        }

+        globbuf.gl_pathv[0] = discard_const_p(char, "truncate");

+        globbuf.gl_pathv[1] = discard_const_p(char, "--no-create");

+        globbuf.gl_pathv[2] = discard_const_p(char, "--size");

+        globbuf.gl_pathv[3] = discard_const_p(char, "0");

+

         PRINT("Truncating log files...\n");

-        ret = sssctl_run_command("truncate --no-create --size 0 " LOG_FILES);

+        ret = sssctl_run_command((const char * const*)globbuf.gl_pathv);

+        globfree(&globbuf);

         if (ret != EOK) {

             ERROR("Unable to truncate log files\n");

             return ret;

@@ -269,8 +283,8 @@ errno_t sssctl_logs_fetch(struct sss_cmdline *cmdline,

                           void *pvt)

 {

     const char *file;

-    const char *cmd;

     errno_t ret;

+    glob_t globbuf;

     /* Parse command line. */

     ret = sss_tool_popt_ex(cmdline, NULL, SSS_TOOL_OPT_OPTIONAL, NULL, NULL,

@@ -280,13 +294,19 @@ errno_t sssctl_logs_fetch(struct sss_cmdline *cmdline,

         return ret;

     }

-    cmd = talloc_asprintf(tool_ctx, "tar -czf %s %s", file, LOG_FILES);

-    if (cmd == NULL) {

-        ERROR("Out of memory!");

+    globbuf.gl_offs = 3;

+    ret = glob(LOG_PATH"/*.log", GLOB_ERR|GLOB_DOOFFS, NULL, &globbuf);

+    if (ret != 0) {

+        DEBUG(SSSDBG_CRIT_FAILURE, "Unable to expand log files list\n");

+        return ret;

     }

+    globbuf.gl_pathv[0] = discard_const_p(char, "tar");

+    globbuf.gl_pathv[1] = discard_const_p(char, "-czf");

+    globbuf.gl_pathv[2] = discard_const_p(char, file);

     PRINT("Archiving log files into %s...\n", file);

-    ret = sssctl_run_command(cmd);

+    ret = sssctl_run_command((const char * const*)globbuf.gl_pathv);

+    globfree(&globbuf);

     if (ret != EOK) {

         ERROR("Unable to archive log files\n");

         return ret;

-- 

2.26.3