OpenSolaris CIFS/SMB Invalid File Flags

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.