Include _ntfs_valid_descr()
authorEric Biggers <ebiggers3@gmail.com>
Fri, 24 Aug 2012 21:18:52 +0000 (16:18 -0500)
committerEric Biggers <ebiggers3@gmail.com>
Fri, 24 Aug 2012 21:18:52 +0000 (16:18 -0500)
Patch the ntfs_valid_descr() function from libntfs-3g to allow security
descriptors that end with an empty SACL

src/ntfs-3g_security.c
src/ntfs-apply.c
src/security.c

index a508be4542eb9b598a21c86a2b3da72a11aad6b6..83b92ed99a247ca762d4804e90adc56e972389c9 100644 (file)
@@ -22,7 +22,7 @@
  * Foundation,Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
-
+#include "util.h"
 
 #ifdef HAVE_CONFIG_H
 #include "config.h"
@@ -4681,6 +4681,125 @@ int ntfs_set_file_security(struct SECURITY_API *scapi,
        return (res);
 }
 #endif
+/*
+ *             Check the validity of the ACEs in a DACL or SACL
+ */
+
+static BOOL valid_acl(const ACL *pacl, unsigned int end)
+{
+       const ACCESS_ALLOWED_ACE *pace;
+       unsigned int offace;
+       unsigned int acecnt;
+       unsigned int acesz;
+       unsigned int nace;
+       BOOL ok;
+
+       ok = TRUE;
+       acecnt = le16_to_cpu(pacl->ace_count);
+       offace = sizeof(ACL);
+       for (nace = 0; (nace < acecnt) && ok; nace++) {
+               /* be sure the beginning is within range */
+               if ((offace + sizeof(ACCESS_ALLOWED_ACE)) > end)
+                       ok = FALSE;
+               else {
+                       pace = (const ACCESS_ALLOWED_ACE*)
+                               &((const char*)pacl)[offace];
+                       acesz = le16_to_cpu(pace->size);
+                       if (((offace + acesz) > end)
+                          || !ntfs_valid_sid(&pace->sid)
+                          || ((ntfs_sid_size(&pace->sid) + 8) != (int)acesz))
+                                ok = FALSE;
+                       offace += acesz;
+               }
+       }
+       return (ok);
+}
+
+/*
+ *             Do sanity checks on security descriptors read from storage
+ *     basically, we make sure that every field holds within
+ *     allocated storage
+ *     Should not be called with a NULL argument
+ *     returns TRUE if considered safe
+ *             if not, error should be logged by caller
+ */
+static BOOL _ntfs_valid_descr(const char *securattr, unsigned int attrsz)
+{
+       const SECURITY_DESCRIPTOR_RELATIVE *phead;
+       const ACL *pdacl;
+       const ACL *psacl;
+       unsigned int offdacl;
+       unsigned int offsacl;
+       unsigned int offowner;
+       unsigned int offgroup;
+       BOOL ok;
+
+       ok = TRUE;
+
+       /*
+        * first check overall size if within allocation range
+        * and a DACL is present
+        * and owner and group SID are valid
+        */
+
+       phead = (const SECURITY_DESCRIPTOR_RELATIVE*)securattr;
+       offdacl = le32_to_cpu(phead->dacl);
+       offsacl = le32_to_cpu(phead->sacl);
+       offowner = le32_to_cpu(phead->owner);
+       offgroup = le32_to_cpu(phead->group);
+       pdacl = (const ACL*)&securattr[offdacl];
+       psacl = (const ACL*)&securattr[offsacl];
+
+               /*
+                * size check occurs before the above pointers are used
+                *
+                * "DR Watson" standard directory on WinXP has an
+                * old revision and no DACL though SE_DACL_PRESENT is set
+                */
+       if ((attrsz >= sizeof(SECURITY_DESCRIPTOR_RELATIVE))
+               && (phead->revision == SECURITY_DESCRIPTOR_REVISION)
+               && (offowner >= sizeof(SECURITY_DESCRIPTOR_RELATIVE))
+               && ((offowner + 2) < attrsz)
+               && (offgroup >= sizeof(SECURITY_DESCRIPTOR_RELATIVE))
+               && ((offgroup + 2) < attrsz)
+               && (!offdacl
+                       || ((offdacl >= sizeof(SECURITY_DESCRIPTOR_RELATIVE))
+                           && (offdacl+sizeof(ACL) <= attrsz)))
+               && (!offsacl
+                       || ((offsacl >= sizeof(SECURITY_DESCRIPTOR_RELATIVE))
+                           && (offsacl+sizeof(ACL) <= attrsz)))
+               && !(phead->owner & const_cpu_to_le32(3))
+               && !(phead->group & const_cpu_to_le32(3))
+               && !(phead->dacl & const_cpu_to_le32(3))
+               && !(phead->sacl & const_cpu_to_le32(3))
+               && (ntfs_attr_size(securattr) <= attrsz)
+               && ntfs_valid_sid((const SID*)&securattr[offowner])
+               && ntfs_valid_sid((const SID*)&securattr[offgroup])
+                       /*
+                        * if there is an ACL, as indicated by offdacl,
+                        * require SE_DACL_PRESENT
+                        * but "Dr Watson" has SE_DACL_PRESENT though no DACL
+                        */
+               && (!offdacl
+                   || ((phead->control & SE_DACL_PRESENT)
+                       && ((pdacl->revision == ACL_REVISION)
+                          || (pdacl->revision == ACL_REVISION_DS))))
+                       /* same for SACL */
+               && (!offsacl
+                   || ((phead->control & SE_SACL_PRESENT)
+                       && ((psacl->revision == ACL_REVISION)
+                           || (psacl->revision == ACL_REVISION_DS))))) {
+                       /*
+                        *  Check the DACL and SACL if present
+                        */
+               if ((offdacl && !valid_acl(pdacl,attrsz - offdacl))
+                  || (offsacl && !valid_acl(psacl,attrsz - offsacl)))
+                       ok = FALSE;
+       } else {
+               ok = FALSE;
+       }
+       return (ok);
+}
 
 /* 
  * Set security data on a NTFS file given an inode
@@ -4708,7 +4827,7 @@ int _ntfs_set_file_security(ntfs_volume *vol, ntfs_inode *ni,
                        && !(phead->control & SE_GROUP_DEFAULTED));
        if (!missing
            && (phead->control & SE_SELF_RELATIVE)
-           && ntfs_valid_descr(attr, attrsz)) {
+           && _ntfs_valid_descr(attr, attrsz)) {
                oldattr = getsecurityattr(vol, ni);
                if (oldattr) {
                        if (mergesecurityattr(
index 9061971548e59ef139cef05c714fe7599ec2384a..f2a79fd0f79cec0d97931fcfc65890df7d801821 100644 (file)
@@ -201,7 +201,12 @@ apply_file_attributes_and_security_data(ntfs_inode *ni,
                wimlib_assert(dentry->security_id < sd->num_entries);
                DEBUG("Applying security descriptor %d to `%s'",
                      dentry->security_id, dentry->full_path_utf8);
-               if (!_ntfs_set_file_security(ni->vol, ni, ~0,
+               u32 selection = OWNER_SECURITY_INFORMATION |
+                               GROUP_SECURITY_INFORMATION |
+                               DACL_SECURITY_INFORMATION  |
+                               SACL_SECURITY_INFORMATION;
+                               
+               if (!_ntfs_set_file_security(ni->vol, ni, selection,
                                             sd->descriptors[dentry->security_id]))
                {
                        ERROR_WITH_ERRNO("Failed to set security data on `%s'",
index bc44b45ae9d3707dea3fe6fb670e47d5c3d326f6..99f09134c18678aeea79a8adf49ea314276129c6 100644 (file)
@@ -178,17 +178,12 @@ u8 *write_security_data(const struct wim_security_data *sd, u8 *p)
        return p;
 }
 
-/* XXX We don't actually do anything with the ACL's yet besides being able to
- * print a few things.  It seems it would be a lot of work to have comprehensive
- * support for all the weird flags and stuff, and Windows PE seems to be okay
- * running from a WIM file that doesn't have any security data at all...  */
-
-static void print_acl(const u8 *p)
+static void print_acl(const u8 *p, const char *type)
 {
        ACL *acl = (ACL*)p;
        TO_LE16(acl->acl_size);
        TO_LE16(acl->acl_count);
-       printf("    [ACL]\n");
+       printf("    [%s ACL]\n", type);
        printf("    Revision = %u\n", acl->revision);
        printf("    ACL Size = %u\n", acl->acl_size);
        printf("    ACE Count = %u\n", acl->ace_count);
@@ -205,12 +200,13 @@ static void print_acl(const u8 *p)
                printf("        SID start = %u\n", to_le32(aaa->sid_start));
                p += hdr->size;
        }
+       putchar('\n');
 }
 
-static void print_sid(const u8 *p)
+static void print_sid(const u8 *p, const char *type)
 {
        SID *sid = (SID*)p;
-       printf("    [SID]\n");
+       printf("    [%s SID]\n", type);
        printf("    Revision = %u\n", sid->revision);
        printf("    Subauthority count = %u\n", sid->sub_authority_count);
        printf("    Identifier authority = ");
@@ -218,6 +214,7 @@ static void print_sid(const u8 *p)
        putchar('\n');
        for (uint i = 0; i < sid->sub_authority_count; i++)
                printf("    Subauthority %u = %u\n", i, to_le32(sid->sub_authority[i]));
+       putchar('\n');
 }
 
 static void print_security_descriptor(const u8 *p, u64 size)
@@ -229,20 +226,20 @@ static void print_security_descriptor(const u8 *p, u64 size)
        TO_LE32(sd->sacl_offset);
        TO_LE32(sd->dacl_offset);
        printf("Revision = %u\n", sd->revision);
-       printf("Security Descriptor Control = %u\n", sd->security_descriptor_control);
+       printf("Security Descriptor Control = %#x\n", sd->security_descriptor_control);
        printf("Owner offset = %u\n", sd->owner_offset);
        printf("Group offset = %u\n", sd->group_offset);
        printf("System ACL offset = %u\n", sd->sacl_offset);
        printf("Discretionary ACL offset = %u\n", sd->dacl_offset);
 
        if (sd->owner_offset != 0)
-               print_sid(p + sd->owner_offset);
+               print_sid(p + sd->owner_offset, "Owner");
        if (sd->group_offset != 0)
-               print_sid(p + sd->group_offset);
+               print_sid(p + sd->group_offset, "Group");
        if (sd->sacl_offset != 0)
-               print_acl(p + sd->sacl_offset);
+               print_acl(p + sd->sacl_offset, "System");
        if (sd->dacl_offset != 0)
-               print_acl(p + sd->dacl_offset);
+               print_acl(p + sd->dacl_offset, "Discretionary");
 }
 
 /*