1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
|
Backport patch to fix CVE-2021-3621.
Upstream-Status: Backport [https://github.com/SSSD/sssd/commit/b4b3267]
CVE: CVE-2021-3621
Signed-off-by: Kai Kang <kai.kang@windriver.com>
From b4b32677a886bc26d60ce0171505aa3ab0c82c8a Mon Sep 17 00:00:00 2001
From: Alexey Tikhonov <atikhono@redhat.com>
Date: Fri, 30 Jul 2021 19:05:31 +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 | 40 +++++++++++++++++-------
src/tools/sssctl/sssctl.h | 2 +-
src/tools/sssctl/sssctl_data.c | 57 +++++++++++-----------------------
src/tools/sssctl/sssctl_logs.c | 31 ++++++++++++++----
4 files changed, 73 insertions(+), 57 deletions(-)
diff --git a/src/tools/sssctl/sssctl.c b/src/tools/sssctl/sssctl.c
index afaa84bc0..403c89c35 100644
--- a/src/tools/sssctl/sssctl.c
+++ b/src/tools/sssctl/sssctl.c
@@ -97,22 +97,37 @@ 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);
fprintf(stderr, _("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));
fprintf(stderr, _("Error while executing external command\n"));
- return EIO;
+ _exit(1);
+ } else {
+ if (waitpid(ret, &wstatus, 0) == -1) {
+ fprintf(stderr,
+ _("Error while executing external command '%s'\n"), argv[0]);
+ return EFAULT;
+ } else if (WEXITSTATUS(wstatus) != 0) {
+ fprintf(stderr,
+ _("Command '%s' failed with [%d]\n"), argv[0], WEXITSTATUS(wstatus));
+ return EIO;
+ }
}
return EOK;
@@ -132,11 +147,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 70fc19eff..71f798b2a 100644
--- a/src/tools/sssctl/sssctl.h
+++ b/src/tools/sssctl/sssctl.h
@@ -42,7 +42,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 cc46cafbf..8a042664c 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) {
fprintf(stderr, _("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) {
fprintf(stderr, _("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) {
fprintf(stderr, _("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) {
fprintf(stderr, _("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 aca988c05..c85cc7a4b 100644
--- a/src/tools/sssctl/sssctl_logs.c
+++ b/src/tools/sssctl/sssctl_logs.c
@@ -32,6 +32,7 @@
#include <popt.h>
#include <stdio.h>
#include <signal.h>
+#include <glob.h>
#include "util/util.h"
#include "tools/common/sss_process.h"
@@ -231,6 +232,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[] = {
@@ -254,8 +256,19 @@ 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[2] = discard_const_p(char, "--size");
+ globbuf.gl_pathv[3] = discard_const_p(char, "0");
+
printf(_("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) {
fprintf(stderr, _("Unable to truncate log files\n"));
return ret;
@@ -270,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,
@@ -281,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) {
- fprintf(stderr, _("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);
printf(_("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) {
fprintf(stderr, _("Unable to archive log files\n"));
return ret;
--
2.33.0
|
