This repository has been archived by the owner on May 30, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 36
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #1216 from kinvolk/jepio/sssd-cve-fix
sys-auth/sssd: fix CVE-2021-3621
- Loading branch information
Showing
2 changed files
with
286 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,284 @@ | ||
From 9377cc4c25a1d889e241f23ec7efcd40fced3c63 Mon Sep 17 00:00:00 2001 | ||
From: Alexey Tikhonov <[email protected]> | ||
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 <[email protected]> | ||
--- | ||
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 04a32bad8..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 --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.25.1 | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters