]> wimlib.net Git - wimlib/blobdiff - src/security.c
bitstream_read_bits(): Remove bogus assertion
[wimlib] / src / security.c
index 63ab5ba090984a0cc8f0656bc7d27d44cdce1972..dd9d1d82b9df56a1763799c9fdae2731c9a42064 100644 (file)
@@ -1,9 +1,7 @@
 /*
  * security.c
  *
- * Read and write the WIM security data.  The security data is a table of
- * security descriptors. Each WIM image has its own security data, but it's
- * possible that an image's security data have no security descriptors.
+ * Read and write the per-WIM-image table of security descriptors.
  */
 
 /*
  */
 
 #include "wimlib_internal.h"
-#include "io.h"
+#include "buffer_io.h"
 #include "security.h"
 
-/* 
+/*
  * This is a hack to work around a problem in libntfs-3g.  libntfs-3g validates
  * security descriptors with a function named ntfs_valid_descr().
  * ntfs_valid_descr() considers a security descriptor that ends in a SACL
  * invalid.  However, a security descriptor like this exists in the Windows 7
  * install.wim.  Here, security descriptors matching this pattern are modified
  * to have no SACL.  This should make no difference since the SACL had no
- * entries anyway; however  his ensures that that the security descriptors pass
+ * entries anyway; however this ensures that that the security descriptors pass
  * the validation in libntfs-3g.
  */
-static void empty_sacl_fixup(char *descr, u64 *size_p)
+static void empty_sacl_fixup(u8 *descr, u64 *size_p)
 {
        if (*size_p >= sizeof(SecurityDescriptor)) {
                SecurityDescriptor *sd = (SecurityDescriptor*)descr;
@@ -52,12 +50,12 @@ static void empty_sacl_fixup(char *descr, u64 *size_p)
        }
 }
 
-/* 
+/*
  * Reads the security data from the metadata resource.
  *
  * @metadata_resource: An array that contains the uncompressed metadata
  *                             resource for the WIM file.
- * @metadata_resource_len:     The length of @metadata_resource.  It MUST be at
+ * @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
  *             will be filled in with a pointer to a new wim_security_data
@@ -74,6 +72,13 @@ int read_security_data(const u8 metadata_resource[], u64 metadata_resource_len,
        int ret;
        u64 total_len;
 
+       wimlib_assert(metadata_resource_len >= 8);
+
+       /*
+        * Sorry this function is excessively complicated--- I'm just being
+        * extremely careful about integer overflows.
+        */
+
        sd = MALLOC(sizeof(struct wim_security_data));
        if (!sd) {
                ERROR("Out of memory");
@@ -87,10 +92,14 @@ int read_security_data(const u8 metadata_resource[], u64 metadata_resource_len,
        p = get_u32(p, &sd->total_length);
        p = get_u32(p, (u32*)&sd->num_entries);
 
-       if (sd->num_entries > 0x7fffffff) {
+       /* 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!");
-               ret = WIMLIB_ERR_INVALID_SECURITY_DATA;
-               goto out_free_sd;
+               goto out_invalid_sd;
        }
 
        /* Verify the listed total length of the security data is big enough to
@@ -105,28 +114,34 @@ int read_security_data(const u8 metadata_resource[], u64 metadata_resource_len,
                ERROR("Security data total length (%u) is bigger than the "
                      "metadata resource length (%"PRIu64")",
                      sd->total_length, metadata_resource_len);
-               ret = WIMLIB_ERR_INVALID_SECURITY_DATA;
-               goto out_free_sd;
+               goto out_invalid_sd;
        }
 
        DEBUG("Reading security data: %u entries, length = %u",
              sd->num_entries, sd->total_length);
 
        if (sd->num_entries == 0) {
-               /* No security data. */
-               total_len = 8;
-               goto out;
+               /* 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;
        }
 
        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 must be at least %"PRIu64" bytes of security data",
+                     "there seem to be at least %"PRIu64" bytes of security data",
                      sd->total_length, 8 + sizes_size);
-               ret = WIMLIB_ERR_INVALID_SECURITY_DATA;
-               goto out_free_sd;
+               goto out_invalid_sd;
        }
+
        sd->sizes = MALLOC(sizes_size);
        if (!sd->sizes) {
                ret = WIMLIB_ERR_NOMEM;
@@ -155,16 +170,19 @@ int read_security_data(const u8 metadata_resource[], u64 metadata_resource_len,
                              "(current total length = %"PRIu64", security "
                              "descriptor size = %"PRIu64")",
                              total_len, sd->sizes[i]);
-                       ret = WIMLIB_ERR_INVALID_SECURITY_DATA;
-                       goto out_free_sd;
+                       goto out_invalid_sd;
                }
                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 are at least %"PRIu64" bytes of "
-                             "security data", sd->total_length, total_len);
-                       ret = WIMLIB_ERR_INVALID_SECURITY_DATA;
-                       goto out_free_sd;
+                             "because there seem to be at least %"PRIu64" "
+                             "bytes of security data",
+                             sd->total_length, total_len);
+                       goto out_invalid_sd;
                }
                sd->descriptors[i] = MALLOC(sd->sizes[i]);
                if (!sd->descriptors[i]) {
@@ -176,16 +194,24 @@ int read_security_data(const u8 metadata_resource[], u64 metadata_resource_len,
                p = get_bytes(p, sd->sizes[i], sd->descriptors[i]);
                empty_sacl_fixup(sd->descriptors[i], &sd->sizes[i]);
        }
-out:
-       sd->total_length = (u32)total_len;
+       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;
+       }
+       sd->total_length = total_len;
+out_return_sd:
        *sd_p = sd;
        return 0;
+out_invalid_sd:
+       ret = WIMLIB_ERR_INVALID_SECURITY_DATA;
 out_free_sd:
        free_security_data(sd);
        return ret;
 }
 
-/* 
+/*
  * Writes security data to an in-memory buffer.
  */
 u8 *write_security_data(const struct wim_security_data *sd, u8 *p)
@@ -224,7 +250,7 @@ static void print_acl(const u8 *p, const char *type)
        printf("    ACE Count = %u\n", ace_count);
 
        p += sizeof(ACL);
-       for (uint i = 0; i < acl->ace_count; i++) {
+       for (u16 i = 0; i < ace_count; i++) {
                const ACEHeader *hdr = (const ACEHeader*)p;
                printf("        [ACE]\n");
                printf("        ACE type  = %d\n", hdr->type);
@@ -248,7 +274,7 @@ static void print_sid(const u8 *p, const char *type)
        print_byte_field(sid->identifier_authority,
                         sizeof(sid->identifier_authority));
        putchar('\n');
-       for (uint i = 0; i < sid->sub_authority_count; i++)
+       for (u8 i = 0; i < sid->sub_authority_count; i++)
                printf("    Subauthority %u = %u\n",
                       i, le32_to_cpu(sid->sub_authority[i]));
        putchar('\n');
@@ -280,17 +306,19 @@ static void print_security_descriptor(const u8 *p, u64 size)
                print_acl(p + dacl_offset, "Discretionary");
 }
 
-/* 
+/*
  * Prints the security data for a WIM file.
  */
 void print_security_data(const struct wim_security_data *sd)
 {
+       wimlib_assert(sd != NULL);
+
        puts("[SECURITY DATA]");
        printf("Length            = %"PRIu32" bytes\n", sd->total_length);
        printf("Number of Entries = %"PRIu32"\n", sd->num_entries);
 
        for (u32 i = 0; i < sd->num_entries; i++) {
-               printf("[SecurityDescriptor %"PRIu32", length = %"PRIu64"]\n", 
+               printf("[SecurityDescriptor %"PRIu32", length = %"PRIu64"]\n",
                       i, sd->sizes[i]);
                print_security_descriptor(sd->descriptors[i], sd->sizes[i]);
                putchar('\n');
@@ -300,17 +328,17 @@ void print_security_data(const struct wim_security_data *sd)
 
 void free_security_data(struct wim_security_data *sd)
 {
-       if (!sd)
-               return;
-       wimlib_assert(sd->refcnt != 0);
-       if (--sd->refcnt == 0) {
-               u8 **descriptors = sd->descriptors;
-               u32 num_entries  = sd->num_entries;
-               if (descriptors)
-                       while (num_entries--)
-                               FREE(*descriptors++);
-               FREE(sd->sizes);
-               FREE(sd->descriptors);
-               FREE(sd);
+       if (sd) {
+               wimlib_assert(sd->refcnt != 0);
+               if (--sd->refcnt == 0) {
+                       u8 **descriptors = sd->descriptors;
+                       u32 num_entries  = sd->num_entries;
+                       if (descriptors)
+                               while (num_entries--)
+                                       FREE(*descriptors++);
+                       FREE(sd->sizes);
+                       FREE(sd->descriptors);
+                       FREE(sd);
+               }
        }
 }