mount.cifs command is using "popen" library call in get_password which allows for shell command execution. Example: sudo /bin/mount -t cifs -o username="test \$(id)" //1 /mnt
Hi Vadim, Thanks for the report! I was able to reproduce it and ended up with the following changes: diff --git a/mount.cifs.c b/mount.cifs.c index 40918c18649f..bb8a7e958898 100644 --- a/mount.cifs.c +++ b/mount.cifs.c @@ -1695,6 +1695,43 @@ drop_child_privs(void) return 0; } +#ifdef ENABLE_SYSTEMD +static int get_passwd_by_systemd(const char *prompt, char *input, int capacity) +{ + int fd[2]; + pid_t pid; + int rc; + + if (pipe(fd) == -1) { + fprintf(stderr, "Failed to create pipe: %s\n", strerror(errno)); + return -1; + } + + pid = fork(); + if (pid == -1) { + fprintf(stderr, "Unable to fork: %s\n", strerror(errno)); + return -1; + } + + if (pid == 0) { + close(fd[0]); + dup2(fd[1], STDOUT_FILENO); + execlp("systemd-ask-password", "systemd-ask-password", prompt, NULL); + } + + close(fd[1]); + wait(&rc); + if (!WIFEXITED(rc)) + return 1; + if (read(fd[0], input, capacity) == -1) { + fprintf(stderr, "Failed to read from pipe: %s\n", strerror(errno)); + return 1; + } + + return 0; +} +#endif + /* * If systemd is running and systemd-ask-password -- * is available, then use that else fallback on getpass(..) @@ -1714,27 +1751,11 @@ get_password(const char *prompt, char *input, int capacity) && (lstat("/sys/fs/cgroup/systemd", &b) == 0) && (a.st_dev != b.st_dev); - if (is_systemd_running) { - char *cmd, *ret; - FILE *ask_pass_fp = NULL; - - cmd = ret = NULL; - if (asprintf(&cmd, "systemd-ask-password \"%s\"", prompt) >= 0) { - ask_pass_fp = popen (cmd, "re"); - free (cmd); - } - - if (ask_pass_fp) { - ret = fgets(input, capacity, ask_pass_fp); - pclose(ask_pass_fp); - } - - if (ret) { - int len = strlen(input); - if (input[len - 1] == '\n') - input[len - 1] = '\0'; - return input; - } + if (is_systemd_running && !get_passwd_by_systemd(prompt, input, capacity)) { + int len = strlen(input); + if (input[len - 1] == '\n') + input[len - 1] = '\0'; + return input; } #endif --- Before the patch: $ sudo ./mount.cifs -o username="test \$(id)" //1 /mnt Password for test uid=0(root) gid=0(root) groups=0(root)@//1: (press TAB for no echo) After the patch: $ sudo ./mount.cifs -o username="test \$(id)" //1 /mnt Password for test $(id)@//1: (press TAB for no echo) Let me know what you think.
It's a step in the right direction, but consider the case when systemd-ask-password is a shell script with( #!/bin/sh) I believe the vulnerability will be still present.... Maybe the best way will be to scan the option string for presence of "$(" and prefix the '$' by '\' or abort the operation?
Hello, Thanks Paulo for your patch. It seem to be good for me. @Vadim, I tested the case when systemd-ask-password is a shell script with (#!/bin/sh). All Arguments are sent correctly and not executed by a shell where the bug was. Maybe I'm wrong. Regards, -- Lancelot Bogard
I think Paulo's patch idea is good, it fixes the shell injection issue. Some little changes are needed: - close(fd[0]) after read() - check return code of wait() and execlp() - exit(1) after execlp() regarding Vadim last comment: If you can change systemd-ask-password (it doesn't matter if its a shell script or not) or edit PATH to make it point to something else, no privilege escalation happens as mount.cifs drops setuid privileges in assemble_mountinfo(). You will have the same rights as you had before. But maybe I'm overlooking something, can you show an example scenario?
Did some changes after testing and reviewing with Aurelien: diff --git a/mount.cifs.c b/mount.cifs.c index 40918c18649f..6c98b9432f10 100644 --- a/mount.cifs.c +++ b/mount.cifs.c @@ -1695,6 +1695,73 @@ drop_child_privs(void) return 0; } +#ifdef ENABLE_SYSTEMD +static int get_passwd_by_systemd(const char *prompt, char *input, int capacity) +{ + int fd[2]; + pid_t pid; + int offs = 0; + int rc = 1; + + if (pipe(fd) == -1) { + fprintf(stderr, "Failed to create pipe: %s\n", strerror(errno)); + return 1; + } + + pid = fork(); + if (pid == -1) { + fprintf(stderr, "Unable to fork: %s\n", strerror(errno)); + close(fd[0]); + close(fd[1]); + return 1; + } + if (pid == 0) { + close(fd[0]); + dup2(fd[1], STDOUT_FILENO); + if (execlp("systemd-ask-password", "systemd-ask-password", prompt, NULL) == -1) { + fprintf(stderr, "Failed to execute systemd-ask-password: %s\n", + strerror(errno)); + } + exit(1); + } + + close(fd[1]); + for (;;) { + if (offs+1 >= capacity) { + fprintf(stderr, "Password too long.\n"); + kill(pid, SIGTERM); + rc = 1; + break; + } + rc = read(fd[0], input + offs, capacity - offs); + if (rc == -1) { + fprintf(stderr, "Failed to read from pipe: %s\n", strerror(errno)); + rc = 1; + break; + } + if (!rc) + break; + offs += rc; + input[offs] = '\0'; + } + if (wait(&rc) == -1) { + fprintf(stderr, "Failed to wait child: %s\n", strerror(errno)); + rc = 1; + goto out; + } + if (!WIFEXITED(rc) || WEXITSTATUS(rc)) { + rc = 1; + goto out; + } + + rc = 0; + +out: + close(fd[0]); + return rc; +} +#endif + /* * If systemd is running and systemd-ask-password -- * is available, then use that else fallback on getpass(..) @@ -1714,27 +1781,11 @@ get_password(const char *prompt, char *input, int capacity) && (lstat("/sys/fs/cgroup/systemd", &b) == 0) && (a.st_dev != b.st_dev); - if (is_systemd_running) { - char *cmd, *ret; - FILE *ask_pass_fp = NULL; - - cmd = ret = NULL; - if (asprintf(&cmd, "systemd-ask-password \"%s\"", prompt) >= 0) { - ask_pass_fp = popen (cmd, "re"); - free (cmd); - } - - if (ask_pass_fp) { - ret = fgets(input, capacity, ask_pass_fp); - pclose(ask_pass_fp); - } - - if (ret) { - int len = strlen(input); - if (input[len - 1] == '\n') - input[len - 1] = '\0'; - return input; - } + if (is_systemd_running && !get_passwd_by_systemd(prompt, input, capacity)) { + int len = strlen(input); + if (input[len - 1] == '\n') + input[len - 1] = '\0'; + return input; } #endif
(In reply to Lancelot Bogard from comment #3) I confirm, even if systemd-ask-password is shell script the arguments is not evaluated as command. Example: vadim@sys76:/tmp$ cat ./test.sh #!/bin/bash prompt=$1 echo prompt is $prompt vadim@sys76:/tmp$ set -x; ./test.sh '$(id)'; set +x + ./test.sh '$(id)' prompt is $(id) + set +x vadim@sys76:/tmp$ So it seems the patch fixes the problem definitively
Looks like a valid CVE scenario. (untrusted users might be asked to input their smb sharwe username which is then passed unfiltered into this kind of mount.cifs construct) additionaly to the proposed fixes, perhaps also check for valid characters and abort if you encounter an invalid one.
I'm preparing CVE request.
(In reply to Marcus Meissner from comment #7) This was EXACTLY the context where we discovered it
This bug was assigned CVE-2020-14342.
Created attachment 16138 [details] patch v2 for 6.2-6.10 new version of patch + checking non-zero len - dont add mount.cifs binary... (oops)
Created attachment 16139 [details] patch v2 for 5.6-6.1
Created attachment 16140 [details] bug annoucement
The content of attachment 16135 [details] has been deleted for the following reason: accidental binary upload
Created attachment 16148 [details] patch v3 for 6.2-6.10 add two memset() to handle empty input and/or bad systemd-ask-password call
Created attachment 16149 [details] patch v3 for 5.6-6.1
updating the bug to public and fixed.