Bug 1468539 - Limit libmar from reading more than a single Additional_Section. r=mhowell
authorJune Wilde <jewilde@mozilla.com>
Mon, 05 Nov 2018 21:29:42 -0500
changeset 444517 609b4f85c0d3da3d29c4f45581af9b2aa465f1e5
parent 444516 0874b793fc046bf31ca80c673d5dfa0bb38ea343
child 444518 d3284735c7fa4bbe5cddc24f3905112f3490fa6d
push id34996
push userrgurzau@mozilla.com
push dateTue, 06 Nov 2018 09:53:23 +0000
treeherdermozilla-central@e160f0a60e4f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmhowell
bugs1468539
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 1468539 - Limit libmar from reading more than a single Additional_Section. r=mhowell Only a single type of additional block has ever been defined for the MAR archive format and only a single block of that type is needed per file. Limiting ourselves to reading only that until we define more seems sensible. Move additionalBlockSize check before first fread Add MAXADDITIONALBLOCKSIZE as a constant for checking block sizes Differential Revision: https://phabricator.services.mozilla.com/D10797
modules/libmar/src/mar_read.c
--- a/modules/libmar/src/mar_read.c
+++ b/modules/libmar/src/mar_read.c
@@ -12,16 +12,22 @@
 #include "mar.h"
 
 #ifdef XP_WIN
 #include <winsock2.h>
 #else
 #include <netinet/in.h>
 #endif
 
+/* 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
 
 /* this is the same hash algorithm used by nsZipArchive.cpp */
 static uint32_t mar_hash_name(const char *name) {
   uint32_t val = 0;
   unsigned char* c;
 
   for (c = (unsigned char *) name; *c; ++c)
     val = val*37 + *c;
@@ -392,62 +398,61 @@ read_product_info_block(char *path,
  *
  * @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)
 {
-  uint32_t i, offsetAdditionalBlocks, numAdditionalBlocks,
+  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[97] = { '\0' };
+  char buf[MAXADDITIONALBLOCKSIZE + 1] = { '\0' };
   if (get_mar_file_info_fp(mar->fp, NULL, NULL,
                            &hasAdditionalBlocks,
                            &offsetAdditionalBlocks,
                            &numAdditionalBlocks) != 0) {
     return -1;
   }
-  for (i = 0; i < numAdditionalBlocks; ++i) {
+
+  /* We only have the one additional block type and only one is expected to be
+     in a MAR file so check if any exist and process the first found */
+  if (numAdditionalBlocks > 0) {
     /* Read the additional block size */
     if (fread(&additionalBlockSize,
               sizeof(additionalBlockSize),
               1, mar->fp) != 1) {
       return -1;
     }
     additionalBlockSize = ntohl(additionalBlockSize) -
                           sizeof(additionalBlockSize) -
                           sizeof(additionalBlockID);
 
+    /* Additional Block sizes should only be 96 bytes long */
+    if (additionalBlockSize > MAXADDITIONALBLOCKSIZE) {
+      return -1;
+    }
+
     /* Read the additional block ID */
     if (fread(&additionalBlockID,
               sizeof(additionalBlockID),
               1, mar->fp) != 1) {
       return -1;
     }
     additionalBlockID = ntohl(additionalBlockID);
 
     if (PRODUCT_INFO_BLOCK_ID == additionalBlockID) {
       const char *location;
       int len;
 
-      /* 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) */
-      if (additionalBlockSize > 96) {
-        return -1;
-      }
-
-    if (fread(buf, additionalBlockSize, 1, mar->fp) != 1) {
+      if (fread(buf, additionalBlockSize, 1, mar->fp) != 1) {
         return -1;
       }
 
       /* Extract the MAR channel name from the buffer.  For now we
          point to the stack allocated buffer but we strdup this
          if we are within bounds of each field's max length. */
       location = buf;
       len = strlen(location);