| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
When verbose logging is enabled, invalid credentials file lines may be
dumped to stderr. This may lead to information disclosure in particular
conditions when the credentials file given is sensitive and contains '='
signs.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=15026
Signed-off-by: Jeffrey Bencteux <jbe@improsec.com>
Reviewed-by: David Disseldorp <ddiss@suse.de>
|
|
Previous check was true whatever the length of the input string was,
leading to a buffer overflow in the subsequent strcpy call.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=15025
Signed-off-by: Jeffrey Bencteux <jbe@improsec.com>
Reviewed-by: David Disseldorp <ddiss@suse.de>
|
|
@mountpointp is initially set to a statically allocated string in
main(), and if we fail to update it in acquire_mountpoint(), make sure
to set it to NULL and avoid freeing it at mount_exit.
This fixes the following crash
$ mount.cifs //srv/share /mnt/foo/bar -o ...
Couldn't chdir to /mnt/foo/bar: No such file or directory
munmap_chunk(): invalid pointer
Aborted
Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>
|
|
In the current mount.cifs logic, when sudo is used for mount,
uid=0, so the mount command searches for cruid=0 unless explicitly
specified by the user. The user may already have cred cache populated
but mount.cifs would end up searching cred cache for uid=0.
mount.cifs can avoid this confusion by reading the cruid from SUDO_UID
environment variable. If it is set to non-zero, we can make cruid=$SUDO_UID.
However, to maintain backward compatibility, keeping this as a fallback option.
If mount fails with ENOKEY, then retry with this option.
To enable this fallback, I had to make a few minor changes in the flow.
Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>
|
|
Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com>
|
|
The code tries to optimise for the last parameter not needing to update
the position which means that every time a new one is added to the end
by copying and pasting, the string position is not updated.
That makes it impossible to use backup_uid=/backup_gid=/snapshot= after
gid= or snapshot= after backup_gid= because part of the string is
overwritten and contains invalid keys like "gbackup_uid".
Prepare for the next parameter to be added on the end by updating the
position for snapshot= even though it will be unused.
|
|
libcap-ng 0.8.1 tightened the error checking on capng_apply, returning an error
of -4 when trying to update the capability bounding set without having the
CAP_SETPCAP capability to be able to do so. Previous versions of libcap-ng
silently skipped updating the bounding set and only updated the normal
CAPNG_SELECT_CAPS capabilities instead.
Check beforehand whether we have CAP_SETPCAP, in which case we can use
CAPNG_SELECT_BOTH to update both the normal capabilities and the bounding set.
Otherwise, we can at least update the normal capabilities, but refrain from
trying to update the bounding set to avoid getting an error.
Signed-off-by: Jonas Witschel <diabonas@archlinux.org>
|
|
|
|
mount.cifs currently complains about the "comment" option:
CIFS: Unknown mount option "comment=foo"
mount(8) on Linux says:
The command mount does not pass the mount options unbindable,
runbindable, private, rprivate, slave, rslave, shared, rshared,
auto, noauto, comment, x-*, loop, offset and sizelimit to the
mount.<suffix> helpers.
So if mount.cifs decides to re-read /etc/fstab it should ignore the
comment option.
A lot of online posts say to use comment=x-gvfs-show as an option to
have a Linux file manager display a mountpoint for a user mountable
filesystem. While the "comment=" part is superfluous when combined
with an x-* option, the problem is still difficult to debug.
Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>
|
|
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>
|
|
|
