From 26cffd03b0f54a37b22c6a4ecee50fef66aceab5 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 11 Apr 2020 15:49:25 +0200 Subject: [PATCH 1/2] [src/core/jpg.js] Remove some redundant marker validation during the MCU parsing in the `decodeScan` function Some of the code in `src/core/jpg.js` is fairly old, and has with time become unnecessary when the surrounding code has been updated to handle various types of JPEG corruption. In particular the `if (!marker || marker <= 0xff00) { ... }` branch is now dead code, since: - The `!marker` case can no longer happen, since we would already have broken out of the loop thanks to the `!fileMarker` branch a handful of lines above. - The `marker <= 0xff00` case can also no longer happen, since the `findNextFileMarker` function validate markers much more thoroughly (by checking `marker >= 0xffc0 && marker <= 0xfffe`). Hence we'd again have broken out of the loop via the `!fileMarker` branch above when no valid marker was found. --- src/core/jpg.js | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/core/jpg.js b/src/core/jpg.js index 3433dadce..53a2c1498 100644 --- a/src/core/jpg.js +++ b/src/core/jpg.js @@ -429,23 +429,17 @@ var JpegImage = (function JpegImageClosure() { bitsCount = 0; fileMarker = findNextFileMarker(data, offset); if (!fileMarker) { - // Reached the end of the image data without finding an EOI marker. - break; - } else if (fileMarker.invalid) { + break; // Reached the end of the image data without finding any marker. + } + if (fileMarker.invalid) { // Some bad images seem to pad Scan blocks with e.g. zero bytes, skip // past those to attempt to find a valid marker (fixes issue4090.pdf). warn( - "decodeScan - unexpected MCU data, current marker is: " + - fileMarker.invalid + `decodeScan - unexpected MCU data, current marker is: ${fileMarker.invalid}` ); offset = fileMarker.offset; } - var marker = fileMarker && fileMarker.marker; - if (!marker || marker <= 0xff00) { - throw new JpegError("decodeScan - a valid marker was not found."); - } - - if (marker >= 0xffd0 && marker <= 0xffd7) { + if (fileMarker.marker >= 0xffd0 && fileMarker.marker <= 0xffd7) { // RSTx offset += 2; } else { @@ -458,8 +452,7 @@ var JpegImage = (function JpegImageClosure() { // attempt to find the next valid marker (fixes issue8182.pdf). if (fileMarker && fileMarker.invalid) { warn( - "decodeScan - unexpected Scan data, current marker is: " + - fileMarker.invalid + `decodeScan - unexpected Scan data, current marker is: ${fileMarker.invalid}` ); offset = fileMarker.offset; } From 06f6f8719f92b4131a72eba4f66e90c4ab898d79 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 12 Apr 2020 12:27:11 +0200 Subject: [PATCH 2/2] Always skip over any additional, unexpected, RSTx (restart) markers in corrupt JPEG images (issue 11794) --- src/core/jpg.js | 50 ++++++++++++++++++++--------------- test/pdfs/issue11794.pdf.link | 1 + test/test_manifest.json | 8 ++++++ 3 files changed, 38 insertions(+), 21 deletions(-) create mode 100644 test/pdfs/issue11794.pdf.link diff --git a/src/core/jpg.js b/src/core/jpg.js index 53a2c1498..8df06677f 100644 --- a/src/core/jpg.js +++ b/src/core/jpg.js @@ -393,35 +393,42 @@ var JpegImage = (function JpegImageClosure() { } var h, v; - while (mcu < mcuExpected) { + while (mcu <= mcuExpected) { // reset interval stuff var mcuToRead = resetInterval ? Math.min(mcuExpected - mcu, resetInterval) : mcuExpected; - for (i = 0; i < componentsLength; i++) { - components[i].pred = 0; - } - eobrun = 0; - if (componentsLength === 1) { - component = components[0]; - for (n = 0; n < mcuToRead; n++) { - decodeBlock(component, decodeFn, mcu); - mcu++; + // The `mcuToRead === 0` case should only occur when all of the expected + // MCU data has been already parsed, i.e. when `mcu === mcuExpected`, but + // some corrupt JPEG images contain more data than intended and we thus + // want to skip over any extra RSTx markers below (fixes issue11794.pdf). + if (mcuToRead > 0) { + for (i = 0; i < componentsLength; i++) { + components[i].pred = 0; } - } else { - for (n = 0; n < mcuToRead; n++) { - for (i = 0; i < componentsLength; i++) { - component = components[i]; - h = component.h; - v = component.v; - for (j = 0; j < v; j++) { - for (k = 0; k < h; k++) { - decodeMcu(component, decodeFn, mcu, j, k); + eobrun = 0; + + if (componentsLength === 1) { + component = components[0]; + for (n = 0; n < mcuToRead; n++) { + decodeBlock(component, decodeFn, mcu); + mcu++; + } + } else { + for (n = 0; n < mcuToRead; n++) { + for (i = 0; i < componentsLength; i++) { + component = components[i]; + h = component.h; + v = component.v; + for (j = 0; j < v; j++) { + for (k = 0; k < h; k++) { + decodeMcu(component, decodeFn, mcu, j, k); + } } } + mcu++; } - mcu++; } } @@ -434,8 +441,9 @@ var JpegImage = (function JpegImageClosure() { if (fileMarker.invalid) { // Some bad images seem to pad Scan blocks with e.g. zero bytes, skip // past those to attempt to find a valid marker (fixes issue4090.pdf). + const partialMsg = mcuToRead > 0 ? "unexpected" : "excessive"; warn( - `decodeScan - unexpected MCU data, current marker is: ${fileMarker.invalid}` + `decodeScan - ${partialMsg} MCU data, current marker is: ${fileMarker.invalid}` ); offset = fileMarker.offset; } diff --git a/test/pdfs/issue11794.pdf.link b/test/pdfs/issue11794.pdf.link new file mode 100644 index 000000000..e1d1ed7a4 --- /dev/null +++ b/test/pdfs/issue11794.pdf.link @@ -0,0 +1 @@ +https://github.com/mozilla/pdf.js/files/4459214/test.pdf diff --git a/test/test_manifest.json b/test/test_manifest.json index 18880029e..b242e1ae5 100644 --- a/test/test_manifest.json +++ b/test/test_manifest.json @@ -2468,6 +2468,14 @@ "link": true, "type": "eq" }, + { "id": "issue11794", + "file": "pdfs/issue11794.pdf", + "md5": "00d17b10a5fd7c06cddd7a0d2066ecdd", + "rounds": 1, + "link": true, + "lastPage": 1, + "type": "eq" + }, { "id": "bug852992", "file": "pdfs/bug852992_reduced.pdf",