xorl %eax, %eax

CVE-2009-1888: SAMBA ACLs Uninitialized Memory Read

with 2 comments

The vulnerability was discovered by Jeremy Allison and disclosed by SAMBA team. Vulnerable releases include 3.0.35, 3.1, 3.2 up tp 3.2.12 and 3.3 up to 3.3.5. The buggy code is located at source/smbd/posix_acls.c. Numerous functions use acl_group_override() routine to check whether they should override a deny. To do so, it checks “acl group control” and “dos filemode”. Here is this function.

static bool acl_group_override(connection_struct *conn,
                                gid_t prim_gid,
                                const char *fname)
{
        SMB_STRUCT_STAT sbuf;

        if ((errno != EPERM) && (errno != EACCES)) {
                return false;
        }

        /* file primary group == user primary or supplementary group */
        if (lp_acl_group_control(SNUM(conn)) &&
                        current_user_in_group(prim_gid)) {
                return true;
        }

        /* user has writeable permission */
        if (lp_dos_filemode(SNUM(conn)) &&
                        can_write_to_file(conn, fname, &sbuf)) {
                return true;
        }

        return false;
}

Variable prim_gid which indicates the group ID and sbuf are uninitialized and thus, if the user is able to modify an access list (which happens only when “dos filemode” directive is set to “yes” in smb.conf), he may manipulate the previous stack frames in order to have predictable values in prim_gid and sbuf and thus bypass the access control list. The read of the unitialized memory appears when it calls current_user_in_group() and also, when it’s looking for write permissions using can_write_to_file(). This was fixed with the following patch.

 static bool acl_group_override(connection_struct *conn,
-				gid_t prim_gid,
+				SMB_STRUCT_STAT *psbuf,
 				const char *fname)
 {
-	SMB_STRUCT_STAT sbuf;
-
 	if ((errno != EPERM) && (errno != EACCES)) {
 		return false;
 	}
 
 	/* file primary group == user primary or supplementary group */
 	if (lp_acl_group_control(SNUM(conn)) &&
-			current_user_in_group(prim_gid)) {
+			current_user_in_group(psbuf->st_gid)) {
 		return true;
 	}
 
 	/* user has writeable permission */
 	if (lp_dos_filemode(SNUM(conn)) &&
-			can_write_to_file(conn, fname, &sbuf)) {
+			can_write_to_file(conn, fname, psbuf)) {
 		return true;
 	}

Variables prim_gid and sbuf were removed and calls to current_user_in_group() and can_write_to_file() were updated in order to include only valid, initialized variables. In addition, set_canon_ace_list() and set_nt_acl() were changed so that they do not make use of the uninitialized variables when invoking acl_group_override().

Written by xorl

June 26, 2009 at 00:56

Posted in bugs

2 Responses

Subscribe to comments with RSS.

  1. Thank you myfriend of the information
    I visit your blog every day,You are the best
    keep it rock

    sec-root

    June 27, 2009 at 00:56

  2. Yes, diff are more relevant when post on a blog.
    Keep the good work.

    Vincent

    June 28, 2009 at 12:53


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