[SCM] Samba Shared Repository - branch v3-5-test updated
Karolin Seeger
kseeger at samba.org
Mon Mar 8 02:08:59 MST 2010
The branch, v3-5-test has been updated
via e6c856a... mount.cifs: don't allow it to be run as setuid root program
via ae24005... mount.cifs: check for invalid characters in device name and mountpoint
via a60afce... mount.cifs: take extra care that mountpoint isn't changed during mount
from cc5e6e6... s3: net_share.c: fix argc handling
http://gitweb.samba.org/?p=samba.git;a=shortlog;h=v3-5-test
- Log -----------------------------------------------------------------
commit e6c856ac84ee18a192edc3e8a6547e2e9387a1b5
Author: Jeff Layton <jlayton at redhat.com>
Date: Tue Jan 26 08:36:11 2010 -0500
mount.cifs: don't allow it to be run as setuid root program
mount.cifs has been the subject of several "security" fire drills due to
distributions installing it as a setuid root program. This program has
not been properly audited for security and the Samba team highly
recommends that it not be installed as a setuid root program at this
time.
To make that abundantly clear, this patch forcibly disables the ability
for mount.cifs to run as a setuid root program. People are welcome to
trivially patch this out, but they do so at their own peril.
A security audit and redesign of this program is in progress and we hope
that we'll be able to remove this in the near future.
Signed-off-by: Jeff Layton <jlayton at redhat.com>
The last 3 patches address bug #6853 (mount.cifs race that allows user to
replace mountpoint with a symlink).
commit ae24005a5a2c165dfd9b859bf1c02b5f7e967be5
Author: Jeff Layton <jlayton at redhat.com>
Date: Tue Jan 26 08:36:03 2010 -0500
mount.cifs: check for invalid characters in device name and mountpoint
It's apparently possible to corrupt the mtab if you pass embedded
newlines to addmntent. Apparently tabs are also a problem with certain
earlier glibc versions. Backslashes are also a minor issue apparently,
but we can't reasonably filter those.
Make sure that neither the devname or mountpoint contain any problematic
characters before allowing the mount to proceed.
Signed-off-by: Jeff Layton <jlayton at redhat.com>
commit a60afceaa71c0c9b53b2ec1014db5d09d777803d
Author: Jeff Layton <jlayton at redhat.com>
Date: Tue Jan 26 08:35:35 2010 -0500
mount.cifs: take extra care that mountpoint isn't changed during mount
It's possible to trick mount.cifs into mounting onto the wrong directory
by replacing the mountpoint with a symlink to a directory. mount.cifs
attempts to check the validity of the mountpoint, but there's still a
possible race between those checks and the mount(2) syscall.
To guard against this, chdir to the mountpoint very early, and only deal
with it as "." from then on out.
Signed-off-by: Jeff Layton <jlayton at redhat.com>
-----------------------------------------------------------------------
Summary of changes:
client/mount.cifs.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 98 insertions(+), 9 deletions(-)
Changeset truncated at 500 lines:
diff --git a/client/mount.cifs.c b/client/mount.cifs.c
index 3baaad7..0b8d5b4 100644
--- a/client/mount.cifs.c
+++ b/client/mount.cifs.c
@@ -43,7 +43,7 @@
#include "mount.h"
#define MOUNT_CIFS_VERSION_MAJOR "1"
-#define MOUNT_CIFS_VERSION_MINOR "13"
+#define MOUNT_CIFS_VERSION_MINOR "14"
#ifndef MOUNT_CIFS_VENDOR_SUFFIX
#ifdef _SAMBA_BUILD_
@@ -89,6 +89,17 @@
#define MAX_ADDRESS_LEN INET6_ADDRSTRLEN
/*
+ * mount.cifs has been the subject of many "security" bugs that have arisen
+ * because of users and distributions installing it as a setuid root program.
+ * mount.cifs has not been audited for security. Thus, we strongly recommend
+ * that it not be installed setuid root. To make that abundantly clear,
+ * mount.cifs now check whether it's running setuid root and exit with an
+ * error if it is. If you wish to disable this check, then set the following
+ * #define to 1, but please realize that you do so at your own peril.
+ */
+#define CIFS_DISABLE_SETUID_CHECK 0
+
+/*
* By default, mount.cifs follows the conventions set forth by /bin/mount
* for user mounts. That is, it requires that the mount be listed in
* /etc/fstab with the "user" option when run as an unprivileged user and
@@ -179,7 +190,7 @@ check_mountpoint(const char *progname, char *mountpoint)
struct stat statbuf;
/* does mountpoint exist and is it a directory? */
- err = stat(mountpoint, &statbuf);
+ err = stat(".", &statbuf);
if (err) {
fprintf(stderr, "%s: failed to stat %s: %s\n", progname,
mountpoint, strerror(errno));
@@ -213,6 +224,29 @@ check_mountpoint(const char *progname, char *mountpoint)
return 0;
}
+#if CIFS_DISABLE_SETUID_CHECK
+static int
+check_setuid(void)
+{
+ return 0;
+}
+#else /* CIFS_DISABLE_SETUID_CHECK */
+static int
+check_setuid(void)
+{
+ if (getuid() && !geteuid()) {
+ printf("This mount.cifs program has been built with the "
+ "ability to run as a setuid root program disabled.\n"
+ "mount.cifs has not been well audited for security "
+ "holes. Therefore the Samba team does not recommend "
+ "installing it as a setuid root program.\n");
+ return 1;
+ }
+
+ return 0;
+}
+#endif /* CIFS_DISABLE_SETUID_CHECK */
+
#if CIFS_LEGACY_SETUID_CHECK
static int
check_fstab(const char *progname, char *mountpoint, char *devname,
@@ -1165,6 +1199,36 @@ static void print_cifs_mount_version(void)
MOUNT_CIFS_VENDOR_SUFFIX);
}
+/*
+ * This function borrowed from fuse-utils...
+ *
+ * glibc's addmntent (at least as of 2.10 or so) doesn't properly encode
+ * newlines embedded within the text fields. To make sure no one corrupts
+ * the mtab, fail the mount if there are embedded newlines.
+ */
+static int check_newline(const char *progname, const char *name)
+{
+ char *s;
+ for (s = "\n"; *s; s++) {
+ if (strchr(name, *s)) {
+ fprintf(stderr, "%s: illegal character 0x%02x in mount entry\n",
+ progname, *s);
+ return EX_USAGE;
+ }
+ }
+ return 0;
+}
+
+static int check_mtab(const char *progname, const char *devname,
+ const char *dir)
+{
+ if (check_newline(progname, devname) == -1 ||
+ check_newline(progname, dir) == -1)
+ return EX_USAGE;
+ return 0;
+}
+
+
int main(int argc, char ** argv)
{
int c;
@@ -1197,6 +1261,9 @@ int main(int argc, char ** argv)
struct sockaddr_in6 *addr6;
FILE * pmntfile;
+ if (check_setuid())
+ return EX_USAGE;
+
/* setlocale(LC_ALL, "");
bindtextdomain(PACKAGE, LOCALEDIR);
textdomain(PACKAGE); */
@@ -1378,6 +1445,14 @@ int main(int argc, char ** argv)
}
/* make sure mountpoint is legit */
+ rc = chdir(mountpoint);
+ if (rc) {
+ fprintf(stderr, "Couldn't chdir to %s: %s\n", mountpoint,
+ strerror(errno));
+ rc = EX_USAGE;
+ goto mount_exit;
+ }
+
rc = check_mountpoint(thisprogram, mountpoint);
if (rc)
goto mount_exit;
@@ -1440,13 +1515,23 @@ int main(int argc, char ** argv)
/* BB save off path and pop after mount returns? */
resolved_path = (char *)malloc(PATH_MAX+1);
- if(resolved_path) {
- /* Note that if we can not canonicalize the name, we get
- another chance to see if it is valid when we chdir to it */
- if (realpath(mountpoint, resolved_path)) {
- mountpoint = resolved_path;
- }
+ if (!resolved_path) {
+ fprintf(stderr, "Unable to allocate memory.\n");
+ rc = EX_SYSERR;
+ goto mount_exit;
+ }
+
+ /* Note that if we can not canonicalize the name, we get
+ another chance to see if it is valid when we chdir to it */
+ if(!realpath(".", resolved_path)) {
+ fprintf(stderr, "Unable to resolve %s to canonical path: %s\n",
+ mountpoint, strerror(errno));
+ rc = EX_SYSERR;
+ goto mount_exit;
}
+
+ mountpoint = resolved_path;
+
if(got_user == 0) {
/* Note that the password will not be retrieved from the
USER env variable (ie user%password form) as there is
@@ -1590,7 +1675,11 @@ mount_retry:
if (verboseflag)
fprintf(stderr, "\n");
- if (!fakemnt && mount(dev_name, mountpoint, "cifs", flags, options)) {
+ rc = check_mtab(thisprogram, dev_name, mountpoint);
+ if (rc)
+ goto mount_exit;
+
+ if (!fakemnt && mount(dev_name, ".", "cifs", flags, options)) {
switch (errno) {
case ECONNREFUSED:
case EHOSTUNREACH:
--
Samba Shared Repository
More information about the samba-cvs
mailing list