Andrew Geissler | 5199d83 | 2021-09-24 16:47:35 -0500 | [diff] [blame] | 1 | Backport patch to fix CVE-2021-3621. |
| 2 | |
| 3 | Upstream-Status: Backport [https://github.com/SSSD/sssd/commit/7ab83f9] |
| 4 | CVE: CVE-2021-3621 |
| 5 | |
| 6 | Signed-off-by: Kai Kang <kai.kang@windriver.com> |
| 7 | |
| 8 | From 7ab83f97e1cbefb78ece17232185bdd2985f0bbe Mon Sep 17 00:00:00 2001 |
| 9 | From: Alexey Tikhonov <atikhono@redhat.com> |
| 10 | Date: Fri, 18 Jun 2021 13:17:19 +0200 |
| 11 | Subject: [PATCH] TOOLS: replace system() with execvp() to avoid execution of |
| 12 | user supplied command |
| 13 | MIME-Version: 1.0 |
| 14 | Content-Type: text/plain; charset=UTF-8 |
| 15 | Content-Transfer-Encoding: 8bit |
| 16 | |
| 17 | :relnote: A flaw was found in SSSD, where the sssctl command was |
| 18 | vulnerable to shell command injection via the logs-fetch and |
| 19 | cache-expire subcommands. This flaw allows an attacker to trick |
| 20 | the root user into running a specially crafted sssctl command, |
| 21 | such as via sudo, to gain root access. The highest threat from this |
| 22 | vulnerability is to confidentiality, integrity, as well as system |
| 23 | availability. |
| 24 | This patch fixes a flaw by replacing system() with execvp(). |
| 25 | |
| 26 | :fixes: CVE-2021-3621 |
| 27 | |
| 28 | Reviewed-by: Pavel Březina <pbrezina@redhat.com> |
| 29 | --- |
| 30 | src/tools/sssctl/sssctl.c | 39 ++++++++++++++++------- |
| 31 | src/tools/sssctl/sssctl.h | 2 +- |
| 32 | src/tools/sssctl/sssctl_data.c | 57 +++++++++++----------------------- |
| 33 | src/tools/sssctl/sssctl_logs.c | 32 +++++++++++++++---- |
| 34 | 4 files changed, 73 insertions(+), 57 deletions(-) |
| 35 | |
| 36 | diff --git a/src/tools/sssctl/sssctl.c b/src/tools/sssctl/sssctl.c |
| 37 | index 2997dbf968..8adaf30910 100644 |
| 38 | --- a/src/tools/sssctl/sssctl.c |
| 39 | +++ b/src/tools/sssctl/sssctl.c |
| 40 | @@ -97,22 +97,36 @@ sssctl_prompt(const char *message, |
| 41 | return SSSCTL_PROMPT_ERROR; |
| 42 | } |
| 43 | |
| 44 | -errno_t sssctl_run_command(const char *command) |
| 45 | +errno_t sssctl_run_command(const char *const argv[]) |
| 46 | { |
| 47 | int ret; |
| 48 | + int wstatus; |
| 49 | |
| 50 | - DEBUG(SSSDBG_TRACE_FUNC, "Running %s\n", command); |
| 51 | + DEBUG(SSSDBG_TRACE_FUNC, "Running '%s'\n", argv[0]); |
| 52 | |
| 53 | - ret = system(command); |
| 54 | + ret = fork(); |
| 55 | if (ret == -1) { |
| 56 | - DEBUG(SSSDBG_CRIT_FAILURE, "Unable to execute %s\n", command); |
| 57 | ERROR("Error while executing external command\n"); |
| 58 | return EFAULT; |
| 59 | - } else if (WEXITSTATUS(ret) != 0) { |
| 60 | - DEBUG(SSSDBG_CRIT_FAILURE, "Command %s failed with [%d]\n", |
| 61 | - command, WEXITSTATUS(ret)); |
| 62 | + } |
| 63 | + |
| 64 | + if (ret == 0) { |
| 65 | + /* cast is safe - see |
| 66 | + https://pubs.opengroup.org/onlinepubs/9699919799/functions/exec.html |
| 67 | + "The statement about argv[] and envp[] being constants ... " |
| 68 | + */ |
| 69 | + execvp(argv[0], discard_const_p(char * const, argv)); |
| 70 | ERROR("Error while executing external command\n"); |
| 71 | - return EIO; |
| 72 | + _exit(1); |
| 73 | + } else { |
| 74 | + if (waitpid(ret, &wstatus, 0) == -1) { |
| 75 | + ERROR("Error while executing external command '%s'\n", argv[0]); |
| 76 | + return EFAULT; |
| 77 | + } else if (WEXITSTATUS(wstatus) != 0) { |
| 78 | + ERROR("Command '%s' failed with [%d]\n", |
| 79 | + argv[0], WEXITSTATUS(wstatus)); |
| 80 | + return EIO; |
| 81 | + } |
| 82 | } |
| 83 | |
| 84 | return EOK; |
| 85 | @@ -132,11 +146,14 @@ static errno_t sssctl_manage_service(enum sssctl_svc_action action) |
| 86 | #elif defined(HAVE_SERVICE) |
| 87 | switch (action) { |
| 88 | case SSSCTL_SVC_START: |
| 89 | - return sssctl_run_command(SERVICE_PATH" sssd start"); |
| 90 | + return sssctl_run_command( |
| 91 | + (const char *[]){SERVICE_PATH, "sssd", "start", NULL}); |
| 92 | case SSSCTL_SVC_STOP: |
| 93 | - return sssctl_run_command(SERVICE_PATH" sssd stop"); |
| 94 | + return sssctl_run_command( |
| 95 | + (const char *[]){SERVICE_PATH, "sssd", "stop", NULL}); |
| 96 | case SSSCTL_SVC_RESTART: |
| 97 | - return sssctl_run_command(SERVICE_PATH" sssd restart"); |
| 98 | + return sssctl_run_command( |
| 99 | + (const char *[]){SERVICE_PATH, "sssd", "restart", NULL}); |
| 100 | } |
| 101 | #endif |
| 102 | |
| 103 | diff --git a/src/tools/sssctl/sssctl.h b/src/tools/sssctl/sssctl.h |
| 104 | index 0115b2457c..599ef65196 100644 |
| 105 | --- a/src/tools/sssctl/sssctl.h |
| 106 | +++ b/src/tools/sssctl/sssctl.h |
| 107 | @@ -47,7 +47,7 @@ enum sssctl_prompt_result |
| 108 | sssctl_prompt(const char *message, |
| 109 | enum sssctl_prompt_result defval); |
| 110 | |
| 111 | -errno_t sssctl_run_command(const char *command); |
| 112 | +errno_t sssctl_run_command(const char *const argv[]); /* argv[0] - command */ |
| 113 | bool sssctl_start_sssd(bool force); |
| 114 | bool sssctl_stop_sssd(bool force); |
| 115 | bool sssctl_restart_sssd(bool force); |
| 116 | diff --git a/src/tools/sssctl/sssctl_data.c b/src/tools/sssctl/sssctl_data.c |
| 117 | index 8d79b977fd..bf22913416 100644 |
| 118 | --- a/src/tools/sssctl/sssctl_data.c |
| 119 | +++ b/src/tools/sssctl/sssctl_data.c |
| 120 | @@ -105,15 +105,15 @@ static errno_t sssctl_backup(bool force) |
| 121 | } |
| 122 | } |
| 123 | |
| 124 | - ret = sssctl_run_command("sss_override user-export " |
| 125 | - SSS_BACKUP_USER_OVERRIDES); |
| 126 | + ret = sssctl_run_command((const char *[]){"sss_override", "user-export", |
| 127 | + SSS_BACKUP_USER_OVERRIDES, NULL}); |
| 128 | if (ret != EOK) { |
| 129 | ERROR("Unable to export user overrides\n"); |
| 130 | return ret; |
| 131 | } |
| 132 | |
| 133 | - ret = sssctl_run_command("sss_override group-export " |
| 134 | - SSS_BACKUP_GROUP_OVERRIDES); |
| 135 | + ret = sssctl_run_command((const char *[]){"sss_override", "group-export", |
| 136 | + SSS_BACKUP_GROUP_OVERRIDES, NULL}); |
| 137 | if (ret != EOK) { |
| 138 | ERROR("Unable to export group overrides\n"); |
| 139 | return ret; |
| 140 | @@ -158,8 +158,8 @@ static errno_t sssctl_restore(bool force_start, bool force_restart) |
| 141 | } |
| 142 | |
| 143 | if (sssctl_backup_file_exists(SSS_BACKUP_USER_OVERRIDES)) { |
| 144 | - ret = sssctl_run_command("sss_override user-import " |
| 145 | - SSS_BACKUP_USER_OVERRIDES); |
| 146 | + ret = sssctl_run_command((const char *[]){"sss_override", "user-import", |
| 147 | + SSS_BACKUP_USER_OVERRIDES, NULL}); |
| 148 | if (ret != EOK) { |
| 149 | ERROR("Unable to import user overrides\n"); |
| 150 | return ret; |
| 151 | @@ -167,8 +167,8 @@ static errno_t sssctl_restore(bool force_start, bool force_restart) |
| 152 | } |
| 153 | |
| 154 | if (sssctl_backup_file_exists(SSS_BACKUP_USER_OVERRIDES)) { |
| 155 | - ret = sssctl_run_command("sss_override group-import " |
| 156 | - SSS_BACKUP_GROUP_OVERRIDES); |
| 157 | + ret = sssctl_run_command((const char *[]){"sss_override", "group-import", |
| 158 | + SSS_BACKUP_GROUP_OVERRIDES, NULL}); |
| 159 | if (ret != EOK) { |
| 160 | ERROR("Unable to import group overrides\n"); |
| 161 | return ret; |
| 162 | @@ -296,40 +296,19 @@ errno_t sssctl_cache_expire(struct sss_cmdline *cmdline, |
| 163 | void *pvt) |
| 164 | { |
| 165 | errno_t ret; |
| 166 | - char *cmd_args = NULL; |
| 167 | - const char *cachecmd = SSS_CACHE; |
| 168 | - char *cmd = NULL; |
| 169 | - int i; |
| 170 | - |
| 171 | - if (cmdline->argc == 0) { |
| 172 | - ret = sssctl_run_command(cachecmd); |
| 173 | - goto done; |
| 174 | - } |
| 175 | |
| 176 | - cmd_args = talloc_strdup(tool_ctx, ""); |
| 177 | - if (cmd_args == NULL) { |
| 178 | - ret = ENOMEM; |
| 179 | - goto done; |
| 180 | + const char **args = talloc_array_size(tool_ctx, |
| 181 | + sizeof(char *), |
| 182 | + cmdline->argc + 2); |
| 183 | + if (!args) { |
| 184 | + return ENOMEM; |
| 185 | } |
| 186 | + memcpy(&args[1], cmdline->argv, sizeof(char *) * cmdline->argc); |
| 187 | + args[0] = SSS_CACHE; |
| 188 | + args[cmdline->argc + 1] = NULL; |
| 189 | |
| 190 | - for (i = 0; i < cmdline->argc; i++) { |
| 191 | - cmd_args = talloc_strdup_append(cmd_args, cmdline->argv[i]); |
| 192 | - if (i != cmdline->argc - 1) { |
| 193 | - cmd_args = talloc_strdup_append(cmd_args, " "); |
| 194 | - } |
| 195 | - } |
| 196 | - |
| 197 | - cmd = talloc_asprintf(tool_ctx, "%s %s", cachecmd, cmd_args); |
| 198 | - if (cmd == NULL) { |
| 199 | - ret = ENOMEM; |
| 200 | - goto done; |
| 201 | - } |
| 202 | - |
| 203 | - ret = sssctl_run_command(cmd); |
| 204 | - |
| 205 | -done: |
| 206 | - talloc_free(cmd_args); |
| 207 | - talloc_free(cmd); |
| 208 | + ret = sssctl_run_command(args); |
| 209 | |
| 210 | + talloc_free(args); |
| 211 | return ret; |
| 212 | } |
| 213 | diff --git a/src/tools/sssctl/sssctl_logs.c b/src/tools/sssctl/sssctl_logs.c |
| 214 | index 9ff2be05b6..ebb2c4571c 100644 |
| 215 | --- a/src/tools/sssctl/sssctl_logs.c |
| 216 | +++ b/src/tools/sssctl/sssctl_logs.c |
| 217 | @@ -31,6 +31,7 @@ |
| 218 | #include <ldb.h> |
| 219 | #include <popt.h> |
| 220 | #include <stdio.h> |
| 221 | +#include <glob.h> |
| 222 | |
| 223 | #include "util/util.h" |
| 224 | #include "tools/common/sss_process.h" |
| 225 | @@ -230,6 +231,7 @@ errno_t sssctl_logs_remove(struct sss_cmdline *cmdline, |
| 226 | { |
| 227 | struct sssctl_logs_opts opts = {0}; |
| 228 | errno_t ret; |
| 229 | + glob_t globbuf; |
| 230 | |
| 231 | /* Parse command line. */ |
| 232 | struct poptOption options[] = { |
| 233 | @@ -253,8 +255,20 @@ errno_t sssctl_logs_remove(struct sss_cmdline *cmdline, |
| 234 | |
| 235 | sss_signal(SIGHUP); |
| 236 | } else { |
| 237 | + globbuf.gl_offs = 4; |
| 238 | + ret = glob(LOG_PATH"/*.log", GLOB_ERR|GLOB_DOOFFS, NULL, &globbuf); |
| 239 | + if (ret != 0) { |
| 240 | + DEBUG(SSSDBG_CRIT_FAILURE, "Unable to expand log files list\n"); |
| 241 | + return ret; |
| 242 | + } |
| 243 | + globbuf.gl_pathv[0] = discard_const_p(char, "truncate"); |
| 244 | + globbuf.gl_pathv[1] = discard_const_p(char, "--no-create"); |
| 245 | + globbuf.gl_pathv[2] = discard_const_p(char, "--size"); |
| 246 | + globbuf.gl_pathv[3] = discard_const_p(char, "0"); |
| 247 | + |
| 248 | PRINT("Truncating log files...\n"); |
| 249 | - ret = sssctl_run_command("truncate --no-create --size 0 " LOG_FILES); |
| 250 | + ret = sssctl_run_command((const char * const*)globbuf.gl_pathv); |
| 251 | + globfree(&globbuf); |
| 252 | if (ret != EOK) { |
| 253 | ERROR("Unable to truncate log files\n"); |
| 254 | return ret; |
| 255 | @@ -269,8 +283,8 @@ errno_t sssctl_logs_fetch(struct sss_cmdline *cmdline, |
| 256 | void *pvt) |
| 257 | { |
| 258 | const char *file; |
| 259 | - const char *cmd; |
| 260 | errno_t ret; |
| 261 | + glob_t globbuf; |
| 262 | |
| 263 | /* Parse command line. */ |
| 264 | ret = sss_tool_popt_ex(cmdline, NULL, SSS_TOOL_OPT_OPTIONAL, NULL, NULL, |
| 265 | @@ -280,13 +294,19 @@ errno_t sssctl_logs_fetch(struct sss_cmdline *cmdline, |
| 266 | return ret; |
| 267 | } |
| 268 | |
| 269 | - cmd = talloc_asprintf(tool_ctx, "tar -czf %s %s", file, LOG_FILES); |
| 270 | - if (cmd == NULL) { |
| 271 | - ERROR("Out of memory!"); |
| 272 | + globbuf.gl_offs = 3; |
| 273 | + ret = glob(LOG_PATH"/*.log", GLOB_ERR|GLOB_DOOFFS, NULL, &globbuf); |
| 274 | + if (ret != 0) { |
| 275 | + DEBUG(SSSDBG_CRIT_FAILURE, "Unable to expand log files list\n"); |
| 276 | + return ret; |
| 277 | } |
| 278 | + globbuf.gl_pathv[0] = discard_const_p(char, "tar"); |
| 279 | + globbuf.gl_pathv[1] = discard_const_p(char, "-czf"); |
| 280 | + globbuf.gl_pathv[2] = discard_const_p(char, file); |
| 281 | |
| 282 | PRINT("Archiving log files into %s...\n", file); |
| 283 | - ret = sssctl_run_command(cmd); |
| 284 | + ret = sssctl_run_command((const char * const*)globbuf.gl_pathv); |
| 285 | + globfree(&globbuf); |
| 286 | if (ret != EOK) { |
| 287 | ERROR("Unable to archive log files\n"); |
| 288 | return ret; |