[Bug 14442] New: Shell command injection vulnerability in mount.cifs

[Bug 14442] New: Shell command injection vulnerability in mount.cifs

[samba-bugs]

https://bugzilla.samba.org/show_bug.cgi?id=14442

            Bug ID: 14442
           Summary: Shell command injection vulnerability in mount.cifs
           Product: CifsVFS
           Version: 2.4
          Hardware: All
                OS: Linux
            Status: NEW
          Severity: major
          Priority: P5
         Component: kernel fs
          Assignee: sfrench@samba.org
          Reporter: vadim@mbdsys.com
        QA Contact: cifs-qa@samba.org
  Target Milestone: ---

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

-- 
You are receiving this mail because:
You are the QA Contact for the bug.

[Bug 14442] Shell command injection vulnerability in mount.cifs

[samba-bugs]

https://bugzilla.samba.org/show_bug.cgi?id=14442

--- Comment #1 from Paulo Alcantara <palcantara@suse.de> ---
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.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.

[Bug 14442] Shell command injection vulnerability in mount.cifs

[samba-bugs]

https://bugzilla.samba.org/show_bug.cgi?id=14442

--- Comment #2 from Vadim Lebedev <vadim@mbdsys.com> ---
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?

-- 
You are receiving this mail because:
You are the QA Contact for the bug.

[Bug 14442] Shell command injection vulnerability in mount.cifs

[samba-bugs]

https://bugzilla.samba.org/show_bug.cgi?id=14442

--- Comment #3 from Lancelot Bogard <lancelot.bogard@orange.com> ---
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

-- 
You are receiving this mail because:
You are the QA Contact for the bug.

[Bug 14442] Shell command injection vulnerability in mount.cifs

[samba-bugs]

https://bugzilla.samba.org/show_bug.cgi?id=14442

--- Comment #4 from Aurélien Aptel <aaptel@samba.org> ---
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?

-- 
You are receiving this mail because:
You are the QA Contact for the bug.

[Bug 14442] Shell command injection vulnerability in mount.cifs

[samba-bugs]

https://bugzilla.samba.org/show_bug.cgi?id=14442

--- Comment #5 from Paulo Alcantara <pc@cjr.nz> ---
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

-- 
You are receiving this mail because:
You are the QA Contact for the bug.

[Bug 14442] Shell command injection vulnerability in mount.cifs

[samba-bugs]

https://bugzilla.samba.org/show_bug.cgi?id=14442

--- Comment #6 from Vadim Lebedev <vadim@mbdsys.com> ---
(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

-- 
You are receiving this mail because:
You are the QA Contact for the bug.

[Bug 14442] Shell command injection vulnerability in mount.cifs

[samba-bugs]

https://bugzilla.samba.org/show_bug.cgi?id=14442

Jim McDonough <jmcd@samba.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Group|                            |samba-devel
                 CC|                            |jmcd@samba.org, pc@cjr.nz

-- 
You are receiving this mail because:
You are the QA Contact for the bug.

[Bug 14442] Shell command injection vulnerability in mount.cifs

[samba-bugs]

https://bugzilla.samba.org/show_bug.cgi?id=14442

--- Comment #7 from Marcus Meissner <meissner@suse.de> ---
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.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.

[Bug 14442] Shell command injection vulnerability in mount.cifs

[samba-bugs]

https://bugzilla.samba.org/show_bug.cgi?id=14442

--- Comment #8 from Aurélien Aptel <aaptel@samba.org> ---
I'm preparing CVE request.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.

[Bug 14442] Shell command injection vulnerability in mount.cifs

[samba-bugs]

https://bugzilla.samba.org/show_bug.cgi?id=14442

--- Comment #9 from Vadim Lebedev <vadim@mbdsys.com> ---
(In reply to Marcus Meissner from comment #7)
This was EXACTLY the context where we discovered it

-- 
You are receiving this mail because:
You are the QA Contact for the bug.

[Bug 14442] CVE-2020-14342: Shell command injection vulnerability in mount.cifs

[samba-bugs]

https://bugzilla.samba.org/show_bug.cgi?id=14442

Aurélien Aptel <aaptel@samba.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Shell command injection     |CVE-2020-14342: Shell
                   |vulnerability in mount.cifs |command injection
                   |                            |vulnerability in mount.cifs

--- Comment #10 from Aurélien Aptel <aaptel@samba.org> ---
This bug was assigned CVE-2020-14342.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.

[Bug 14442] CVE-2020-14342: Shell command injection vulnerability in mount.cifs

[samba-bugs]

https://bugzilla.samba.org/show_bug.cgi?id=14442

--- Comment #14 from Aurélien Aptel <aaptel@samba.org> ---
Created attachment 16138
  --> https://bugzilla.samba.org/attachment.cgi?id=16138&action=edit
patch v2 for 6.2-6.10

new version of patch
+ checking non-zero len
- dont add mount.cifs binary... (oops)

-- 
You are receiving this mail because:
You are the QA Contact for the bug.

[Bug 14442] CVE-2020-14342: Shell command injection vulnerability in mount.cifs

[samba-bugs]

https://bugzilla.samba.org/show_bug.cgi?id=14442

--- Comment #15 from Aurélien Aptel <aaptel@samba.org> ---
Created attachment 16139
  --> https://bugzilla.samba.org/attachment.cgi?id=16139&action=edit
patch v2 for 5.6-6.1

-- 
You are receiving this mail because:
You are the QA Contact for the bug.

[Bug 14442] CVE-2020-14342: Shell command injection vulnerability in mount.cifs

[samba-bugs]

https://bugzilla.samba.org/show_bug.cgi?id=14442

--- Comment #16 from Aurélien Aptel <aaptel@samba.org> ---
Created attachment 16140
  --> https://bugzilla.samba.org/attachment.cgi?id=16140&action=edit
bug annoucement

-- 
You are receiving this mail because:
You are the QA Contact for the bug.

[Bug 14442] CVE-2020-14342: Shell command injection vulnerability in mount.cifs

[samba-bugs]

https://bugzilla.samba.org/show_bug.cgi?id=14442

Aurélien Aptel <aaptel@samba.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #16137|1                           |0
         is private|                            |

-- 
You are receiving this mail because:
You are the QA Contact for the bug.

[Bug 14442] CVE-2020-14342: Shell command injection vulnerability in mount.cifs

[samba-bugs]

https://bugzilla.samba.org/show_bug.cgi?id=14442

--- Comment #17 from Björn Jacke <bjacke@samba.org> ---
The content of attachment 16135 has been deleted for the following reason:

accidental binary upload

-- 
You are receiving this mail because:
You are the QA Contact for the bug.

[Bug 14442] CVE-2020-14342: Shell command injection vulnerability in mount.cifs

[samba-bugs]

https://bugzilla.samba.org/show_bug.cgi?id=14442

Aurélien Aptel <aaptel@samba.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #16138|0                           |1
        is obsolete|                            |

--- Comment #18 from Aurélien Aptel <aaptel@samba.org> ---
Created attachment 16148
  --> https://bugzilla.samba.org/attachment.cgi?id=16148&action=edit
patch v3 for 6.2-6.10

add two memset() to handle empty input and/or bad systemd-ask-password call

-- 
You are receiving this mail because:
You are the QA Contact for the bug.

[Bug 14442] CVE-2020-14342: Shell command injection vulnerability in mount.cifs

[samba-bugs]

https://bugzilla.samba.org/show_bug.cgi?id=14442

Aurélien Aptel <aaptel@samba.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #16139|0                           |1
        is obsolete|                            |

--- Comment #19 from Aurélien Aptel <aaptel@samba.org> ---
Created attachment 16149
  --> https://bugzilla.samba.org/attachment.cgi?id=16149&action=edit
patch v3 for 5.6-6.1

-- 
You are receiving this mail because:
You are the QA Contact for the bug.