security.c: Rewrite some code
authorEric Biggers <ebiggers3@gmail.com>
Fri, 17 May 2013 02:29:53 +0000 (21:29 -0500)
committerEric Biggers <ebiggers3@gmail.com>
Fri, 17 May 2013 02:29:53 +0000 (21:29 -0500)
include/wimlib/security.h
src/add_image.c
src/metadata_resource.c
src/security.c
src/verify.c
src/wim.c

index 09d602d471bc23b21a59bfad4da2bf3bff6efa9b..2f51560ffe0a749a4a98010466d4231951f26fdb 100644 (file)
@@ -18,19 +18,18 @@ struct wim_security_data {
        /* The total length of the security data, in bytes.  If there are no
         * security descriptors, this field, when read from the on-disk metadata
         * resource, may be either 8 (which is correct) or 0 (which is
-        * interpreted as 0). */
+        * interpreted as 8). */
        u32 total_length;
 
-       /* The number of security descriptors in the array @descriptors, below.
-        * It is really an unsigned int on-disk, but it must fit into an int
-        * because the security ID's are signed.  (Not like you would ever have
-        * more than a few hundred security descriptors anyway.) */
-       int32_t num_entries;
+       /* The number of security descriptors in the array @descriptors. */
+       u32 num_entries;
 
-       /* Array of sizes of the descriptors in the array @descriptors. */
-       u64 *sizes;
+       /* Array of sizes of the descriptors, in bytes, in the array
+        * @descriptors. */
+       size_t *sizes;
 
-       /* Array of descriptors. */
+       /* Array of pointers to the security descriptors in the
+        * SECURITY_DESCRIPTOR_RELATIVE format. */
        u8 **descriptors;
 };
 
@@ -51,16 +50,17 @@ extern struct wim_security_data *
 new_wim_security_data(void);
 
 extern int
-read_security_data(const u8 metadata_resource[],
-                  u64 metadata_resource_len, struct wim_security_data **sd_p);
-extern void
-print_security_data(const struct wim_security_data *sd);
+read_wim_security_data(const u8 metadata_resource[], size_t
+                      metadata_resource_len, struct wim_security_data **sd_p);
 
 extern u8 *
-write_security_data(const struct wim_security_data * restrict sd,
-                   u8 * restrict p);
+write_wim_security_data(const struct wim_security_data * restrict sd, u8 *
+                       restrict p);
+
+extern void
+print_wim_security_data(const struct wim_security_data *sd);
 
 extern void
-free_security_data(struct wim_security_data *sd);
+free_wim_security_data(struct wim_security_data *sd);
 
 #endif /* _WIMLIB_SECURITY_H */
index 7f337c5d04e032e8b0309c73c918a3b169e9f0de..caffafebd23de4935cd2bd4d84591b8dc1cc779e 100644 (file)
@@ -120,7 +120,7 @@ out_put_image_metadata:
                           wim->lookup_table);
        goto out;
 out_free_security_data:
-       free_security_data(sd);
+       free_wim_security_data(sd);
 out:
        return ret;
 }
index 8bd9734e8c545a4a407fa73f0485a81ecb167502..da0be2885b47ae159577fd353bcf2494a6f0eb9e 100644 (file)
@@ -114,7 +114,7 @@ read_metadata_resource(WIMStruct *w, struct wim_image_metadata *imd)
         * resource of the root dentry. */
 
        wimlib_assert(imd->security_data == NULL);
-       ret = read_security_data(buf, metadata_len, &imd->security_data);
+       ret = read_wim_security_data(buf, metadata_len, &imd->security_data);
        if (ret)
                goto out_free_buf;
 
@@ -181,7 +181,7 @@ read_metadata_resource(WIMStruct *w, struct wim_image_metadata *imd)
 out_free_dentry_tree:
        free_dentry_tree(dentry, NULL);
 out_free_security_data:
-       free_security_data(imd->security_data);
+       free_wim_security_data(imd->security_data);
        imd->security_data = NULL;
 out_free_buf:
        FREE(buf);
@@ -194,7 +194,7 @@ recalculate_security_data_length(struct wim_security_data *sd)
        u32 total_length = sizeof(u64) * sd->num_entries + 2 * sizeof(u32);
        for (u32 i = 0; i < sd->num_entries; i++)
                total_length += sd->sizes[i];
-       sd->total_length = total_length;
+       sd->total_length = (total_length + 7) & ~7;
 }
 
 /* Like write_wim_resource(), but the resource is specified by a buffer of
@@ -280,7 +280,7 @@ write_metadata_resource(WIMStruct *w)
        }
 
        /* Write the security data into the resource buffer */
-       p = write_security_data(sd, buf);
+       p = write_wim_security_data(sd, buf);
 
        /* Write the dentry tree into the resource buffer */
        p = write_dentry_tree(root, p);
index 5b0e81b3363f1b0372f741e4a56aedc52d6c06f4..62b6e8e9eb42cd1693544ad22d593c34988881e9 100644 (file)
@@ -43,21 +43,21 @@ typedef struct _ACE_HEADER {
        u8 flags;
 
        /* Size of the access control entry. */
-       u16 size;
+       le16 size;
 } _packed_attribute ACE_HEADER;
 
 /* Grants rights to a user or group */
 typedef struct _ACCESS_ALLOWED_ACE {
        ACE_HEADER hdr;
-       u32 mask;
-       u32 sid_start;
+       le32 mask;
+       le32 sid_start;
 } _packed_attribute ACCESS_ALLOWED_ACE;
 
 /* Denies rights to a user or group */
 typedef struct _ACCESS_DENIED_ACE {
        ACE_HEADER hdr;
-       u32 mask;
-       u32 sid_start;
+       le32 mask;
+       le32 sid_start;
 } _packed_attribute ACCESS_DENIED_ACE;
 
 typedef struct _SYSTEM_AUDIT_ACE {
@@ -97,10 +97,9 @@ typedef struct _SID {
         * have to be, one of enum sid_authority_value */
        u8  identifier_authority[6];
 
-       u32 sub_authority[0];
+       u32 sub_authority[];
 } _packed_attribute SID;
 
-
 typedef struct _SECURITY_DESCRIPTOR_RELATIVE  {
        /* Example: 0x1 */
        u8 revision;
@@ -128,6 +127,12 @@ typedef struct _SECURITY_DESCRIPTOR_RELATIVE  {
        u32 dacl_offset;
 } _packed_attribute SECURITY_DESCRIPTOR_RELATIVE;
 
+struct wim_security_data_disk {
+       u32 total_length;
+       u32 num_entries;
+       u64 sizes[];
+} _packed_attribute;
+
 /*
  * This is a hack to work around a problem in libntfs-3g.  libntfs-3g validates
  * security descriptors with a function named ntfs_valid_descr().
@@ -140,16 +145,15 @@ typedef struct _SECURITY_DESCRIPTOR_RELATIVE  {
  * the validation in libntfs-3g.
  */
 static void
-empty_sacl_fixup(u8 *descr, u64 *size_p)
+empty_sacl_fixup(SECURITY_DESCRIPTOR_RELATIVE *descr, size_t *size_p)
 {
        /* No-op if no NTFS-3g support, or if NTFS-3g is version 2013 or later
         * */
 #if defined(WITH_NTFS_3G) && !defined(HAVE_NTFS_MNT_RDONLY)
        if (*size_p >= sizeof(SECURITY_DESCRIPTOR_RELATIVE)) {
-               SECURITY_DESCRIPTOR_RELATIVE *sd = (SECURITY_DESCRIPTOR_RELATIVE*)descr;
-               u32 sacl_offset = le32_to_cpu(sd->sacl_offset);
+               u32 sacl_offset = le32_to_cpu(descr->sacl_offset);
                if (sacl_offset == *size_p - sizeof(ACL)) {
-                       sd->sacl_offset = cpu_to_le32(0);
+                       descr->sacl_offset = cpu_to_le32(0);
                        *size_p -= sizeof(ACL);
                }
        }
@@ -163,56 +167,60 @@ new_wim_security_data(void)
 }
 
 /*
- * Reads the security data from the metadata resource.
+ * Reads the security data from the metadata resource of a WIM image.
  *
  * @metadata_resource: An array that contains the uncompressed metadata
- *                             resource for the WIM file.
+ *                             resource for the WIM image.
  * @metadata_resource_len:     The length of @metadata_resource.  It must be at
  *                             least 8 bytes.
- * @sd_p:      A pointer to a pointer to a wim_security_data structure that
+ * @sd_ret:    A pointer to a pointer to a wim_security_data structure that
  *             will be filled in with a pointer to a new wim_security_data
- *             structure on success.
+ *             structure containing the security data on success.
  *
  * Note: There is no `offset' argument because the security data is located at
  * the beginning of the metadata resource.
+ *
+ * Possible errors include:
+ *     WIMLIB_ERR_NOMEM
+ *     WIMLIB_ERR_INVALID_SECURITY_DATA
  */
 int
-read_security_data(const u8 metadata_resource[], u64 metadata_resource_len,
-                  struct wim_security_data **sd_p)
+read_wim_security_data(const u8 metadata_resource[], size_t metadata_resource_len,
+                      struct wim_security_data **sd_ret)
 {
        struct wim_security_data *sd;
-       const u8 *p;
        int ret;
        u64 total_len;
+       u64 sizes_size;
+       u64 size_no_descriptors;
+       const struct wim_security_data_disk *sd_disk;
+       const u8 *p;
 
        wimlib_assert(metadata_resource_len >= 8);
 
-       /*
-        * Sorry this function is excessively complicated--- I'm just being
-        * extremely careful about integer overflows.
-        */
+       sd = new_wim_security_data();
+       if (!sd)
+               goto out_of_memory;
 
-       sd = MALLOC(sizeof(struct wim_security_data));
-       if (!sd) {
-               ERROR("Out of memory");
-               return WIMLIB_ERR_NOMEM;
-       }
-       sd->sizes       = NULL;
-       sd->descriptors = NULL;
+       sd_disk = (const struct wim_security_data_disk*)metadata_resource;
+       sd->total_length = le32_to_cpu(sd_disk->total_length);
+       sd->num_entries = le32_to_cpu(sd_disk->num_entries);
+
+       DEBUG("Reading security data: num_entries=%u, total_length=%u",
+             sd->num_entries, sd->total_length);
 
-       p = metadata_resource;
-       p = get_u32(p, &sd->total_length);
-       p = get_u32(p, (u32*)&sd->num_entries);
+       /* Length field of 0 is a special case that really means length
+        * of 8. */
+       if (sd->total_length == 0)
+               sd->total_length = 8;
 
        /* The security_id field of each dentry is a signed 32-bit integer, so
         * the possible indices into the security descriptors table are 0
         * through 0x7fffffff.  Which means 0x80000000 security descriptors
         * maximum.  Not like you should ever have anywhere close to that many
         * security descriptors! */
-       if (sd->num_entries > 0x80000000) {
-               ERROR("Security data has too many entries!");
+       if (sd->num_entries > 0x80000000)
                goto out_invalid_sd;
-       }
 
        /* Verify the listed total length of the security data is big enough to
         * include the sizes array, verify that the file data is big enough to
@@ -222,167 +230,151 @@ read_security_data(const u8 metadata_resource[], u64 metadata_resource_len,
         * integer, even though each security descriptor size is a 64-bit
         * integer.  This is stupid, and we need to be careful not to actually
         * let the security descriptor sizes be over 0xffffffff.  */
-       if ((u64)sd->total_length > metadata_resource_len) {
-               ERROR("Security data total length (%u) is bigger than the "
-                     "metadata resource length (%"PRIu64")",
-                     sd->total_length, metadata_resource_len);
+       if (sd->total_length > metadata_resource_len)
                goto out_invalid_sd;
-       }
 
-       DEBUG("Reading security data: %u entries, length = %u",
-             sd->num_entries, sd->total_length);
+       sizes_size = (u64)sd->num_entries * sizeof(u64);
+       size_no_descriptors = 8 + sizes_size;
+       if (size_no_descriptors > sd->total_length)
+               goto out_invalid_sd;
 
-       if (sd->num_entries == 0) {
-               /* No security descriptors.  We allow the total_length field to
-                * be either 8 (which is correct, since there are always 2
-                * 32-bit integers) or 0. */
-               if (sd->total_length != 0 && sd->total_length != 8) {
-                       ERROR("Invalid security data length (%u): expected 0 or 8",
-                             sd->total_length);
-                       goto out_invalid_sd;
-               }
-               sd->total_length = 8;
-               goto out_return_sd;
-       }
+       total_len = size_no_descriptors;
 
-       u64 sizes_size = (u64)sd->num_entries * sizeof(u64);
-       u64 size_no_descriptors = 8 + sizes_size;
-       if (size_no_descriptors > (u64)sd->total_length) {
-               ERROR("Security data total length of %u is too short because "
-                     "there seem to be at least %"PRIu64" bytes of security data",
-                     sd->total_length, 8 + sizes_size);
-               goto out_invalid_sd;
-       }
+       /* Return immediately if no security descriptors. */
+       if (sd->num_entries == 0)
+               goto out_align_total_length;
 
+       /* Allocate a new buffer for the sizes array */
        sd->sizes = MALLOC(sizes_size);
-       if (!sd->sizes) {
-               ret = WIMLIB_ERR_NOMEM;
-               goto out_free_sd;
-       }
+       if (!sd->sizes)
+               goto out_of_memory;
 
-       /* Copy the sizes array in from the file data. */
-       p = get_bytes(p, sizes_size, sd->sizes);
-       array_le64_to_cpu(sd->sizes, sd->num_entries);
-
-       /* Allocate the array of pointers to descriptors, and read them in. */
-       sd->descriptors = CALLOC(sd->num_entries, sizeof(u8*));
-       if (!sd->descriptors) {
-               ERROR("Out of memory while allocating security "
-                     "descriptors");
-               ret = WIMLIB_ERR_NOMEM;
-               goto out_free_sd;
+       /* Copy the sizes array into the new buffer */
+       for (u32 i = 0; i < sd->num_entries; i++) {
+               sd->sizes[i] = le64_to_cpu(sd_disk->sizes[i]);
+               if (sd->sizes[i] > 0xffffffff)
+                       goto out_invalid_sd;
        }
-       total_len = size_no_descriptors;
+
+       p = (const u8*)sd_disk + size_no_descriptors;
+
+       /* Allocate the array of pointers to the security descriptors, then read
+        * them into separate buffers. */
+       sd->descriptors = CALLOC(sd->num_entries, sizeof(sd->descriptors[0]));
+       if (!sd->descriptors)
+               goto out_of_memory;
 
        for (u32 i = 0; i < sd->num_entries; i++) {
-               /* Watch out for huge security descriptor sizes that could
-                * overflow the total length and wrap it around. */
-               if (total_len + sd->sizes[i] < total_len) {
-                       ERROR("Caught overflow in security descriptor lengths "
-                             "(current total length = %"PRIu64", security "
-                             "descriptor size = %"PRIu64")",
-                             total_len, sd->sizes[i]);
-                       goto out_invalid_sd;
-               }
+               if (sd->sizes[i] == 0)
+                       continue;
                total_len += sd->sizes[i];
-               /* This check ensures that the descriptor size fits in a 32 bit
-                * integer.  Because if it didn't, the total length would come
-                * out bigger than sd->total_length, which is a 32 bit integer.
-                * */
-               if (total_len > (u64)sd->total_length) {
-                       ERROR("Security data total length of %u is too short "
-                             "because there seem to be at least %"PRIu64" "
-                             "bytes of security data",
-                             sd->total_length, total_len);
+               if (total_len > (u64)sd->total_length)
                        goto out_invalid_sd;
-               }
                sd->descriptors[i] = MALLOC(sd->sizes[i]);
-               if (!sd->descriptors[i]) {
-                       ERROR("Out of memory while allocating security "
-                             "descriptors");
-                       ret = WIMLIB_ERR_NOMEM;
-                       goto out_free_sd;
-               }
-               p = get_bytes(p, sd->sizes[i], sd->descriptors[i]);
-               empty_sacl_fixup(sd->descriptors[i], &sd->sizes[i]);
+               if (!sd->descriptors[i])
+                       goto out_of_memory;
+               memcpy(sd->descriptors[i], p, sd->sizes[i]);
+               p += sd->sizes[i];
+               empty_sacl_fixup((SECURITY_DESCRIPTOR_RELATIVE*)sd->descriptors[i],
+                                &sd->sizes[i]);
        }
-       wimlib_assert(total_len <= 0xffffffff);
-       if (((total_len + 7) & ~7) != ((sd->total_length + 7) & ~7)) {
-               ERROR("Expected security data total length = %u, but "
-                     "calculated %u", sd->total_length, (unsigned)total_len);
-               goto out_invalid_sd;
+out_align_total_length:
+       total_len = (total_len + 7) & ~7;
+       sd->total_length = (sd->total_length + 7) & ~7;
+       if (total_len != sd->total_length) {
+               WARNING("Expected WIM security data total length of "
+                       "%u bytes, but calculated %u bytes",
+                       sd->total_length, (unsigned)total_len);
        }
-       sd->total_length = total_len;
 out_return_sd:
-       *sd_p = sd;
-       return 0;
+       *sd_ret = sd;
+       ret = 0;
+       goto out;
 out_invalid_sd:
+       ERROR("WIM security data is invalid!");
        ret = WIMLIB_ERR_INVALID_SECURITY_DATA;
+       goto out_free_sd;
+out_of_memory:
+       ERROR("Out of memory while reading WIM security data!");
+       ret = WIMLIB_ERR_NOMEM;
 out_free_sd:
-       free_security_data(sd);
+       free_wim_security_data(sd);
+out:
        return ret;
 }
 
 /*
- * Writes security data to an in-memory buffer.
+ * Writes the security data for a WIM image to an in-memory buffer.
  */
 u8 *
-write_security_data(const struct wim_security_data * restrict sd,
-                   u8 * restrict p)
+write_wim_security_data(const struct wim_security_data * restrict sd,
+                       u8 * restrict p)
 {
        DEBUG("Writing security data (total_length = %"PRIu32", num_entries "
              "= %"PRIu32")", sd->total_length, sd->num_entries);
 
-       u32 aligned_length = (sd->total_length + 7) & ~7;
-
        u8 *orig_p = p;
-       p = put_u32(p, aligned_length);
-       p = put_u32(p, sd->num_entries);
+       struct wim_security_data_disk *sd_disk = (struct wim_security_data_disk*)p;
+
+       sd_disk->total_length = cpu_to_le32(sd->total_length);
+       sd_disk->num_entries = cpu_to_le32(sd->num_entries);
 
        for (u32 i = 0; i < sd->num_entries; i++)
-               p = put_u64(p, sd->sizes[i]);
+               sd_disk->sizes[i] = cpu_to_le64(sd->sizes[i]);
+
+       p = (u8*)&sd_disk->sizes[sd_disk->num_entries];
 
        for (u32 i = 0; i < sd->num_entries; i++)
-               p = put_bytes(p, sd->sizes[i], sd->descriptors[i]);
+               p = mempcpy(p, sd->descriptors[i], sd->sizes[i]);
+
+       while (p - orig_p < sd->total_length)
+               *p++ = 0;
 
        wimlib_assert(p - orig_p == sd->total_length);
-       p = put_zeroes(p, aligned_length - sd->total_length);
 
        DEBUG("Successfully wrote security data.");
        return p;
 }
 
 static void
-print_acl(const void *p, const tchar *type)
+print_acl(const ACL *acl, const tchar *type, size_t max_size)
 {
-       const ACL *acl = p;
+       const u8 *p;
+
+       if (max_size < sizeof(ACL))
+               return;
+
        u8 revision = acl->revision;
        u16 acl_size = le16_to_cpu(acl->acl_size);
        u16 ace_count = le16_to_cpu(acl->ace_count);
+
        tprintf(T("    [%"TS" ACL]\n"), type);
        tprintf(T("    Revision = %u\n"), revision);
        tprintf(T("    ACL Size = %u\n"), acl_size);
        tprintf(T("    ACE Count = %u\n"), ace_count);
 
-       p += sizeof(ACL);
+       p = (const u8*)acl + sizeof(ACL);
        for (u16 i = 0; i < ace_count; i++) {
-               const ACE_HEADER *hdr = p;
-               tprintf(T("        [ACE]\n"));
-               tprintf(T("        ACE type  = %d\n"), hdr->type);
-               tprintf(T("        ACE flags = 0x%x\n"), hdr->flags);
-               tprintf(T("        ACE size  = %u\n"), hdr->size);
+               if (max_size < p + sizeof(ACCESS_ALLOWED_ACE) - (const u8*)acl)
+                       break;
                const ACCESS_ALLOWED_ACE *aaa = (const ACCESS_ALLOWED_ACE*)p;
+               tprintf(T("        [ACE]\n"));
+               tprintf(T("        ACE type  = %d\n"), aaa->hdr.type);
+               tprintf(T("        ACE flags = 0x%x\n"), aaa->hdr.flags);
+               tprintf(T("        ACE size  = %u\n"), le16_to_cpu(aaa->hdr.size));
                tprintf(T("        ACE mask = %x\n"), le32_to_cpu(aaa->mask));
                tprintf(T("        SID start = %u\n"), le32_to_cpu(aaa->sid_start));
-               p += hdr->size;
+               p += le16_to_cpu(aaa->hdr.size);
        }
        tputchar(T('\n'));
 }
 
 static void
-print_sid(const void *p, const tchar *type)
+print_sid(const SID *sid, const tchar *type, size_t max_size)
 {
-       const SID *sid = p;
+       if (max_size < sizeof(SID))
+               return;
+
        tprintf(T("    [%"TS" SID]\n"), type);
        tprintf(T("    Revision = %u\n"), sid->revision);
        tprintf(T("    Subauthority count = %u\n"), sid->sub_authority_count);
@@ -390,6 +382,8 @@ print_sid(const void *p, const tchar *type)
        print_byte_field(sid->identifier_authority,
                         sizeof(sid->identifier_authority), stdout);
        tputchar(T('\n'));
+       if (max_size < sizeof(SID) + (size_t)sid->sub_authority_count * sizeof(u32))
+               return;
        for (u8 i = 0; i < sid->sub_authority_count; i++) {
                tprintf(T("    Subauthority %u = %u\n"),
                        i, le32_to_cpu(sid->sub_authority[i]));
@@ -398,38 +392,45 @@ print_sid(const void *p, const tchar *type)
 }
 
 static void
-print_security_descriptor(const void *p, u64 size)
+print_security_descriptor(const SECURITY_DESCRIPTOR_RELATIVE *descr,
+                         size_t size)
 {
-       const SECURITY_DESCRIPTOR_RELATIVE *sd = p;
-
-       u8 revision      = sd->revision;
-       u16 control      = le16_to_cpu(sd->security_descriptor_control);
-       u32 owner_offset = le32_to_cpu(sd->owner_offset);
-       u32 group_offset = le32_to_cpu(sd->group_offset);
-       u32 sacl_offset  = le32_to_cpu(sd->sacl_offset);
-       u32 dacl_offset  = le32_to_cpu(sd->dacl_offset);
+       u8 revision      = descr->revision;
+       u16 control      = le16_to_cpu(descr->security_descriptor_control);
+       u32 owner_offset = le32_to_cpu(descr->owner_offset);
+       u32 group_offset = le32_to_cpu(descr->group_offset);
+       u32 dacl_offset  = le32_to_cpu(descr->dacl_offset);
+       u32 sacl_offset  = le32_to_cpu(descr->sacl_offset);
+
        tprintf(T("Revision = %u\n"), revision);
        tprintf(T("Security Descriptor Control = %#x\n"), control);
        tprintf(T("Owner offset = %u\n"), owner_offset);
        tprintf(T("Group offset = %u\n"), group_offset);
-       tprintf(T("System ACL offset = %u\n"), sacl_offset);
        tprintf(T("Discretionary ACL offset = %u\n"), dacl_offset);
+       tprintf(T("System ACL offset = %u\n"), sacl_offset);
+
+       if (owner_offset != 0 && owner_offset <= size)
+               print_sid((const SID*)((const u8*)descr + owner_offset),
+                         T("Owner"), size - owner_offset);
 
-       if (sd->owner_offset != 0)
-               print_sid(p + owner_offset, T("Owner"));
-       if (sd->group_offset != 0)
-               print_sid(p + group_offset, T("Group"));
-       if (sd->sacl_offset != 0)
-               print_acl(p + sacl_offset, T("System"));
-       if (sd->dacl_offset != 0)
-               print_acl(p + dacl_offset, T("Discretionary"));
+       if (group_offset != 0 && group_offset <= size)
+               print_sid((const SID*)((const u8*)descr + group_offset),
+                         T("Group"), size - group_offset);
+
+       if (dacl_offset != 0 && dacl_offset <= size)
+               print_acl((const ACL*)((const u8*)descr + dacl_offset),
+                         T("Discretionary"), size - dacl_offset);
+
+       if (sacl_offset != 0 && sacl_offset <= size)
+               print_acl((const ACL*)((const u8*)descr + sacl_offset),
+                         T("System"), size - sacl_offset);
 }
 
 /*
  * Prints the security data for a WIM file.
  */
 void
-print_security_data(const struct wim_security_data *sd)
+print_wim_security_data(const struct wim_security_data *sd)
 {
        tputs(T("[SECURITY DATA]"));
        tprintf(T("Length            = %"PRIu32" bytes\n"), sd->total_length);
@@ -438,14 +439,15 @@ print_security_data(const struct wim_security_data *sd)
        for (u32 i = 0; i < sd->num_entries; i++) {
                tprintf(T("[SECURITY_DESCRIPTOR_RELATIVE %"PRIu32", length = %"PRIu64"]\n"),
                        i, sd->sizes[i]);
-               print_security_descriptor(sd->descriptors[i], sd->sizes[i]);
+               print_security_descriptor((const SECURITY_DESCRIPTOR_RELATIVE*)sd->descriptors[i],
+                                         sd->sizes[i]);
                tputchar(T('\n'));
        }
        tputchar(T('\n'));
 }
 
 void
-free_security_data(struct wim_security_data *sd)
+free_wim_security_data(struct wim_security_data *sd)
 {
        if (sd) {
                u8 **descriptors = sd->descriptors;
@@ -481,8 +483,10 @@ destroy_sd_set(struct wim_sd_set *sd_set, bool rollback)
 {
        if (rollback) {
                struct wim_security_data *sd = sd_set->sd;
-               for (s32 i = sd_set->orig_num_entries; i < sd->num_entries; i++)
-                       FREE(sd->descriptors[i]);
+               u8 **descriptors = sd->descriptors + sd_set->orig_num_entries;
+               u32 num_entries  = sd->num_entries - sd_set->orig_num_entries;
+               while (num_entries--)
+                       FREE(*descriptors++);
                sd->num_entries = sd_set->orig_num_entries;
        }
        free_sd_tree(sd_set->rb_root.rb_node);
@@ -556,9 +560,11 @@ sd_set_add_sd(struct wim_sd_set *sd_set, const char *descriptor, size_t size)
 
        security_id = lookup_sd(sd_set, hash);
        if (security_id >= 0) /* Identical descriptor already exists */
-               return security_id;
+               goto out;
 
        /* Need to add a new security descriptor */
+       security_id = -1;
+
        new = MALLOC(sizeof(*new));
        if (!new)
                goto out;
@@ -588,17 +594,17 @@ sd_set_add_sd(struct wim_sd_set *sd_set, const char *descriptor, size_t size)
        sd->descriptors[sd->num_entries] = descr_copy;
        sd->sizes[sd->num_entries] = size;
        sd->num_entries++;
-       DEBUG("There are now %d security descriptors", sd->num_entries);
-       sd->total_length += size + sizeof(sd->sizes[0]);
+       DEBUG("There are now %u security descriptors", sd->num_entries);
        bret = insert_sd_node(sd_set, new);
        wimlib_assert(bret);
-       return new->security_id;
+       security_id = new->security_id;
+       goto out;
 out_free_descr:
        FREE(descr_copy);
 out_free_node:
        FREE(new);
 out:
-       return -1;
+       return security_id;
 }
 
 /* Initialize a `struct sd_set' mapping from SHA1 message digests of security
@@ -615,7 +621,7 @@ init_sd_set(struct wim_sd_set *sd_set, struct wim_security_data *sd)
        /* Remember the original number of security descriptors so that newly
         * added ones can be rolled back if needed. */
        sd_set->orig_num_entries = sd->num_entries;
-       for (s32 i = 0; i < sd->num_entries; i++) {
+       for (u32 i = 0; i < sd->num_entries; i++) {
                struct sd_node *new;
 
                new = MALLOC(sizeof(struct sd_node));
index eef45d3ce3ee29eae902c9834302c8f0313964a2..89da4b8dff999d0e35b2312ee6297a5ea61186b7 100644 (file)
@@ -52,7 +52,9 @@ verify_inode(struct wim_inode *inode, const WIMStruct *w)
                goto out;
        }
 
-       if (inode->i_security_id >= sd->num_entries) {
+       if (inode->i_security_id >= 0 &&
+           inode->i_security_id >= sd->num_entries)
+       {
                ERROR("Dentry `%"TS"' has an invalid security ID (%d) "
                      "(there are only %u entries in the security table)",
                      dentry_full_path(first_dentry), inode->i_security_id,
index 811ea2afa35e5aebd2c56663ca90f0edc5988a5e..2907cd31c42f933b03067f10e61cad86954d0a51 100644 (file)
--- a/src/wim.c
+++ b/src/wim.c
@@ -57,7 +57,7 @@ static int
 image_print_metadata(WIMStruct *w)
 {
        DEBUG("Printing metadata for image %d", w->current_image);
-       print_security_data(wim_security_data(w));
+       print_wim_security_data(wim_security_data(w));
        return for_dentry_in_tree(wim_root_dentry(w), print_dentry,
                                  w->lookup_table);
 }
@@ -555,7 +555,7 @@ destroy_image_metadata(struct wim_image_metadata *imd,
 {
        free_dentry_tree(imd->root_dentry, table);
        imd->root_dentry = NULL;
-       free_security_data(imd->security_data);
+       free_wim_security_data(imd->security_data);
        imd->security_data = NULL;
 
        if (free_metadata_lte) {