Make JPEG parsing less strict

nodeca · Nov 11, 2021 · 05faba2 · 05faba2
1 parent 7527733
commit 05faba2
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 9 deletions.
diff --git a/lib/parse_stream/jpeg.js b/lib/parse_stream/jpeg.js
@@ -44,11 +44,21 @@ function parseJpegMarker_afterFF(parser, callback) {
 }
 
 
-function parseJpegMarker(parser, callback) {
+function parseJpegMarker(parser, sandbox, callback) {
+  var start = sandbox.start;
+  sandbox.start = false;
+
   parser._bytes(1, function (data) {
     if (data[0] !== 0xFF) {
       // not a JPEG marker
-      callback();
+      if (start) {
+        // expect JPEG file to start with `FFD8 FFE0`, `FFD8 FFE2` or `FFD8 FFE1`,
+        // don't allow garbage before second marker
+        callback();
+      } else {
+        // skip until we see 0xFF, see https://github.com/nodeca/probe-image-size/issues/68
+        parseJpegMarker(parser, sandbox, callback);
+      }
       return;
     }
 
@@ -59,7 +69,7 @@ function parseJpegMarker(parser, callback) {
 
 // sandbox is a storage for intermediate data retrieved from jpeg while parsing it
 function getJpegSize(parser, sandbox) {
-  parseJpegMarker(parser, function (code, length) {
+  parseJpegMarker(parser, sandbox, function (code, length) {
     if (!code || length < 0) {
       // invalid jpeg
       parser._skipBytes(Infinity);
@@ -134,7 +144,7 @@ module.exports = function () {
       return;
     }
 
-    getJpegSize(parser, {});
+    getJpegSize(parser, { start: true });
   });
 
   return parser;

diff --git a/lib/parse_sync/jpeg.js b/lib/parse_sync/jpeg.js
@@ -14,15 +14,18 @@ var SIG_EXIF  = str2arr('Exif\0\0');
 module.exports = function (data) {
   if (data.length < 2) return;
 
-  // first marker of the file MUST be 0xFFD8
-  if (data[0] !== 0xFF || data[1] !== 0xD8) return;
+  // first marker of the file MUST be 0xFFD8,
+  // following by either 0xFFE0, 0xFFE2 or 0xFFE3
+  if (data[0] !== 0xFF || data[1] !== 0xD8 || data[2] !== 0xFF) return;
 
   var offset = 2;
 
   for (;;) {
-    if (data.length - offset < 2) return;
-    // not a JPEG marker
-    if (data[offset++] !== 0xFF) return;
+    // skip until we see 0xFF, see https://github.com/nodeca/probe-image-size/issues/68
+    for (;;) {
+      if (data.length - offset < 2) return;
+      if (data[offset++] === 0xFF) break;
+    }
 
     var code = data[offset++];
     var length;

diff --git a/test/test_formats.js b/test/test_formats.js
@@ -200,6 +200,24 @@ describe('File formats', function () {
     });
 
 
+    it('should skip unknown bytes between segments', async function () {
+      // not exactly standard, https://github.com/nodeca/probe-image-size/issues/68, `AA`s are extras in fixture
+      let buf = Buffer.from('FFD8 FFE00002 AAAAAAAA FFC00011 08000F000F03012200021101031101 FFD9'.replace(/ /g, ''), 'hex');
+
+      let size = await probe(Readable.from([ buf ]));
+
+      assert.deepStrictEqual(size, { width: 15, height: 15, type: 'jpg', mime: 'image/jpeg', wUnits: 'px', hUnits: 'px' });
+
+      // but don't allow garbage after first segment
+      buf = Buffer.from('FFD8 AAAAAAAA FFE00002 FFC00011 08000F000F03012200021101031101 FFD9'.replace(/ /g, ''), 'hex');
+
+      await assert.rejects(
+        async () => probe(Readable.from([ buf ])),
+        /unrecognized file format/
+      );
+    });
+
+
     it('coverage - EOI before SOI', async function () {
       let buf = Buffer.from('FFD8 FFD0 FFD9'.replace(/ /g, ''), 'hex');
 
@@ -280,6 +298,20 @@ describe('File formats', function () {
     });
 
 
+    it('should skip unknown bytes between segments', async function () {
+      // not exactly standard, https://github.com/nodeca/probe-image-size/issues/68, `AA`s are extras in fixture
+      let buf = str2arr('FFD8 FFE00002 AAAAAAAA FFC00011 08000F000F03012200021101031101 FFD9'.replace(/ /g, ''), 'hex');
+      let size = probe.sync(buf);
+
+      assert.deepStrictEqual(size, { width: 15, height: 15, type: 'jpg', mime: 'image/jpeg', wUnits: 'px', hUnits: 'px' });
+
+      // but don't allow garbage after first segment
+      buf = str2arr('FFD8 AAAAAAAA FFE00002 FFC00011 08000F000F03012200021101031101 FFD9'.replace(/ /g, ''), 'hex');
+
+      assert.strictEqual(probe.sync(buf), null);
+    });
+
+
     it('coverage - EOI before SOI', function () {
       let buf = str2arr('FFD8 FFD0 FFD9'.replace(/ /g, ''), 'hex');