summaryrefslogtreecommitdiff
path: root/core/fxcodec/codec
diff options
context:
space:
mode:
authorRyan Harrison <rharrison@chromium.org>2018-01-17 18:12:16 +0000
committerChromium commit bot <commit-bot@chromium.org>2018-01-17 18:12:16 +0000
commitb73a96938d0fc932a9d498359c98f4cf6ef34160 (patch)
tree9223a1b178dc7ceedff28dd96df294d3f3eaddca /core/fxcodec/codec
parent6d6a243893532de40f636dbdc61d10c04ab019fb (diff)
downloadpdfium-b73a96938d0fc932a9d498359c98f4cf6ef34160.tar.xz
Correctly handle errors when starting jpeg codec
The current implementation treats both returning false and longjmp'ing out of jpeg_start_decompress as indicating that the decompression has paused and needs more data. This is incorrect, in reality only the false return value indicates this. The longjmp path indicates a fatal error in the processing of the jpeg. The default implementation actually calls exit() in this case, and the documentation explicitly calls out that in this case recovery isn't possible and the decode process will have to start from scratch. This resolves a situation where the progressive decoder would get a malformed jpeg and keep on grabbing blocks from it and try to start decoding it. This would eventually fail when it ran out of data to read, but would cause a large memory leak and a crash on the MSAN fuzzers. BUG=pdfium:986,chromium:798665 Change-Id: Ifd2ed7a2dc46fa20bab34e9c461a8d4c4718c4d7 Reviewed-on: https://pdfium-review.googlesource.com/23072 Reviewed-by: dsinclair <dsinclair@chromium.org> Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Diffstat (limited to 'core/fxcodec/codec')
-rw-r--r--core/fxcodec/codec/ccodec_jpegmodule.h2
-rw-r--r--core/fxcodec/codec/fx_codec_jpeg.cpp8
-rw-r--r--core/fxcodec/codec/fx_codec_progress.cpp13
3 files changed, 17 insertions, 6 deletions
diff --git a/core/fxcodec/codec/ccodec_jpegmodule.h b/core/fxcodec/codec/ccodec_jpegmodule.h
index 9f4c80d08d..aed01bda9b 100644
--- a/core/fxcodec/codec/ccodec_jpegmodule.h
+++ b/core/fxcodec/codec/ccodec_jpegmodule.h
@@ -7,6 +7,7 @@
#ifndef CORE_FXCODEC_CODEC_CCODEC_JPEGMODULE_H_
#define CORE_FXCODEC_CODEC_CCODEC_JPEGMODULE_H_
+#include <csetjmp>
#include <memory>
#include "core/fxcrt/fx_system.h"
@@ -24,6 +25,7 @@ class CCodec_JpegModule {
class Context {
public:
virtual ~Context() {}
+ virtual jmp_buf* GetJumpMark() = 0;
};
std::unique_ptr<CCodec_ScanlineDecoder> CreateDecoder(const uint8_t* src_buf,
diff --git a/core/fxcodec/codec/fx_codec_jpeg.cpp b/core/fxcodec/codec/fx_codec_jpeg.cpp
index 5b3c25d0f7..4d4adfd817 100644
--- a/core/fxcodec/codec/fx_codec_jpeg.cpp
+++ b/core/fxcodec/codec/fx_codec_jpeg.cpp
@@ -34,6 +34,8 @@ class CJpegContext : public CCodec_JpegModule::Context {
CJpegContext();
~CJpegContext() override;
+ jmp_buf* GetJumpMark() override { return &m_JumpMark; }
+
jmp_buf m_JumpMark;
jpeg_decompress_struct m_Info;
jpeg_error_mgr m_ErrMgr;
@@ -476,11 +478,11 @@ int CCodec_JpegModule::ReadHeader(Context* pContext,
}
bool CCodec_JpegModule::StartScanline(Context* pContext, int down_scale) {
- auto* ctx = static_cast<CJpegContext*>(pContext);
- if (setjmp(ctx->m_JumpMark) == -1)
+ if (down_scale < 0)
return false;
- ctx->m_Info.scale_denom = down_scale;
+ auto* ctx = static_cast<CJpegContext*>(pContext);
+ ctx->m_Info.scale_denom = static_cast<unsigned int>(down_scale);
return !!jpeg_start_decompress(&ctx->m_Info);
}
diff --git a/core/fxcodec/codec/fx_codec_progress.cpp b/core/fxcodec/codec/fx_codec_progress.cpp
index ea9cdd2ea9..c9436186ab 100644
--- a/core/fxcodec/codec/fx_codec_progress.cpp
+++ b/core/fxcodec/codec/fx_codec_progress.cpp
@@ -1869,10 +1869,15 @@ FXCODEC_STATUS CCodec_ProgressiveDecoder::StartDecode(
case FXCODEC_IMAGE_JPG: {
int down_scale = 1;
GetDownScale(down_scale);
+ // Setting jump marker before calling StartScanLine, since a longjmp to
+ // the marker indicates a fatal error.
+ if (setjmp(*m_pJpegContext->GetJumpMark()) == -1)
+ return FXCODEC_STATUS_ERROR;
+
CCodec_JpegModule* pJpegModule = m_pCodecMgr->GetJpegModule();
- bool bStart =
+ bool startStatus =
pJpegModule->StartScanline(m_pJpegContext.get(), down_scale);
- while (!bStart) {
+ while (!startStatus) {
FXCODEC_STATUS error_status = FXCODEC_STATUS_ERROR;
if (!JpegReadMoreData(pJpegModule, error_status)) {
m_pDeviceBitmap = nullptr;
@@ -1880,7 +1885,9 @@ FXCODEC_STATUS CCodec_ProgressiveDecoder::StartDecode(
m_status = error_status;
return m_status;
}
- bStart = pJpegModule->StartScanline(m_pJpegContext.get(), down_scale);
+
+ startStatus =
+ pJpegModule->StartScanline(m_pJpegContext.get(), down_scale);
}
int scanline_size = (m_SrcWidth + down_scale - 1) / down_scale;
scanline_size = (scanline_size * m_SrcComponents + 3) / 4 * 4;