Bug 1380204: Improve error handling in XZStream.cpp r?glandium draft
authorBrian Murray <bmurray7jhu@gmail.com>
Tue, 11 Jul 2017 17:45:07 -0700
changeset 616396 3a8693410ea38bf0a8d19196a37b220e6b36ce78
parent 606416 5d794bf4c4653153e602631f1b8818acd559d8f5
child 639451 282e87ed351a297cfdb06dd24ed1a69d4f530381
push id70661
push userbmo:bmurray7jhu@gmail.com
push dateThu, 27 Jul 2017 00:13:43 +0000
reviewersglandium
bugs1380204
milestone56.0a1
Bug 1380204: Improve error handling in XZStream.cpp r?glandium Report init failure if uncompressed stream size is 0. Check for overflows when casting. Verify LZMA stream only has a single block. Detailed error logging. MozReview-Commit-ID: DZ4cWGxAzkw
mozglue/linker/XZStream.cpp
mozglue/linker/XZStream.h
--- a/mozglue/linker/XZStream.cpp
+++ b/mozglue/linker/XZStream.cpp
@@ -1,13 +1,13 @@
 #include "XZStream.h"
 
 #include <algorithm>
-#include <cstring>
 #include "mozilla/Assertions.h"
+#include "mozilla/CheckedInt.h"
 #include "Logging.h"
 
 // LZMA dictionary size, should have a minimum size for the given compression
 // rate, see XZ Utils docs for details.
 static const uint32_t kDictSize = 1 << 24;
 
 static const size_t kFooterSize = 12;
 
@@ -67,16 +67,19 @@ XZStream::Init()
 
   mDec = xz_dec_init(XZ_DYNALLOC, kDictSize);
 
   if (!mDec) {
     return false;
   }
 
   mUncompSize = ParseUncompressedSize();
+  if (!mUncompSize) {
+    return false;
+  }
 
   return true;
 }
 
 size_t
 XZStream::Decode(void* aOutBuf, size_t aOutSize)
 {
   if (!mDec) {
@@ -159,56 +162,76 @@ XZStream::ParseIndexSize() const
   static const uint8_t kFooterMagic[] = {'Y', 'Z'};
 
   const uint8_t* footer = mInBuf + mBuffers.in_size - kFooterSize;
   // The magic bytes are at the end of the footer.
   if (memcmp(reinterpret_cast<const void*>(kFooterMagic),
              footer + kFooterSize - sizeof(kFooterMagic),
              sizeof(kFooterMagic))) {
     // Not a valid footer at stream end.
+    ERROR("XZ parsing: Invalid footer at end of stream");
     return 0;
   }
   // Backward size is a 32 bit LE integer field positioned after the 32 bit CRC32
   // code. It encodes the index size as a multiple of 4 bytes with a minimum
   // size of 4 bytes.
-  const uint32_t backwardSize = *(footer + 4);
-  return (backwardSize + 1) * 4;
+  const uint32_t backwardSizeRaw = *(footer + 4);
+  // Check for overflow.
+  mozilla::CheckedInt<size_t> backwardSizeBytes(backwardSizeRaw);
+  backwardSizeBytes = (backwardSizeBytes + 1) * 4;
+  if (!backwardSizeBytes.isValid()) {
+    ERROR("XZ parsing: Cannot parse index size");
+    return 0;
+  }
+  return backwardSizeBytes.value();
 }
 
 size_t
 XZStream::ParseUncompressedSize() const
 {
   static const uint8_t kIndexIndicator[] = {0x0};
 
   const size_t indexSize = ParseIndexSize();
   if (!indexSize) {
     return 0;
   }
   // The footer follows directly the index, so we can use it as a reference.
   const uint8_t* end = mInBuf + mBuffers.in_size;
   const uint8_t* index = end - kFooterSize - indexSize;
 
-  // The index consists of a one byte indicator followed by a VLI field for the
-  // number of records (1 expected) followed by a list of records. One record
-  // contains a VLI field for unpadded size followed by a VLI field for
-  // uncompressed size.
+  // The xz stream index consists of three concatenated elements:
+  //  (1) 1 byte indicator (always OxOO)
+  //  (2) a Variable Length Integer (VLI) field for the number of records
+  //  (3) a list of records
+  // See https://tukaani.org/xz/xz-file-format-1.0.4.txt
+  // Each record contains a VLI field for unpadded size followed by a var field for
+  // uncompressed size. We only support xz streams with a single record.
+
   if (memcmp(reinterpret_cast<const void*>(kIndexIndicator),
              index, sizeof(kIndexIndicator))) {
-    // Not a valid index.
+    ERROR("XZ parsing: Invalid stream index");
     return 0;
   }
 
   index += sizeof(kIndexIndicator);
   uint64_t numRecords = 0;
   index += ParseVarLenInt(index, end - index, &numRecords);
-  if (!numRecords) {
+  // Only streams with a single record are supported.
+  if (numRecords != 1) {
+    ERROR("XZ parsing: Multiple records not supported");
     return 0;
   }
   uint64_t unpaddedSize = 0;
   index += ParseVarLenInt(index, end - index, &unpaddedSize);
   if (!unpaddedSize) {
+    ERROR("XZ parsing: Unpadded size is 0");
     return 0;
   }
   uint64_t uncompressedSize = 0;
   index += ParseVarLenInt(index, end - index, &uncompressedSize);
+  mozilla::CheckedInt<size_t> checkedSize(uncompressedSize);
+  if (!checkedSize.isValid()) {
+    ERROR("XZ parsing: Uncompressed stream size is too large");
+    return 0;
+  }
 
-  return uncompressedSize;
+  return checkedSize.value();
 }
--- a/mozglue/linker/XZStream.h
+++ b/mozglue/linker/XZStream.h
@@ -1,16 +1,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef XZSTREAM_h
 #define XZSTREAM_h
 
 #include <cstdlib>
+#include <stdint.h>
 
 #define XZ_DEC_DYNALLOC
 #include "xz.h"
 
 // Used to decode XZ stream buffers.
 class XZStream
 {
 public: