| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
A bug has been reported recently for the mount.cifs utility which is
part of the cifs-utils package. The tool has a shell injection issue
where one can embed shell commands via the username mount option. Those
commands will be run via popen() in the context of the user calling
mount.
The bug requires cifs-utils to be built with --with-systemd (enabled
by default if supported).
A quick test to check if the mount.cifs binary is vulnerable is to look
for popen() calls like so:
$ nm mount.cifs | grep popen
U popen@@GLIBC_2.2.5
If the user is allowed to run mount.cifs via sudo, he can obtain a root
shell.
sudo mount.cifs -o username='`sh`' //1 /mnt
If mount.cifs has the setuid bit, the command will still be run as the
calling user (no privilege escalation).
The bug was introduced in June 2012 with commit 4e264031d0da7d3f2
("mount.cifs: Use systemd's mechanism for getting password, if
present.").
Affected versions:
cifs-utils-5.6
cifs-utils-5.7
cifs-utils-5.8
cifs-utils-5.9
cifs-utils-6.0
cifs-utils-6.1
cifs-utils-6.2
cifs-utils-6.3
cifs-utils-6.4
cifs-utils-6.5
cifs-utils-6.6
cifs-utils-6.7
cifs-utils-6.8
cifs-utils-6.9
cifs-utils-6.10
Bug: https://bugzilla.samba.org/show_bug.cgi?id=14442
Reported-by: Vadim Lebedev <vadim@mbdsys.com>
Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
Signed-off-by: Aurelien Aptel <aaptel@suse.com>
|
|
As we are supporting mount.smb3 to be invoked, the error output
should contain the called program and not mount.cifs
Signed-off-by: Kenneth D'souza <kdsouza@redhat.com>
|
|
As we will slowly move towards smb3 filesystem,
supporting through "mount -t smb3" is important.
Signed-off-by: Kenneth D'souza <kdsouza@redhat.com>
|
|
When attemping to chdir into non-existing directories, mount.cifs
crashes.
This patch fixes the following ASAN report:
$ ./mount.cifs //localhost/foo /mnt/invalid-dir -o ...
/mnt/bar -o username=foo,password=foo,vers=1.0
Couldn't chdir to /mnt/bar: No such file or directory
=================================================================
==11846==ERROR: AddressSanitizer: attempting free on address which was
not malloc()-ed: 0x7ffd86332e97 in thread T0
#0 0x7f0860ca01e7 in
__interceptor_free (/usr/lib64/libasan.so.5+0x10a1e7)
#1 0x557edece9ccb in
acquire_mountpoint (/home/paulo/src/cifs-utils/mount.cifs+0xeccb)
#2 0x557edecea63d in
main (/home/paulo/src/cifs-utils/mount.cifs+0xf63d)
#3 0x7f08609f0bca in __libc_start_main (/lib64/libc.so.6+0x26bca)
#4 0x557edece27d9 in
_start (/home/paulo/src/cifs-utils/mount.cifs+0x77d9)
Address 0x7ffd86332e97 is located in stack of thread T0 at offset 8951
in frame
#0 0x557edece9ce0 in
main (/home/paulo/src/cifs-utils/mount.cifs+0xece0)
This frame has 2 object(s):
[48, 52) 'rc' (line 1959)
[64, 72) 'mountpoint' (line 1955) <== Memory access at offset 8951
overflows this variable
HINT: this may be a false positive if your program uses some custom
stack unwind mechanism, swapcontext or vfork
(longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: bad-free (/usr/lib64/libasan.so.5+0x10a1e7)
in __interceptor_free
==11846==ABORTING
Fixes: bf7f48f4c7dc ("mount.cifs.c: fix memory leaks in main func")
Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
Reviewed-by: David Mulder <dmulder@suse.com>
|
|
It can be easily reproduced with the following:
# chmod +s `which mount.cifs`
# echo "//localhost/share /mnt cifs \
users,username=foo,password=XXXX" >> /etc/fstab
# su - foo
$ mount /mnt
free(): double free detected in tcache 2
Child process terminated abnormally.
The problem was that check_fstab() already freed orgoptions pointer
and then we freed it again in main() function.
Fixes: bf7f48f4c7dc ("mount.cifs.c: fix memory leaks in main func")
Signed-off-by: Paulo Alcantara (SUSE) <paulo@paulo.ac>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>
|
|
In mount.cifs module, orgoptions and mountpoint in the main func
point to the memory allocated by func realpath and strndup respectively.
However, they are not freed before the main func returns so that the
memory leaks occurred.
The memory leak problem is reported by LeakSanitizer tool.
LeakSanitizer url: "https://github.com/google/sanitizers"
Here I free the pointers orgoptions and mountpoint before main
func returns.
Fixes：7549ad5e7126 ("memory leaks: caused by func realpath and strndup")
Signed-off-by: Jiawen Liu <liujiawen10@huawei.com>
Reported-by: Jin Du <dujin1@huawei.com>
Reviewed-by: Saisai Zhang <zhangsaisai@huawei.com>
Reviewed-by: Aurélien Aptel <aaptel@suse.com>
|
|
In order to provide an easy way to access snapshots a GMT
token string should be allowed as a "snapshot" mount option
argument, not SMB 100-nanoseconds time only. Detect if the
argument is in GMT format and convert it to SMB 100-nanoseconds
time before passing to the kernel.
Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com>
Reviewed-by: Paulo Alcantara <palcantara@suse.de>
|
|
Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com>
Reviewed-by: Paulo Alcantara <palcantara@suse.de>
|
|
When you type mount.cifs --help there were more than 40 mount parms
missing. Add 12 of the more common ones to what is displayed by help.
Signed-off-by: Steve French <stfrench@microsoft.com>
Reviewed-by: Paulo Alcantara <palcantara@suse.de>
|
|
Signed-off-by: Aurelien Aptel <aaptel@suse.com>
|
|
...to silence a couple of compiler warnings.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
Signed-off-by: Aurelien Aptel <aaptel@suse.com>
|
|
data_blob.h includes talloc.h from libtalloc, but that is only marked as
a dependency for cifs.upcall. No symbols from that header are used by
cifs.mount, so remove it to avoid the libtalloc dependency
Signed-off-by: Thomas Witt <pyromaniac@exherbo.org>
|
|
It just frees and then zeroes out the pointer. That's of dubious
value in the places where it's currently being used. Just use
free() instead.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
Signed-off-by: Germano Percossi <germano.percossi@citrix.com>
|
|
The way token matching was done was consuming the parameters namespace
quickly. For example, anything starting with "dom" was interpreted with
domain, while it could have been a completely different word. The same
is true even for "ro".
Moreover, many perfectly valid options like "addr" where not accepted.
The cifs kernel module is very strict when it comes to names: 'dom' and
'domain' are valid while 'domai' is not, so the userspace tool needs to
comply otherwise it becomes very difficult to come up with new names for
options.
Now, checking is strict and as close as possible to kernel. When it is
not, it is just to avoid breaking compatibility with some users.
However, workg has been removed because it is too lazy and undocumented.
The only variable left without strict checking is 'x-' because the
intent is to ignore anything starting in that way
Signed-off-by: Germano Percossi <germano.percossi@citrix.com>
|
|
If we do not allow empty domains on the command line we are preventing
the kernel module from taking different actions if the domain has not
been specified at all or just passed empty.
In fact, with this fix the cifs module behaves differently once an empty
domain is passed: the find_domain_name function is not invoked when an
empty domain is passed.
It is possible to pass both 'domain=' or 'domain=""' even though the
kernel module will accept the former only when associated with the
sloppy option.
Signed-off-by: Germano Percossi <germano.percossi@citrix.com>
|
|
Signed-off-by: Germano Percossi <germano.percossi@citrix.com>
|
|
x-* prefix is used for userspace mount options and it's pretty
commonly used to extend fstab configuration in systemd world (e.g.
x-systemd.automount). These options is necessary to ignored.
The command mount(8) does not pass x-* mount options to mount.<type>
helpers, but in some use-cases it's possible that the cifs helper reads
mount options from fstab or users directly call mount.cifs and copy & past
mount options, etc.
This patch marks all options prefixed by "x-" as OPT_IGNORE to make
things more robust for end-users. We already uses the same concept for
_netdev.
Signed-off-by: Karel Zak <kzak@redhat.com>
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
Recent kernels now ignore "unc=..." mount option. mount.cifs, when
getting errno=ENXIO, retries the mount with uppercased hostname,
sharename and prefixpath in the "unc=..." mount option, which is ignored
now in the kernel. Used e.g. during OS/2 mounts, which fail now.
Also uppercase the now used "orig_dev" parameter.
Signed-off-by: Guenter Kukkukk <kukks@samba.org>
|
|
Coverity says:
Error: CPPCHECK_WARNING: [#def10]
cifs-utils-6.2/mount.cifs.c:1518: error[memleakOnRealloc]: Common realloc mistake: 'mtabdir' nulled but not freed upon failure
del_mtab has a number of bugs in handling of allocated memory:
a) the return value of strdup() is not checked
b) It calls realloc() on a pointer that wasn't returned by an allocation
function (e.g. malloc, calloc, etc.)
c) If realloc() fails, it doesn't call free() on the original memory
returned by strdup()
Fix all of these bugs and add newlines to the end of the error messages
in del_mtab.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
Relying on hardcoded /bin/systemd-ask-password path breaks systemd that
install systemd-ask-password in /usr/bin. Since both paths are supposed
to be in ${PATH} and popen() passes the command to shell, just pass
'systemd-ask-password' and let the shell find it.
Fixes: https://bugzilla.samba.org/show_bug.cgi?id=10054
Signed-off-by: Michał Górny <mgorny@gentoo.org>
|
|
The max size of the username, domain, and password strings are now
consistent with the kernel and Microsoft's documentation.
Signed-off-by: Scott Lovenberg <scott.lovenberg@gmail.com>
|
|
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
Two trivial comment fixes.
Signed-off-by: Scott Lovenberg <scott.lovenberg@gmail.com>
|
|
...as promised for version 6.0.
Cc: Scott Lovenberg <scott.lovenberg@gmail.com>
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
commit 85d18a1ed introduced a regression when using a credentials file.
It set the username in the parsed mount info properly, but didn't set
the "got_user" flag in it.
Also, fix an incorrect strlcpy length specifier in open_cred_file.
Reported-by: "Mantas M." <grawity@gmail.com>
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
In commit 569cfcb3a, we added a warning of the removal for support for
username= options in the form of DOMAIN/username%password. This patch
removes that support as promised prior to the 5.9 release.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
When certain options are passed to the mount helper, we want to turn
them into mountflags for the mount() syscall. There's no need to copy
them to the options string in that case though.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
number
Sergio Conrad reported a problem trying to set up an autofs map to do
a krb5 mount. In his environment, many users have usernames that are
comprised entirely of numbers. While that's a bit odd, POSIX apparently
allows for it.
The current code assumes that when a numeric argument is passed to one
of the above options, that it's a uid or gid. Instead, try to treat the
argument as a user or group name first, and only try to treat it as a
number if that fails.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
The argv < 3 check could return true if you pass in some option flags.
If you don't provide any further arguments then you might just walk
off the end of the argv array. The values past the end aren't
guaranteed to be NULL in that case.
Fix the check to just look at whether there are 2 more arguments after
the getopt processing is done.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
removed in cifs-utils-6.0.
[jlayton: Added newline to end of warning]
Signed-off-by: Scott Lovenberg <scott.lovenberg@gmail.com>
|
|
The mount(8) manpage lists this as a fs-independent option:
nofail: Do not report errors for this device if it does not exist.
Implement that in mount.cifs by not returning an error if we were unable
to find a suitable address for the mount attempt.
Reported-by: Peter Trenholme <PTrenholme@gmail.com>
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
This patch fixes a minor regression. It used to be that when the mount
helper would run out of addresses that it would return EX_FAIL to
userspace. It now returns EX_SYSERR which is incorrect. Reinstate
the correct error code.
Reported-by: Ales Zelinka <azelinka@redhat.com>
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
In this case we explicitly don't care what these functions return, so
declare a couple of unused variables to catch the results.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
This patch is intended as a temporary workaround for krb5 users that need
to specify usernames with '/' in them. I intend to remove this hack from
mount.cifs once the legacy username handling code is removed.
The idea here is to save off the raw username string while we're parsing
options. If the mount options specify "sec=krb5" or "sec=krb5i" then
we'll not do the legacy username parsing and will instead just pass in
the username string as-is.
Obviously, this is a nasty hack and we don't really want to carry this
in perpetuity, so this can go away once the "legacy" username parsing
has gone away.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
mount.cifs has in the past allowed users to specify a username using
the above syntax, which would populate the domain and password fields
with the different pieces.
Unfortunately, there are cases where it is legit to have a '/' in a
username. krb5 SPNs generally contain a '/' and we have no clear way
to distinguish between the two.
I don't see any real value in keeping that syntax allowed. It's no
easier than specifying "pass=" and "domain=" on the command line. Ditto
for credential files.
Begin the transition away from that syntax by adding a warning message
that support for it will be removed in 5.9.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
When access() fails, use errno for a sensible error message.
Signed-off-by: Luk Claes <luk@debian.org>
|
|
If systemd is running and /bin/systemd-ask-password if available,
then use that else fallback on getpass(..).
And add a --enable-systemd configure option, which defaults to yes.
Signed-off-by: Ankit Jain <jankit@suse.com>
|
|
Thus spake Jochen:
The mount.cifs program from the cifs-utils package 5.5 did not work on
my Linux system. It just exited without an error message and did not
mount anything.
[...]
I think, when this variable rc is now used in this function, it has also
to be properly initialized there.
Reported-by: Jochen Roderburg <roderburg@uni-koeln.de>
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
|
|
This was added when it was thought that smb2 would be a different fstype
altogether. Now that we are not adding a separate fstype, this code is
no longer needed since nothing will ever call /sbin/mount.smb2.
Cc: Steve French <smfrench@gmail.com>
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
Traditionally, this ver= option was used to specify the "options
version" that we're passing in. It has always been set to '1' though
and we have never changed that.
Eventually we want to have a ver= (or vers=) option that allows users
to specify the SMB version that they want to use to talk to the server.
At that point, this option will just get in the way. Let's go ahead
and remove it now in preparation for that day.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
We handle this option in userspace, so there's little value in also
passing it to the kernel.
Also fix minor double-comma nit in the options string.
Reported-by: Ronald <ronald645@gmail.com>
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
toggle_dac_capability
I'm not sure what I was thinking when I added that check in, but it's
been there since the inception. We shouldn't care at all what the
real uid is when we call toggle_dac_capability and indeed we don't
care with the libcap-ng version. Remove that check.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
the build process of the cifs-utils for Mandriva 2011 made me notice of
the unused variable rc in toggle_dac_capability() of mount.cifs.c.
A bit up in the code we store the return value and do not make use of it
while calling return.
The attached patch intends to fix this.
The failing build result is still visible at
https://build.opensuse.org/package/live_build_log?arch=x86_64&package=cifs-utils&project=network%3Asamba%3ASTABLE&repository=Mandriva_2011
Acked-by: Suresh Jayaraman <sjayaraman@suse.com>
Signed-off-by: Lars Mueller <lmuelle@suse.com>
|
|
older gcc versions (4.3 in the case of SUSE Linux Enterprise 11 SP 1 and
SP 2) complain about uninitialized variables in the recent 5.4 release.
The attached patch makes the build process a bit quieter.
Acked-by: Suresh Jayaraman <sjayaraman@suse.com>
Signed-off-by: Lars Mueller <lmuelle@suse.com>
|
|
...and add -D_FORTIFY_SOURCE=2 to the default $CFLAGS.
Acked-by: Acked-by: Suresh Jayaraman <sjayaraman@suse.com>
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
can't chdir
If mount.cifs is installed as a setuid root program, then a user can
use it to gather information about files and directories to which he
does not have access.
One of the first things that mount.cifs does is to chdir() into the
mountpoint and then proceeds to perform the mount onto ".". A malicious
user could exploit this fact to determine information about directories
to which he does not have access. Specifically, whether the dentry in
question is a file or directory and whether it exists at all.
This patch fixes this by making the program switch the fsuid to the
real uid for unprivileged users when mounting.
Note that this is a behavior change. mount.cifs has in the past allowed
users to mount onto any directory as long as it's listed in /etc/fstab
as a user mount. With this change, the user must also be able to chdir
into the mountpoint without needing special privileges. Hopefully not
many people have such a pathological configuration.
This patch should fix CVE-2012-1586.
Reported-by: Jesus Olmos <jesus.olmos@blueliv.com>
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
autofs generally calls mount helpers with '-s'. Handle that the same
way we do for NFS -- append ",sloppy" option to the mount options.
The kernel can look for that option to decide whether to ignore
unknown mount options, warn, or error out.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
