CVE-2009-1888: SAMBA ACLs Uninitialized Memory Read

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().