Bug 1468556 - Protect against overlapping files in libmar; r=mhowell
authorJune Wilde <jewilde@mozilla.com>
Mon, 26 Nov 2018 17:25:24 +0000
changeset 504760 06463d10abdee48650cbe28ae63e957951430412
parent 504759 447f8ddd9bbd1c5b618025758019e01525e2cb45
child 504761 2b4f08a645a0385021b64b4f995c18cb2250a5c6
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmhowell
bugs1468556
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1468556 - Protect against overlapping files in libmar; r=mhowell Disallows files from referencing the same bytes in the content blocks of a MAR file by storing a list of structs containing a file's byte offsets and lengths. A list was chosen since the cap of 256 files wouldn't produce considerable overhead when extracting/reading/searching/etc through the archive. Removing the ability for a MAR file to reference the same content block repeatedly seems like a better solution than what was suggested in the BLRG report. (limiting the number of files or checking for overly large decompressed files) Allows us to prohibit this type of file bomb while only losing an attribute of the MAR file format that wasn't being leveraged. The fix is applied in mar_enum_items and mar_find_item so that the manifest the updater uses is equally safeguarded as the mar host tool. Differential Revision: https://phabricator.services.mozilla.com/D11706
modules/libmar/src/mar.h
modules/libmar/src/mar_read.c
modules/libmar/tests/unit/data/manipulated_backend_collision.mar
modules/libmar/tests/unit/data/manipulated_frontend_collision.mar
modules/libmar/tests/unit/data/manipulated_is_contained.mar
modules/libmar/tests/unit/data/manipulated_is_container.mar
modules/libmar/tests/unit/data/manipulated_multiple_collision.mar
modules/libmar/tests/unit/data/manipulated_multiple_collision_first.mar
modules/libmar/tests/unit/data/manipulated_multiple_collision_last.mar
modules/libmar/tests/unit/data/manipulated_same_offset.mar
modules/libmar/tests/unit/head_libmar.js
modules/libmar/tests/unit/test_extract.js
--- a/modules/libmar/src/mar.h
+++ b/modules/libmar/src/mar.h
@@ -22,143 +22,171 @@ extern "C" {
 */
 #define MAX_SIGNATURES 8
 #ifdef __cplusplus
 static_assert(MAX_SIGNATURES <= 9, "too many signatures");
 #else
 MOZ_STATIC_ASSERT(MAX_SIGNATURES <= 9, "too many signatures");
 #endif
 
-struct ProductInformationBlock {
-  const char *MARChannelID;
-  const char *productVersion;
+struct ProductInformationBlock
+{
+  const char* MARChannelID;
+  const char* productVersion;
 };
 
 /**
  * The MAR item data structure.
  */
-typedef struct MarItem_ {
-  struct MarItem_ *next;  /* private field */
+typedef struct MarItem_
+{
+  struct MarItem_* next;  /* private field */
   uint32_t offset;        /* offset into archive */
   uint32_t length;        /* length of data in bytes */
   uint32_t flags;         /* contains file mode bits */
   char name[1];           /* file path */
 } MarItem;
 
+/**
+ * File offset and length for tracking access of byte indexes
+ */
+typedef struct SeenIndex_
+{
+  struct SeenIndex_* next; /* private field */
+  uint32_t offset;         /* offset into archive */
+  uint32_t length;         /* length of the data in bytes */
+} SeenIndex;
+
 #define TABLESIZE 256
 
-struct MarFile_ {
-  FILE *fp;
-  MarItem *item_table[TABLESIZE];
-  int item_table_is_valid;
+/**
+ * Mozilla ARchive (MAR) file data structure
+ */
+struct MarFile_
+{
+  FILE* fp;                       /* file pointer to the archive */
+  MarItem* item_table[TABLESIZE]; /* hash table of files in the archive */
+  SeenIndex* index_list;          /* file indexes processed */
+  int item_table_is_valid;        /* header and index validation flag */
 };
 
 typedef struct MarFile_ MarFile;
 
 /**
  * Signature of callback function passed to mar_enum_items.
  * @param mar       The MAR file being visited.
  * @param item      The MAR item being visited.
  * @param data      The data parameter passed by the caller of mar_enum_items.
  * @return          A non-zero value to stop enumerating.
  */
-typedef int (* MarItemCallback)(MarFile *mar, const MarItem *item, void *data);
+typedef int (*MarItemCallback)(MarFile* mar, const MarItem* item, void* data);
 
 /**
  * Open a MAR file for reading.
  * @param path      Specifies the path to the MAR file to open.  This path must
  *                  be compatible with fopen.
  * @return          NULL if an error occurs.
  */
-MarFile *mar_open(const char *path);
+MarFile*
+mar_open(const char* path);
 
 #ifdef XP_WIN
 MarFile *mar_wopen(const wchar_t *path);
 #endif
 
 /**
  * Close a MAR file that was opened using mar_open.
  * @param mar       The MarFile object to close.
  */
-void mar_close(MarFile *mar);
+void
+mar_close(MarFile* mar);
 
 /**
  * Find an item in the MAR file by name.
  * @param mar       The MarFile object to query.
  * @param item      The name of the item to query.
  * @return          A const reference to a MAR item or NULL if not found.
  */
-const MarItem *mar_find_item(MarFile *mar, const char *item);
+const MarItem*
+mar_find_item(MarFile* mar, const char* item);
 
 /**
  * Enumerate all MAR items via callback function.
  * @param mar       The MAR file to enumerate.
  * @param callback  The function to call for each MAR item.
  * @param data      A caller specified value that is passed along to the
  *                  callback function.
  * @return          0 if the enumeration ran to completion.  Otherwise, any
  *                  non-zero return value from the callback is returned.
  */
-int mar_enum_items(MarFile *mar, MarItemCallback callback, void *data);
+int
+mar_enum_items(MarFile* mar, MarItemCallback callback, void* data);
 
 /**
  * Read from MAR item at given offset up to bufsize bytes.
  * @param mar       The MAR file to read.
  * @param item      The MAR item to read.
  * @param offset    The byte offset relative to the start of the item.
  * @param buf       A pointer to a buffer to copy the data into.
  * @param bufsize   The length of the buffer to copy the data into.
  * @return          The number of bytes written or a negative value if an
  *                  error occurs.
  */
-int mar_read(MarFile *mar, const MarItem *item, int offset, uint8_t *buf,
-             int bufsize);
+int
+mar_read(MarFile* mar,
+         const MarItem* item,
+         int offset,
+         uint8_t* buf,
+         int bufsize);
 
 /**
  * Create a MAR file from a set of files.
  * @param dest      The path to the file to create.  This path must be
  *                  compatible with fopen.
  * @param numfiles  The number of files to store in the archive.
  * @param files     The list of null-terminated file paths.  Each file
  *                  path must be compatible with fopen.
  * @param infoBlock The information to store in the product information block.
  * @return          A non-zero value if an error occurs.
  */
-int mar_create(const char *dest,
-               int numfiles,
-               char **files,
-               struct ProductInformationBlock *infoBlock);
+int
+mar_create(const char* dest,
+           int numfiles,
+           char** files,
+           struct ProductInformationBlock* infoBlock);
 
 /**
  * Extract a MAR file to the current working directory.
  * @param path      The path to the MAR file to extract.  This path must be
  *                  compatible with fopen.
  * @return          A non-zero value if an error occurs.
  */
-int mar_extract(const char *path);
+int
+mar_extract(const char* path);
 
 #define MAR_MAX_CERT_SIZE (16*1024) // Way larger than necessary
 
 /* Read the entire file (not a MAR file) into a newly-allocated buffer.
  * This function does not write to stderr. Instead, the caller should
  * write whatever error messages it sees fit. The caller must free the returned
  * buffer using free().
  *
  * @param filePath The path to the file that should be read.
  * @param maxSize  The maximum valid file size.
  * @param data     On success, *data will point to a newly-allocated buffer
  *                 with the file's contents in it.
  * @param size     On success, *size will be the size of the created buffer.
  *
  * @return 0 on success, -1 on error
  */
-int mar_read_entire_file(const char * filePath,
-                         uint32_t maxSize,
-                         /*out*/ const uint8_t * *data,
-                         /*out*/ uint32_t *size);
+int
+mar_read_entire_file(const char* filePath,
+                     uint32_t maxSize,
+                     /*out*/ const uint8_t** data,
+                     /*out*/ uint32_t* size);
 
 /**
  * Verifies a MAR file by verifying each signature with the corresponding
  * certificate. That is, the first signature will be verified using the first
  * certificate given, the second signature will be verified using the second
  * certificate given, etc. The signature count must exactly match the number of
  * certificates given, and all signature verifications must succeed.
  * We do not check that the certificate was issued by any trusted authority.
@@ -170,30 +198,31 @@ int mar_read_entire_file(const char * fi
  *                       file data.
  * @param certDataSizes  Pointer to the first element in an array for size of
  *                       the cert data.
  * @param certCount      The number of elements in certData and certDataSizes
  * @return 0 on success
  *         a negative number if there was an error
  *         a positive number if the signature does not verify
  */
-int mar_verify_signatures(MarFile *mar,
-                          const uint8_t * const *certData,
-                          const uint32_t *certDataSizes,
-                          uint32_t certCount);
+int
+mar_verify_signatures(MarFile* mar,
+                      const uint8_t* const* certData,
+                      const uint32_t* certDataSizes,
+                      uint32_t certCount);
 
 /**
  * Reads the product info block from the MAR file's additional block section.
  * The caller is responsible for freeing the fields in infoBlock
  * if the return is successful.
  *
  * @param infoBlock Out parameter for where to store the result to
  * @return 0 on success, -1 on failure
 */
 int
-mar_read_product_info_block(MarFile *mar,
-                            struct ProductInformationBlock *infoBlock);
+mar_read_product_info_block(MarFile* mar,
+                            struct ProductInformationBlock* infoBlock);
 
 #ifdef __cplusplus
 }
 #endif
 
 #endif  /* MAR_H__ */
--- a/modules/libmar/src/mar_read.c
+++ b/modules/libmar/src/mar_read.c
@@ -14,22 +14,30 @@
 
 /* This block must be at most 104 bytes.
    MAR channel name < 64 bytes, and product version < 32 bytes + 3 NULL
    terminator bytes. We only check for 96 though because we remove 8
    bytes above from the additionalBlockSize: We subtract
    sizeof(additionalBlockSize) and sizeof(additionalBlockID) */
 #define MAXADDITIONALBLOCKSIZE 96
 
-static uint32_t mar_hash_name(const char *name) {
+static uint32_t
+mar_hash_name(const char* name)
+{
   return CityHash64(name, strlen(name)) % TABLESIZE;
 }
 
-static int mar_insert_item(MarFile *mar, const char *name, int namelen,
-                           uint32_t offset, uint32_t length, uint32_t flags) {
+static int
+mar_insert_item(MarFile* mar,
+                const char* name,
+                int namelen,
+                uint32_t offset,
+                uint32_t length,
+                uint32_t flags)
+{
   MarItem *item, *root;
   uint32_t hash;
 
   item = (MarItem *) malloc(sizeof(MarItem) + namelen);
   if (!item)
     return -1;
   item->next = NULL;
   item->offset = offset;
@@ -46,17 +54,19 @@ static int mar_insert_item(MarFile *mar,
     /* append item */
     while (root->next)
       root = root->next;
     root->next = item;
   }
   return 0;
 }
 
-static int mar_consume_index(MarFile *mar, char **buf, const char *buf_end) {
+static int
+mar_consume_index(MarFile* mar, char** buf, const char* buf_end)
+{
   /*
    * Each item has the following structure:
    *   uint32_t offset      (network byte order)
    *   uint32_t length      (network byte order)
    *   uint32_t flags       (network byte order)
    *   char     name[N]     (where N >= 1)
    *   char     null_byte;
    */
@@ -98,17 +108,19 @@ static int mar_consume_index(MarFile *ma
   /* consume null byte */
   if (*buf == buf_end)
     return -1;
   ++(*buf);
 
   return mar_insert_item(mar, name, namelen, offset, length, flags);
 }
 
-static int mar_read_index(MarFile *mar) {
+static int
+mar_read_index(MarFile* mar)
+{
   char id[MAR_ID_SIZE], *buf, *bufptr, *bufend;
   uint32_t offset_to_index, size_of_index;
 
   /* verify MAR ID */
   fseek(mar->fp, 0, SEEK_SET);
   if (fread(id, MAR_ID_SIZE, 1, mar->fp) != 1)
     return -1;
   if (memcmp(id, MAR_ID, MAR_ID_SIZE) != 0)
@@ -136,79 +148,151 @@ static int mar_read_index(MarFile *mar) 
   bufend = buf + size_of_index;
   while (bufptr < bufend && mar_consume_index(mar, &bufptr, bufend) == 0);
 
   free(buf);
   return (bufptr == bufend) ? 0 : -1;
 }
 
 /**
+ * Adds an offset and length to the MarFile's index_list
+ * @param mar     The MarFile that owns this offset length pair
+ * @param offset  The byte offset in the archive to be marked as processed
+ * @param length  The length corresponding to this byte offset
+ * @return int    1 on success, 0 if offset has been previously processed
+ *                -1 if unable to allocate space for the SeenIndexes
+ */
+static int
+mar_insert_offset(MarFile* mar, uint32_t offset, uint32_t length)
+{
+  /* Ignore files with no length */
+  if (length == 0) {
+    return 1;
+  }
+
+  SeenIndex* index = (SeenIndex*)malloc(sizeof(SeenIndex));
+  if (!index) {
+    return -1;
+  }
+  index->next = NULL;
+  index->offset = offset;
+  index->length = length;
+  uint32_t index_end = index->offset + index->length - 1;
+
+  /* If this is our first index store it at the front */
+  if (mar->index_list == NULL) {
+    mar->index_list = index;
+    return 1;
+  }
+
+  /* Search for matching indexes in the list of those previously visited */
+  SeenIndex* previous;
+  SeenIndex* current = mar->index_list;
+  while (current != NULL) {
+    uint32_t current_end = current->offset + current->length - 1;
+
+    /* If index has collided with the front or end of current or if current has
+       collided with the front or end of index return false */
+    if ((index->offset >= current->offset && index->offset <= current_end) ||
+        (index_end >= current->offset && index_end <= current_end) ||
+        (current->offset >= index->offset && current->offset <= index_end) ||
+        (current_end >= index->offset && current_end <= index_end)) {
+      free(index);
+      return 0;
+    }
+
+    /* else move to the next in the list */
+    previous = current;
+    current = current->next;
+  }
+
+  /* These indexes are valid, track them */
+  previous->next = index;
+  return 1;
+}
+
+/**
  * Internal shared code for mar_open and mar_wopen.
  * On failure, will fclose(fp).
  */
-static MarFile *mar_fpopen(FILE *fp)
+static MarFile*
+mar_fpopen(FILE* fp)
 {
-  MarFile *mar;
+  MarFile* mar;
 
-  mar = (MarFile *) malloc(sizeof(*mar));
+  mar = (MarFile*)malloc(sizeof(*mar));
   if (!mar) {
     fclose(fp);
     return NULL;
   }
 
   mar->fp = fp;
   mar->item_table_is_valid = 0;
   memset(mar->item_table, 0, sizeof(mar->item_table));
+  mar->index_list = NULL;
 
   return mar;
 }
 
-MarFile *mar_open(const char *path) {
+MarFile*
+mar_open(const char* path)
+{
   FILE *fp;
 
   fp = fopen(path, "rb");
   if (!fp) {
     fprintf(stderr, "ERROR: could not open file in mar_open()\n");
     perror(path);
     return NULL;
   }
 
   return mar_fpopen(fp);
 }
 
 #ifdef XP_WIN
-MarFile *mar_wopen(const wchar_t *path) {
+MarFile*
+mar_wopen(const wchar_t* path)
+{
   FILE *fp;
 
   _wfopen_s(&fp, path, L"rb");
   if (!fp) {
     fprintf(stderr, "ERROR: could not open file in mar_wopen()\n");
     _wperror(path);
     return NULL;
   }
 
   return mar_fpopen(fp);
 }
 #endif
 
-void mar_close(MarFile *mar) {
-  MarItem *item;
+void
+mar_close(MarFile* mar)
+{
+  MarItem* item;
+  SeenIndex* index;
   int i;
 
   fclose(mar->fp);
 
   for (i = 0; i < TABLESIZE; ++i) {
     item = mar->item_table[i];
     while (item) {
-      MarItem *temp = item;
+      MarItem* temp = item;
       item = item->next;
       free(temp);
     }
   }
 
+  while (mar->index_list != NULL) {
+    index = mar->index_list;
+    mar->index_list = index->next;
+    free(index);
+  }
+
   free(mar);
 }
 
 /**
  * Determines the MAR file information.
  *
  * @param fp                     An opened MAR file in read mode.
  * @param hasSignatureBlock      Optional out parameter specifying if the MAR
@@ -220,22 +304,23 @@ void mar_close(MarFile *mar) {
  * @param offsetAdditionalBlocks Optional out parameter for the offset to the
  *                               first additional block. Value is only valid if
  *                               hasAdditionalBlocks is not equal to 0.
  * @param numAdditionalBlocks    Optional out parameter for the number of
  *                               additional blocks.  Value is only valid if
  *                               hasAdditionalBlocks is not equal to 0.
  * @return 0 on success and non-zero on failure.
  */
-int get_mar_file_info_fp(FILE *fp,
-                         int *hasSignatureBlock,
-                         uint32_t *numSignatures,
-                         int *hasAdditionalBlocks,
-                         uint32_t *offsetAdditionalBlocks,
-                         uint32_t *numAdditionalBlocks)
+int
+get_mar_file_info_fp(FILE* fp,
+                     int* hasSignatureBlock,
+                     uint32_t* numSignatures,
+                     int* hasAdditionalBlocks,
+                     uint32_t* offsetAdditionalBlocks,
+                     uint32_t* numAdditionalBlocks)
 {
   uint32_t offsetToIndex, offsetToContent, signatureCount, signatureLen, i;
 
   /* One of hasSignatureBlock or hasAdditionalBlocks must be non NULL */
   if (!hasSignatureBlock && !hasAdditionalBlocks) {
     return -1;
   }
 
@@ -358,18 +443,17 @@ int get_mar_file_info_fp(FILE *fp,
  * Reads the product info block from the MAR file's additional block section.
  * The caller is responsible for freeing the fields in infoBlock
  * if the return is successful.
  *
  * @param infoBlock Out parameter for where to store the result to
  * @return 0 on success, -1 on failure
 */
 int
-read_product_info_block(char *path,
-                        struct ProductInformationBlock *infoBlock)
+read_product_info_block(char* path, struct ProductInformationBlock* infoBlock)
 {
   int rv;
   MarFile mar;
   mar.fp = fopen(path, "rb");
   if (!mar.fp) {
     fprintf(stderr, "ERROR: could not open file in read_product_info_block()\n");
     perror(path);
     return -1;
@@ -383,18 +467,18 @@ read_product_info_block(char *path,
  * Reads the product info block from the MAR file's additional block section.
  * The caller is responsible for freeing the fields in infoBlock
  * if the return is successful.
  *
  * @param infoBlock Out parameter for where to store the result to
  * @return 0 on success, -1 on failure
 */
 int
-mar_read_product_info_block(MarFile *mar,
-                            struct ProductInformationBlock *infoBlock)
+mar_read_product_info_block(MarFile* mar,
+                            struct ProductInformationBlock* infoBlock)
 {
   uint32_t offsetAdditionalBlocks, numAdditionalBlocks,
     additionalBlockSize, additionalBlockID;
   int hasAdditionalBlocks;
 
   /* The buffer size is 97 bytes because the MAR channel name < 64 bytes, and
      product version < 32 bytes + 3 NULL terminator bytes. */
   char buf[MAXADDITIONALBLOCKSIZE + 1] = { '\0' };
@@ -471,64 +555,87 @@ mar_read_product_info_block(MarFile *mar
       }
     }
   }
 
   /* If we had a product info block we would have already returned */
   return -1;
 }
 
-const MarItem *mar_find_item(MarFile *mar, const char *name) {
+const MarItem*
+mar_find_item(MarFile* mar, const char* name)
+{
   uint32_t hash;
-  const MarItem *item;
+  const MarItem* item;
 
   if (!mar->item_table_is_valid) {
     if (mar_read_index(mar)) {
       return NULL;
     } else {
       mar->item_table_is_valid = 1;
     }
   }
 
   hash = mar_hash_name(name);
 
   item = mar->item_table[hash];
-  while (item && strcmp(item->name, name) != 0)
+  while (item && strcmp(item->name, name) != 0) {
     item = item->next;
+  }
 
-  return item;
+  /* If this is the first time seeing this item's indexes, return it */
+  if (mar_insert_offset(mar, item->offset, item->length) == 1) {
+    return item;
+  } else {
+    fprintf(stderr, "ERROR: file content collision in mar_find_item()\n");
+    return NULL;
+  }
 }
 
-int mar_enum_items(MarFile *mar, MarItemCallback callback, void *closure) {
-  MarItem *item;
-  int i;
+int
+mar_enum_items(MarFile* mar, MarItemCallback callback, void* closure)
+{
+  MarItem* item;
+  int i, rv;
 
   if (!mar->item_table_is_valid) {
     if (mar_read_index(mar)) {
       return -1;
     } else {
       mar->item_table_is_valid = 1;
     }
   }
 
   for (i = 0; i < TABLESIZE; ++i) {
     item = mar->item_table[i];
     while (item) {
-      int rv = callback(mar, item, closure);
-      if (rv)
-        return rv;
+      /* if this is the first time seeing this item's indexes, process it */
+      if (mar_insert_offset(mar, item->offset, item->length) == 1) {
+        rv = callback(mar, item, closure);
+        if (rv) {
+          return rv;
+        }
+      } else {
+        fprintf(stderr, "ERROR: file content collision in mar_enum_items()\n");
+        return 1;
+      }
       item = item->next;
     }
   }
 
   return 0;
 }
 
-int mar_read(MarFile *mar, const MarItem *item, int offset, uint8_t *buf,
-             int bufsize) {
+int
+mar_read(MarFile* mar,
+         const MarItem* item,
+         int offset,
+         uint8_t* buf,
+         int bufsize)
+{
   int nr;
 
   if (offset == (int) item->length)
     return 0;
   if (offset > (int) item->length)
     return -1;
 
   nr = item->length - offset;
@@ -554,22 +661,23 @@ int mar_read(MarFile *mar, const MarItem
  * @param offsetAdditionalBlocks Optional out parameter for the offset to the
  *                               first additional block. Value is only valid if
  *                               hasAdditionalBlocks is not equal to 0.
  * @param numAdditionalBlocks    Optional out parameter for the number of
  *                               additional blocks.  Value is only valid if
  *                               has_additional_blocks is not equal to 0.
  * @return 0 on success and non-zero on failure.
  */
-int get_mar_file_info(const char *path,
-                      int *hasSignatureBlock,
-                      uint32_t *numSignatures,
-                      int *hasAdditionalBlocks,
-                      uint32_t *offsetAdditionalBlocks,
-                      uint32_t *numAdditionalBlocks)
+int
+get_mar_file_info(const char* path,
+                  int* hasSignatureBlock,
+                  uint32_t* numSignatures,
+                  int* hasAdditionalBlocks,
+                  uint32_t* offsetAdditionalBlocks,
+                  uint32_t* numAdditionalBlocks)
 {
   int rv;
   FILE *fp = fopen(path, "rb");
   if (!fp) {
     fprintf(stderr, "ERROR: could not open file in get_mar_file_info()\n");
     perror(path);
     return -1;
   }
new file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..41d4f78482848d279230c35db097f81bcf21bb44
GIT binary patch
literal 210
zc%1Wf3^HV3U|7Kb0hgdOBM@hRXa`@%pm=8wM?XJTpLkCf2L>}!J%dC;25JFA1tSGx
z1rr5R1v3S61q%gB1p|ddg(QV!g%pKUg*1h9g$#vEg)9Z2QZ*oM1Y!}OvzDY~=A_0a
Mf=p;Y5lUhJ0KAtN(*OVf
new file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..582af58b59b13ceebc261dd0fe142471361ce241
GIT binary patch
literal 210
zc%1Wf3^HV3U|7Kb0hgdOBM@hRXa`@%pm=8wM?XJTpLkCf2L>}!J%dC;25JFA1tSGx
z1rr5R1v3S61q%gB1p|ddg(QV!g%pKUg*1h9g$#vEg)9Z2QZ*oM0AdlKvzDY~=A_0a
Mf=p;c5lUhJ0K9}4(*OVf
new file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..d51b23587d0b49f6282bcaf5f77697f1e5bad3ea
GIT binary patch
literal 210
zc%1Wf3^HV3U|7Kb0hgdOBM@hRXa`@%pm=8wM?XJTpLkCf2L>}!J%dC;25JFA1tSGx
z1rr5R1v3S61q%gB1p|ddg(QV!g%pKUg*1h9g$#vEg)9Z2QZ*oM0b&uLvzDY~=A_0a
Of=p-tVhy-Z5(5Ce@EGa<
new file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..98b33ce9e5a895acf90a5ebf3ef9a68f7672f3bc
GIT binary patch
literal 210
zc%1Wf3^HV3U|7Kb0hgdOBM@hRXa`@%pm=8wM?XJTpLkCf2L>}!J%dC;25JFA1tSGx
z1rr5R1v3S61q%gB1p|ddg(QV!g%pKUg*1h9g$#vEg)9Z2QZ*oM0AdZGvzDY~=A_0a
Of=p-xViCAd5(5Cf@EGR+
new file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..7e0a3dd72458417ce0f7f38b44abd868a902883d
GIT binary patch
literal 249
zc%1Wf3^HV3VA#U|0Y9NMBM@hRXa`@%pm=8wM?XJTpLkCf2L>}!J%dC;25JFA1tSGx
z1rr5R1v3S61q%gB1p|ddg(QV!g%pKUg*1h9g$#vEg)9X{1qTIX1r-HVg*XK@1uX>)
m1x*E@c1s{`0AdlKQ<tP==A_0af=r!)B9sIYYDE!BW&i*pyC2{H
new file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..a10d3eb53b30e247f9e4fae42a40f6b2c2c7b230
GIT binary patch
literal 249
zc%1Wf3^HV3VA#U|0Y9NMBM@hRXa`@%pm=8wM?XJTpLkCf2L>}!J%dC;25JFA1tSGx
z1rr5R1v3S61q%gB1p|ddg(QV!g%pKUg*1h9g$#vEg)9X{1qTIX1r-HVg*XK@1uX>)
m1x*E@c1s{`0AdlKQ<tP==A_0af=r!)B9sIYT8<)=%m4r++aLk}
new file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..bfbb9ba8535739329d560a6d84b3ddbe5aef2ed9
GIT binary patch
literal 249
zc%1Wf3^HV3VA#U|0Y9NMBM@hRXa`@%pm=8wM?XJTpLkCf2L>}!J%dC;25JFA1tSGx
z1rr5R1v3S61q%gB1p|ddg(QV!g%pKUg*1h9g$#vEg)9X{1qTIX1r-HVg*XK@1uX>)
m1x*E@c1s{`0AdlKQ<tP==A_0af=r!^B9sIYT7e>z%m4r+$RGj$
new file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..1326d1afd8d56e0311930d6e93b529ac93765a02
GIT binary patch
literal 210
zc%1Wf3^HV3U|7Kb0hgdOBM@hRXa`@%pm=8wM?XJTpLkCf2L>}!J%dC;25JFA1tSGx
z1rr5R1v3S61q%gB1p|ddg(QV!g%pKUg*1h9g$#vEg)9Z2QZ*oM0AdlKvzDY~=A_0a
KGGGWLF#rI(bQsbA
--- a/modules/libmar/tests/unit/head_libmar.js
+++ b/modules/libmar/tests/unit/head_libmar.js
@@ -1,28 +1,28 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
-'use strict';
+"use strict";
 
 const BIN_SUFFIX = mozinfo.bin_suffix;
 const tempDir = do_get_tempdir();
 
 /**
  * Compares binary data of 2 arrays and throws if they aren't the same.
  * Throws on mismatch, does nothing on match.
  *
  * @param arr1 The first array to compare
  * @param arr2 The second array to compare
  */
 function compareBinaryData(arr1, arr2) {
   Assert.equal(arr1.length, arr2.length);
   for (let i = 0; i < arr1.length; i++) {
     if (arr1[i] != arr2[i]) {
-      throw "Data differs at index " + i + 
+      throw "Data differs at index " + i +
             ", arr1: " + arr1[i] + ", arr2: " + arr2[i];
     }
   }
 }
 
 /**
  * Reads a file's data and returns it
  *
@@ -108,17 +108,17 @@ function createMAR(outMAR, dataDir, file
     f.permissions = 0o664;
   }
 
   // Setup the command line arguments to create the MAR.
   let args = ["-C", dataDir.path, "-H", "\@MAR_CHANNEL_ID\@",
               "-V", "13.0a1", "-c", outMAR.path];
   args = args.concat(files);
 
-  info('Running: ' + signmarBin.path + " " + args.join(" "));
+  info("Running: " + signmarBin.path + " " + args.join(" "));
   process.init(signmarBin);
   process.run(true, args, args.length);
 
   // Verify signmar returned 0 for success.
   Assert.equal(process.exitValue, 0);
 
   // Verify the out MAR file actually exists.
   Assert.ok(outMAR.exists());
@@ -135,18 +135,17 @@ function extractMAR(mar, dataDir) {
   let process = Cc["@mozilla.org/process/util;1"].
                 createInstance(Ci.nsIProcess);
   let signmarBin = do_get_file("signmar" + BIN_SUFFIX);
 
   // Make sure the signmar binary exists and is an executable.
   Assert.ok(signmarBin.exists());
   Assert.ok(signmarBin.isExecutable());
 
-  // Setup the command line arguments to create the MAR.
+  // Setup the command line arguments to extract the MAR.
   let args = ["-C", dataDir.path, "-x", mar.path];
 
-  info('Running: ' + signmarBin.path + " " + args.join(" "));
+  info("Running: " + signmarBin.path + " " + args.join(" "));
   process.init(signmarBin);
   process.run(true, args, args.length);
 
-  // Verify signmar returned 0 for success.
-  Assert.equal(process.exitValue, 0);
+  return process.exitValue;
 }
--- a/modules/libmar/tests/unit/test_extract.js
+++ b/modules/libmar/tests/unit/test_extract.js
@@ -4,17 +4,17 @@
 function run_test() {
 
   /**
    * Extracts a MAR and makes sure each file matches the reference files.
    *
    * @param marFileName The name of the MAR file to extract
    * @param files       The files that the extracted MAR should contain
    */
-  function run_one_test(marFileName, files) {
+  function extract_and_compare(marFileName, files) {
     // Get the MAR file that we will be extracting
     let mar = do_get_file("data/" + marFileName);
 
     // Get the path that we will extract to
     let outDir = tempDir.clone();
     outDir.append("out");
     Assert.ok(!outDir.exists());
     outDir.create(Ci.nsIFile.DIRECTORY_TYPE, 0o777);
@@ -26,62 +26,114 @@ function run_test() {
       let outFile = outDir.clone();
       outFile.append(files[i]);
       Assert.ok(!outFile.exists());
 
       outFiles.push(outFile);
       refFiles.push(do_get_file("data/" + files[i]));
     }
 
-    // Extract the MAR contents into the ./out dir.
-    extractMAR(mar, outDir);
+    // Extract the MAR contents to ./out dir and verify 0 for success.
+    Assert.equal(extractMAR(mar, outDir), 0);
 
     // Compare to make sure the extracted files are the same.
     for (let i = 0; i < files.length; i++) {
       Assert.ok(outFiles[i].exists());
       let refFileData = getBinaryFileData(refFiles[i]);
       let outFileData = getBinaryFileData(outFiles[i]);
       compareBinaryData(refFileData, outFileData);
     }
   }
 
+  /**
+   * Attempts to extract a MAR and expects a failure
+   *
+   * @param marFileName The name of the MAR file to extract
+   */
+  function extract_and_fail(marFileName) {
+    // Get the MAR file that we will be extracting
+    let mar = do_get_file("data/" + marFileName);
+
+    // Get the path that we will extract to
+    let outDir = tempDir.clone();
+    outDir.append("out");
+    Assert.ok(!outDir.exists());
+    outDir.create(Ci.nsIFile.DIRECTORY_TYPE, 0o777);
+
+    // Extract the MAR contents to ./out dir and verify -1 (255 from the
+    // nsIprocess) for failure
+    Assert.equal(extractMAR(mar, outDir), 1);
+  }
+
   // Define the unit tests to run.
   let tests = {
     // Test extracting a MAR file with a 0 byte file.
     test_zero_sized: function _test_zero_sized() {
-      return run_one_test("0_sized.mar", ["0_sized_file"]);
+      return extract_and_compare("0_sized.mar", ["0_sized_file"]);
     },
     // Test extracting a MAR file with a 1 byte file.
     test_one_byte: function _test_one_byte() {
-      return run_one_test("1_byte.mar", ["1_byte_file"]);
+      return extract_and_compare("1_byte.mar", ["1_byte_file"]);
     },
     // Test extracting a MAR file with binary data.
     test_binary_data: function _test_binary_data() {
-      return run_one_test("binary_data.mar", ["binary_data_file"]);
+      return extract_and_compare("binary_data.mar", ["binary_data_file"]);
     },
     // Test extracting a MAR without a product information block (PIB) which
     // contains binary data.
     test_no_pib: function _test_no_pib() {
-      return run_one_test("no_pib.mar", ["binary_data_file"]);
+      return extract_and_compare("no_pib.mar", ["binary_data_file"]);
     },
     // Test extracting a MAR without a product information block (PIB) that is
     // signed and which contains binary data.
     test_no_pib_signed: function _test_no_pib_signed() {
-      return run_one_test("signed_no_pib.mar", ["binary_data_file"]);
+      return extract_and_compare("signed_no_pib.mar", ["binary_data_file"]);
     },
     // Test extracting a MAR with a product information block (PIB) that is
     // signed and which contains binary data.
     test_pib_signed: function _test_pib_signed() {
-      return run_one_test("signed_pib.mar", ["binary_data_file"]);
+      return extract_and_compare("signed_pib.mar", ["binary_data_file"]);
     },
     // Test extracting a MAR file with multiple files inside of it.
     test_multiple_file: function _test_multiple_file() {
-      return run_one_test("multiple_file.mar",
+      return extract_and_compare("multiple_file.mar",
                           ["0_sized_file", "1_byte_file", "binary_data_file"]);
     },
+    // Test collision detection where file A + B are the same offset
+    test_collision_same_offset: function test_collision_same_offset() {
+      return extract_and_fail("manipulated_same_offset.mar");
+    },
+    // Test collision detection where file A's indexes are a subset of file B's
+    test_collision_is_contained: function test_collision_is_contained() {
+      return extract_and_fail("manipulated_is_container.mar");
+    },
+    // Test collision detection where file B's indexes are a subset of file A's
+    test_collision_contained_by: function test_collision_contained_by() {
+      return extract_and_fail("manipulated_is_contained.mar");
+    },
+    // Test collision detection where file A ends in file B's indexes
+    test_collision_a_onto_b: function test_collision_a_onto_b() {
+      return extract_and_fail("manipulated_frontend_collision.mar");
+    },
+    // Test collision detection where file B ends in file A's indexes
+    test_collsion_b_onto_a: function test_collsion_b_onto_a()  {
+      return extract_and_fail("manipulated_backend_collision.mar");
+    },
+    // Test collision detection where file C shares indexes with both file A & B
+    test_collision_multiple: function test_collision_multiple() {
+      return extract_and_fail("manipulated_multiple_collision.mar");
+    },
+    // Test collision detection where A is the last file in the list
+    test_collision_last: function test_collision_multiple_last() {
+      return extract_and_fail("manipulated_multiple_collision_last.mar");
+    },
+    // Test collision detection where A is the first file in the list
+    test_collision_first: function test_collision_multiple_first() {
+      return extract_and_fail("manipulated_multiple_collision_first.mar");
+    },
     // Between each test make sure the out directory and its subfiles do
     // not exist.
     cleanup_per_test: function _cleanup_per_test() {
       let outDir = tempDir.clone();
       outDir.append("out");
       if (outDir.exists()) {
         outDir.remove(true);
       }