xorl %eax, %eax

OpenSolaris CIFS/SMB Invalid File Flags

with 2 comments

This bug was disclosed by Sun on 10 January 2009 and affects OpenSolaris prior to build snv_111 for both SPARC and x86 architectures. It does not affect Solaris at all. The bug appears in mount operations of SMB/CIFS filesystems. Here is the code of mount located at usr/src/cmd/fs.d/smbclnt/mount/mount.c.

* Copyright (c) 2000-2001, Boris Popov
* All rights reserved.
     ...
int
main(int argc, char *argv[])
{
	struct smb_ctx sctx, *ctx = &sctx;
	struct smbfs_args mdata;
	struct stat st;
	int opt, error, mntflags;
	struct mnttab mnt;
	struct mnttab *mntp = &mnt;
	char optbuf[MAX_MNTOPT_STR];
	static char *fstype = MNTTYPE_SMBFS;
     ...
        if (stat(mount_point, &st) == -1)
		err(EX_OSERR, gettext("could not find mount point %s"),
		    mount_point);
     ...
        /*
         * Darwin takes defaults from the
         * mounted-on directory.
         * We want the real uid/gid.
         * XXX: Is this correct?
         */
#ifdef __sun
                 if (mdata.uid == (uid_t)-1)
		mdata.uid = getuid();
	if (mdata.gid == (gid_t)-1)
		mdata.gid = getgid();
#else
	if (mdata.uid == (uid_t)-1)
		mdata.uid = st.st_uid;
	if (mdata.gid == (gid_t)-1)
		mdata.gid = st.st_gid;
#endif
	if (mdata.file_mode == 0)
		mdata.file_mode = st.st_mode & (S_IRWXU | S_IRWXG | S_IRWXO);
     ...
   return (0);
}

The first problem is quite obvious. During the mdata.uid and mdata.gid initialization, if __sun is not set, it will jump to the else clause where it initiallizes the UID and GID with those stored in the mount point options and retrieved using stat() earlier. Clearly this is something really insecure and it was removedlike this:

-       /*
-        * Darwin takes defaults from the
+        * Fill in mdata defaults.
-         * mounted-on directory.
-         * We want the real uid/gid.
-        * XXX: Is this correct?
-        */
-#ifdef __sun
        if (mdata.uid == (uid_t)-1)
		mdata.uid = getuid();
	if (mdata.gid == (gid_t)-1)
		mdata.gid = getgid();
-#else
-	if (mdata.uid == (uid_t)-1)
-		mdata.uid = st.st_uid;
-	if (mdata.gid == (gid_t)-1)
-		mdata.gid = st.st_gid;
-#endif
-

Now, the bug discussed by their advisory is in the initialization of the mode which is next. If its mode is 0, it is initialized to S_IRWXU | S_IRWXG | S_IRWXO. That is, read, write and execute for the owner, read, write and execute for the group and read write and execute for the others. Now, depending on the umask this can result in a world readable filesystem. This was fixed by applying this patch:

	if (mdata.file_mode == 0)
-		mdata.file_mode = st.st_mode & (S_IRWXU | S_IRWXG | S_IRWXO);
+               mdata.file_mode = S_IRWXU;
        if (mdata.dir_mode == 0) {

This way, only the owner’s flags are affected.

Written by xorl

June 14, 2009 at 03:10

Posted in bugs, solaris

2 Responses

Subscribe to comments with RSS.

  1. WordPress ate your ampersands.

    egypt

    June 15, 2009 at 05:55

  2. Oh.. thanks for that. Fixed. :)

    xorl

    June 15, 2009 at 11:23


Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s