add option validateEntrySizes. closes #53

README.md

@@ -71,7 +71,7 @@ function defaultCallback(err) {
 
 Calls `fs.open(path, "r")` and reads the `fd` effectively the same as `fromFd()` would.
 
-`options` may be omitted or `null`. The defaults are `{autoClose: true, lazyEntries: false, decodeStrings: true}`.
+`options` may be omitted or `null`. The defaults are `{autoClose: true, lazyEntries: false, decodeStrings: true, validateEntrySizes: true}`.
 
 `autoClose` is effectively equivalent to:
 
@@ -96,6 +96,11 @@ The exact effects of turning this option off are:
 * Any Info-ZIP Unicode Path Extra Field will be ignored. See `extraFields`.
 * Automatic file name validation will not be performed. See `validateFileName()`.
 
+`validateEntrySizes` is the default and ensures that an entry's reported uncompressed size matches its actual uncompressed size.
+This check happens as early as possible, which is either before emitting each `"entry"` event (for entries with no compression),
+or during the `readStream` piping after calling `openReadStream()`.
+See `openReadStream()` for more information on defending against zip bomb attacks.
+
 The `callback` is given the arguments `(err, zipfile)`.
 An `err` is provided if the End of Central Directory Record cannot be found, or if its metadata appears malformed.
 This kind of error usually indicates that this is not a zip file.
@@ -107,7 +112,7 @@ Reads from the fd, which is presumed to be an open .zip file.
 Note that random access is required by the zip file specification,
 so the fd cannot be an open socket or any other fd that does not support random access.
 
-`options` may be omitted or `null`. The defaults are `{autoClose: false, lazyEntries: false, decodeStrings: true}`.
+`options` may be omitted or `null`. The defaults are `{autoClose: false, lazyEntries: false, decodeStrings: true, validateEntrySizes: true}`.
 
 See `open()` for the meaning of the options and callback.
 
@@ -120,7 +125,7 @@ If a `ZipFile` is acquired from this method,
 it will never emit the `close` event,
 and calling `close()` is not necessary.
 
-`options` may be omitted or `null`. The defaults are `{lazyEntries: false, decodeStrings: true}`.
+`options` may be omitted or `null`. The defaults are `{lazyEntries: false, decodeStrings: true, validateEntrySizes: true}`.
 
 See `open()` for the meaning of the options and callback.
 The `autoClose` option is ignored for this method.
@@ -134,7 +139,7 @@ The `reader` parameter must be of a type that is a subclass of
 [RandomAccessReader](#class-randomaccessreader) that implements the required methods.
 The `totalSize` is a Number and indicates the total file size of the zip file.
 
-`options` may be omitted or `null`. The defaults are `{autoClose: true, lazyEntries: false, decodeStrings: true}`.
+`options` may be omitted or `null`. The defaults are `{autoClose: true, lazyEntries: false, decodeStrings: true, validateEntrySizes: true}`.
 
 See `open()` for the meaning of the options and callback.
 
@@ -172,6 +177,10 @@ See `open()` and `readEntry()` for when this event is emitted.
 If `decodeStrings` is `true`, entries emitted via this event have already passed file name validation.
 See `validateFileName()` and `open()` for more information.
 
+If `validateEntrySizes` is `true` and this entry's `compressionMethod` is `0` (stored without compression),
+this entry has already passed entry size validation.
+See `open()` for more information.
+
 #### Event: "end"
 
 Emitted after the last `entry` event has been emitted.
@@ -219,12 +228,12 @@ If this zipfile is already closed (see `close()`), the `callback` will receive a
 
 It's possible for the `readStream` it to emit errors for several reasons.
 For example, if zlib cannot decompress the data, the zlib error will be emitted from the `readStream`.
-Two more error cases are if the decompressed data has too many or too few actual bytes
-compared to the reported byte count from the entry's `uncompressedSize` field.
+Two more error cases (when `validateEntrySizes` is `true`) are if the decompressed data has too many
+or too few actual bytes compared to the reported byte count from the entry's `uncompressedSize` field.
 yauzl notices this false information and emits an error from the `readStream`
 after some number of bytes have already been piped through the stream.
 
-Because of this check, clients can always trust the `uncompressedSize` field in `Entry` objects.
+This check allows clients to trust the `uncompressedSize` field in `Entry` objects.
 Guarding against [zip bomb](http://en.wikipedia.org/wiki/Zip_bomb) attacks can be accomplished by
 doing some heuristic checks on the size metadata and then watching out for the above errors.
 Such heuristics are outside the scope of this library,
@@ -360,6 +369,8 @@ Subclasses *must* implement this method.
 This method should return a readable stream which will be `pipe()`ed into another stream.
 It is expected that the readable stream will provide data in several chunks if necessary.
 If the readable stream provides too many or too few bytes, an error will be emitted.
+(Note that `validateEntrySizes` has no effect on this check,
+because this is a low-level API that should behave correctly regardless of the contents of the file.)
 Any errors emitted on the readable stream will be handled and re-emitted on the client-visible stream
 (returned from `zipfile.openReadStream()`) or provided as the `err` argument to the appropriate callback
 (for example, for `fromRandomAccessReader()`).
@@ -490,6 +501,8 @@ This library makes no attempt to interpret the Language Encoding Flag.
 
 ## Change History
 
+ * 2.8.0
+   * Added option `validateEntrySizes`. [issue #53](https://github.com/thejoshwolfe/yauzl/issues/53)
  * 2.7.0
    * Added option `decodeStrings`. [issue #42](https://github.com/thejoshwolfe/yauzl/issues/42)
    * Fixed documentation for `entry.fileComment` and added compatibility alias. [issue #47](https://github.com/thejoshwolfe/yauzl/issues/47)

index.js

@@ -27,6 +27,7 @@ function open(path, options, callback) {
   if (options.autoClose == null) options.autoClose = true;
   if (options.lazyEntries == null) options.lazyEntries = false;
   if (options.decodeStrings == null) options.decodeStrings = true;
+  if (options.validateEntrySizes == null) options.validateEntrySizes = true;
   if (callback == null) callback = defaultCallback;
   fs.open(path, "r", function(err, fd) {
     if (err) return callback(err);
@@ -46,6 +47,7 @@ function fromFd(fd, options, callback) {
   if (options.autoClose == null) options.autoClose = false;
   if (options.lazyEntries == null) options.lazyEntries = false;
   if (options.decodeStrings == null) options.decodeStrings = true;
+  if (options.validateEntrySizes == null) options.validateEntrySizes = true;
   if (callback == null) callback = defaultCallback;
   fs.fstat(fd, function(err, stats) {
     if (err) return callback(err);
@@ -63,6 +65,7 @@ function fromBuffer(buffer, options, callback) {
   options.autoClose = false;
   if (options.lazyEntries == null) options.lazyEntries = false;
   if (options.decodeStrings == null) options.decodeStrings = true;
+  if (options.validateEntrySizes == null) options.validateEntrySizes = true;
   // i got your open file right here.
   var reader = fd_slicer.createFromBuffer(buffer);
   fromRandomAccessReader(reader, buffer.length, options, callback);
@@ -78,6 +81,7 @@ function fromRandomAccessReader(reader, totalSize, options, callback) {
   if (options.lazyEntries == null) options.lazyEntries = false;
   if (options.decodeStrings == null) options.decodeStrings = true;
   var decodeStrings = !!options.decodeStrings;
+  if (options.validateEntrySizes == null) options.validateEntrySizes = true;
   if (callback == null) callback = defaultCallback;
   if (typeof totalSize !== "number") throw new Error("expected totalSize parameter to be a number");
   if (totalSize > Number.MAX_SAFE_INTEGER) {
@@ -130,7 +134,7 @@ function fromRandomAccessReader(reader, totalSize, options, callback) {
                                   : eocdrBuffer.slice(22);
 
       if (!(entryCount === 0xffff || centralDirectoryOffset === 0xffffffff)) {
-        return callback(null, new ZipFile(reader, centralDirectoryOffset, totalSize, entryCount, comment, options.autoClose, options.lazyEntries, decodeStrings));
+        return callback(null, new ZipFile(reader, centralDirectoryOffset, totalSize, entryCount, comment, options.autoClose, options.lazyEntries, decodeStrings, options.validateEntrySizes));
       }
 
       // ZIP64 format
@@ -171,7 +175,7 @@ function fromRandomAccessReader(reader, totalSize, options, callback) {
           // 48 - offset of start of central directory with respect to the starting disk number     8 bytes
           centralDirectoryOffset = readUInt64LE(zip64EocdrBuffer, 48);
           // 56 - zip64 extensible data sector                                (variable size)
-          return callback(null, new ZipFile(reader, centralDirectoryOffset, totalSize, entryCount, comment, options.autoClose, options.lazyEntries, decodeStrings));
+          return callback(null, new ZipFile(reader, centralDirectoryOffset, totalSize, entryCount, comment, options.autoClose, options.lazyEntries, decodeStrings, options.validateEntrySizes));
         });
       });
       return;
@@ -181,7 +185,7 @@ function fromRandomAccessReader(reader, totalSize, options, callback) {
 }
 
 util.inherits(ZipFile, EventEmitter);
-function ZipFile(reader, centralDirectoryOffset, fileSize, entryCount, comment, autoClose, lazyEntries, decodeStrings) {
+function ZipFile(reader, centralDirectoryOffset, fileSize, entryCount, comment, autoClose, lazyEntries, decodeStrings, validateEntrySizes) {
   var self = this;
   EventEmitter.call(self);
   self.reader = reader;
@@ -201,6 +205,7 @@ function ZipFile(reader, centralDirectoryOffset, fileSize, entryCount, comment,
   self.autoClose = !!autoClose;
   self.lazyEntries = !!lazyEntries;
   self.decodeStrings = !!decodeStrings;
+  self.validateEntrySizes = !!validateEntrySizes;
   self.isOpen = true;
   self.emittedError = false;
 
@@ -389,7 +394,7 @@ ZipFile.prototype.readEntry = function() {
       }
 
       // validate file size
-      if (entry.compressionMethod === 0) {
+      if (self.validateEntrySizes && entry.compressionMethod === 0) {
         if (entry.compressedSize !== entry.uncompressedSize) {
           var msg = "compressed/uncompressed size mismatch for stored file: " + entry.compressedSize + " != " + entry.uncompressedSize;
           return emitErrorAndAutoClose(self, new Error(msg));
@@ -469,22 +474,29 @@ ZipFile.prototype.openReadStream = function(entry, callback) {
             if (!destroyed) inflateFilter.emit("error", err);
           });
         });
-
-        var checkerStream = new AssertByteCountStream(entry.uncompressedSize);
-        inflateFilter.on("error", function(err) {
-          // forward zlib errors to the client-visible stream
-          setImmediate(function() {
-            if (!destroyed) checkerStream.emit("error", err);
+        readStream.pipe(inflateFilter);
+
+        if (self.validateEntrySizes) {
+          endpointStream = new AssertByteCountStream(entry.uncompressedSize);
+          inflateFilter.on("error", function(err) {
+            // forward zlib errors to the client-visible stream
+            setImmediate(function() {
+              if (!destroyed) endpointStream.emit("error", err);
+            });
           });
-        });
-        checkerStream.destroy = function() {
+          inflateFilter.pipe(endpointStream);
+        } else {
+          // the zlib filter is the client-visible stream
+          endpointStream = inflateFilter;
+        }
+        // this is part of yauzl's API, so implement this function on the client-visible stream
+        endpointStream.destroy = function() {
           destroyed = true;
-          inflateFilter.unpipe(checkerStream);
+          if (inflateFilter !== endpointStream) inflateFilter.unpipe(endpointStream);
           readStream.unpipe(inflateFilter);
-          // TODO: the inflateFilter now causes a memory leak. see Issue #27.
+          // TODO: the inflateFilter may cause a memory leak. see Issue #27.
           readStream.destroy();
         };
-        endpointStream = readStream.pipe(inflateFilter).pipe(checkerStream);
       }
       callback(null, endpointStream);
     } finally {

test/test.js

@@ -24,13 +24,18 @@ function shouldDoTest(testPath) {
 }
 
 // success tests
-listZipFiles(path.join(__dirname, "success")).forEach(function(zipfilePath) {
+listZipFiles([path.join(__dirname, "success"), path.join(__dirname, "wrong-entry-sizes")]).forEach(function(zipfilePath) {
   if (!shouldDoTest(zipfilePath)) return;
   var optionConfigurations = [
     // you can find more options coverage in the failure tests.
     {lazyEntries: true},
     {lazyEntries: true, decodeStrings: false},
   ];
+  if (/\/wrong-entry-sizes\//.test(zipfilePath)) {
+    optionConfigurations.forEach(function(options) {
+      options.validateEntrySizes = false;
+    });
+  }
   var openFunctions = [
     function(testId, options, callback) { yauzl.open(zipfilePath, options, callback); },
     function(testId, options, callback) { yauzl.fromBuffer(fs.readFileSync(zipfilePath), options, callback); },
@@ -127,7 +132,7 @@ listZipFiles(path.join(__dirname, "success")).forEach(function(zipfilePath) {
 });
 
 // failure tests
-listZipFiles(path.join(__dirname, "failure")).forEach(function(zipfilePath) {
+listZipFiles([path.join(__dirname, "failure")]).forEach(function(zipfilePath) {
   if (!shouldDoTest(zipfilePath)) return;
   var expectedErrorMessage = path.basename(zipfilePath).replace(/(_\d+)?\.zip$/, "");
   var failedYet = false;
@@ -311,11 +316,14 @@ pend.wait(function() {
 });
 
 
-function listZipFiles(dir) {
-  var zipfilePaths = fs.readdirSync(dir).filter(function(filepath) {
-    return /\.zip$/.exec(filepath);
-  }).map(function(name) {
-    return path.relative(".", path.join(dir, name));
+function listZipFiles(dirList) {
+  var zipfilePaths = [];
+  dirList.forEach(function(dir) {
+    fs.readdirSync(dir).filter(function(filepath) {
+      return /\.zip$/.exec(filepath);
+    }).forEach(function(name) {
+      zipfilePaths.push(path.relative(".", path.join(dir, name)));
+    });
   });
   zipfilePaths.sort();
   return zipfilePaths;

test/wrong-entry-sizes/wrong-entry-sizes/a.txt

@@ -0,0 +1 @@
+======================================

test/wrong-entry-sizes/wrong-entry-sizes/b.txt

@@ -0,0 +1 @@
+===========================================================================