Skip to content

Commit

Permalink
TOOLS: replace system() with execvp() to avoid execution of user supp…
Browse files Browse the repository at this point in the history
…lied command

: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>
  • Loading branch information
alexey-tikhonov authored and pbrezina committed Aug 16, 2021
1 parent 365cd67 commit 7ab83f9
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 57 deletions.
39 changes: 28 additions & 11 deletions src/tools/sssctl/sssctl.c
Expand Up @@ -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;
Expand All @@ -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

Expand Down
2 changes: 1 addition & 1 deletion src/tools/sssctl/sssctl.h
Expand Up @@ -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);
Expand Down
57 changes: 18 additions & 39 deletions src/tools/sssctl/sssctl_data.c
Expand Up @@ -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;
Expand Down Expand Up @@ -158,17 +158,17 @@ 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;
}
}

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;
Expand Down Expand Up @@ -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;
}
32 changes: 26 additions & 6 deletions src/tools/sssctl/sssctl_logs.c
Expand Up @@ -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"
Expand Down Expand Up @@ -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[] = {
Expand All @@ -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;
Expand All @@ -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,
Expand All @@ -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;
Expand Down

0 comments on commit 7ab83f9

Please sign in to comment.