From 0afbad0509578a5fee6fec4394d6b3c55425cf28 Mon Sep 17 00:00:00 2001 From: Oliver Chang Date: Fri, 20 Nov 2015 10:01:48 -0800 Subject: Merge to XFA: Change |CCodec_ScanlineDecoder::m_Pitch| to FX_DWORD This matches the type of the corresponding |CFX_DIBSource::m_Pitch|, where integer overflow is checked for FX_DWORD. This change is propagated to many other places. Also, check for integer overflow in |CCodec_RLScanlineDecoder::Create| during the calculation of |m_Pitch| since it aligns to 4 bytes while overflow was was previously checked without this alignment. TBR=tsepez@chromium.org BUG=555784 Review URL: https://codereview.chromium.org/1460033002 . (cherry picked from commit e7950df70a2fd658f466751b29483436cb31e829) Review URL: https://codereview.chromium.org/1461363002 . --- .../fpdf_parser_decode_embeddertest.cpp | 10 +++++++++ core/src/fxcodec/codec/codec_int.h | 6 ++--- core/src/fxcodec/codec/fx_codec.cpp | 26 +++++++++++++++------- core/src/fxcodec/codec/fx_codec_fax.cpp | 5 +++-- core/src/fxcodec/codec/fx_codec_flate.cpp | 26 ++++++++++++++-------- core/src/fxcodec/codec/fx_codec_jpeg.cpp | 6 +++-- 6 files changed, 55 insertions(+), 24 deletions(-) (limited to 'core') diff --git a/core/src/fpdfapi/fpdf_parser/fpdf_parser_decode_embeddertest.cpp b/core/src/fpdfapi/fpdf_parser/fpdf_parser_decode_embeddertest.cpp index a5a198e7b2..c80770366b 100644 --- a/core/src/fpdfapi/fpdf_parser/fpdf_parser_decode_embeddertest.cpp +++ b/core/src/fpdfapi/fpdf_parser/fpdf_parser_decode_embeddertest.cpp @@ -105,4 +105,14 @@ TEST_F(FPDFParserDecodeEmbeddertest, Bug_552046) { UnloadPage(page); } +TEST_F(FPDFParserDecodeEmbeddertest, Bug_555784) { + // Tests bad input to the run length decoder that caused a heap overflow. + // Should not cause a crash when rendered. + EXPECT_TRUE(OpenDocument("bug_555784.pdf")); + FPDF_PAGE page = LoadPage(0); + FPDF_BITMAP bitmap = RenderPage(page); + FPDFBitmap_Destroy(bitmap); + UnloadPage(page); +} + #undef TEST_CASE diff --git a/core/src/fxcodec/codec/codec_int.h b/core/src/fxcodec/codec/codec_int.h index 5513abce73..3d2acaf8c0 100644 --- a/core/src/fxcodec/codec/codec_int.h +++ b/core/src/fxcodec/codec/codec_int.h @@ -58,7 +58,7 @@ class CCodec_ScanlineDecoder : public ICodec_ScanlineDecoder { protected: class ImageDataCache { public: - ImageDataCache(int width, int height, int pitch); + ImageDataCache(int width, int height, FX_DWORD pitch); ~ImageDataCache(); bool AllocateCache(); @@ -75,7 +75,7 @@ class CCodec_ScanlineDecoder : public ICodec_ScanlineDecoder { const int m_Width; const int m_Height; - const int m_Pitch; + const FX_DWORD m_Pitch; int m_nCachedLines; nonstd::unique_ptr m_Data; }; @@ -93,7 +93,7 @@ class CCodec_ScanlineDecoder : public ICodec_ScanlineDecoder { int m_OutputHeight; int m_nComps; int m_bpc; - int m_Pitch; + FX_DWORD m_Pitch; FX_BOOL m_bColorTransformed; int m_NextLine; uint8_t* m_pLastScanline; diff --git a/core/src/fxcodec/codec/fx_codec.cpp b/core/src/fxcodec/codec/fx_codec.cpp index b357f8ac08..1efccf9af5 100644 --- a/core/src/fxcodec/codec/fx_codec.cpp +++ b/core/src/fxcodec/codec/fx_codec.cpp @@ -29,15 +29,14 @@ CCodec_ModuleMgr::CCodec_ModuleMgr() CCodec_ScanlineDecoder::ImageDataCache::ImageDataCache(int width, int height, - int pitch) - : m_Width(width), m_Height(height), m_Pitch(pitch), m_nCachedLines(0) { -} + FX_DWORD pitch) + : m_Width(width), m_Height(height), m_Pitch(pitch), m_nCachedLines(0) {} CCodec_ScanlineDecoder::ImageDataCache::~ImageDataCache() { } bool CCodec_ScanlineDecoder::ImageDataCache::AllocateCache() { - if (m_Pitch <= 0 || m_Height < 0) + if (m_Pitch == 0 || m_Height < 0) return false; FX_SAFE_SIZE_T size = m_Pitch; @@ -51,7 +50,7 @@ bool CCodec_ScanlineDecoder::ImageDataCache::AllocateCache() { void CCodec_ScanlineDecoder::ImageDataCache::AppendLine(const uint8_t* line) { // If the callers adds more lines than there is room, fail. - if (m_Pitch <= 0 || m_nCachedLines >= m_Height) { + if (m_Pitch == 0 || m_nCachedLines >= m_Height) { NOTREACHED(); return; } @@ -62,7 +61,7 @@ void CCodec_ScanlineDecoder::ImageDataCache::AppendLine(const uint8_t* line) { } const uint8_t* CCodec_ScanlineDecoder::ImageDataCache::GetLine(int line) const { - if (m_Pitch <= 0 || line < 0 || line >= m_nCachedLines) + if (m_Pitch == 0 || line < 0 || line >= m_nCachedLines) return nullptr; size_t offset = m_Pitch; @@ -364,8 +363,19 @@ FX_BOOL CCodec_RLScanlineDecoder::Create(const uint8_t* src_buf, m_bpc = bpc; m_bColorTransformed = FALSE; m_DownScale = 1; - m_Pitch = (width * nComps * bpc + 31) / 32 * 4; - m_dwLineBytes = (width * nComps * bpc + 7) / 8; + // Aligning the pitch to 4 bytes requires an integer overflow check. + FX_SAFE_DWORD pitch = width; + pitch *= nComps; + pitch *= bpc; + pitch += 31; + pitch /= 32; + pitch *= 4; + if (!pitch.IsValid()) { + return FALSE; + } + m_Pitch = pitch.ValueOrDie(); + // Overflow should already have been checked before this is called. + m_dwLineBytes = (static_cast(width) * nComps * bpc + 7) / 8; m_pScanline = FX_Alloc(uint8_t, m_Pitch); return CheckDestSize(); } diff --git a/core/src/fxcodec/codec/fx_codec_fax.cpp b/core/src/fxcodec/codec/fx_codec_fax.cpp index d376fe293b..b198e74784 100644 --- a/core/src/fxcodec/codec/fx_codec_fax.cpp +++ b/core/src/fxcodec/codec/fx_codec_fax.cpp @@ -656,7 +656,8 @@ FX_BOOL CCodec_FaxDecoder::Create(const uint8_t* src_buf, if (m_OrigHeight == 0) { m_OrigHeight = height; } - m_Pitch = (m_OrigWidth + 31) / 32 * 4; + // Should not overflow. Checked by FPDFAPI_CreateFaxDecoder. + m_Pitch = (static_cast(m_OrigWidth) + 31) / 32 * 4; m_OutputWidth = m_OrigWidth; m_OutputHeight = m_OrigHeight; m_pScanlineBuf = FX_Alloc(uint8_t, m_Pitch); @@ -716,7 +717,7 @@ uint8_t* CCodec_FaxDecoder::v_GetNextLine() { } } if (m_bBlack) { - for (int i = 0; i < m_Pitch; i++) { + for (FX_DWORD i = 0; i < m_Pitch; i++) { m_pScanlineBuf[i] = ~m_pScanlineBuf[i]; } } diff --git a/core/src/fxcodec/codec/fx_codec_flate.cpp b/core/src/fxcodec/codec/fx_codec_flate.cpp index 8fba3af882..f9959589b3 100644 --- a/core/src/fxcodec/codec/fx_codec_flate.cpp +++ b/core/src/fxcodec/codec/fx_codec_flate.cpp @@ -558,7 +558,7 @@ FX_BOOL TIFF_PredictorEncode(uint8_t*& data_buf, } void TIFF_PredictLine(uint8_t* dest_buf, - int row_size, + FX_DWORD row_size, int BitsPerComponent, int Colors, int Columns) { @@ -582,7 +582,7 @@ void TIFF_PredictLine(uint8_t* dest_buf, } int BytesPerPixel = BitsPerComponent * Colors / 8; if (BitsPerComponent == 16) { - for (int i = BytesPerPixel; i < row_size; i += 2) { + for (FX_DWORD i = BytesPerPixel; i < row_size; i += 2) { FX_WORD pixel = (dest_buf[i - BytesPerPixel] << 8) | dest_buf[i - BytesPerPixel + 1]; pixel += (dest_buf[i] << 8) | dest_buf[i + 1]; @@ -590,7 +590,7 @@ void TIFF_PredictLine(uint8_t* dest_buf, dest_buf[i + 1] = (uint8_t)pixel; } } else { - for (int i = BytesPerPixel; i < row_size; i++) { + for (FX_DWORD i = BytesPerPixel; i < row_size; i++) { dest_buf[i] += dest_buf[i - BytesPerPixel]; } } @@ -761,7 +761,11 @@ class CCodec_FlateScanlineDecoder : public CCodec_ScanlineDecoder { uint8_t* m_pPredictBuffer; uint8_t* m_pPredictRaw; int m_Predictor; - int m_Colors, m_BitsPerComponent, m_Columns, m_PredictPitch, m_LeftOver; + int m_Colors; + int m_BitsPerComponent; + int m_Columns; + FX_DWORD m_PredictPitch; + size_t m_LeftOver; }; CCodec_FlateScanlineDecoder::CCodec_FlateScanlineDecoder() { @@ -798,7 +802,7 @@ void CCodec_FlateScanlineDecoder::Create(const uint8_t* src_buf, m_nComps = nComps; m_bpc = bpc; m_bColorTransformed = FALSE; - m_Pitch = (width * nComps * bpc + 7) / 8; + m_Pitch = (static_cast(width) * nComps * bpc + 7) / 8; m_pScanline = FX_Alloc(uint8_t, m_Pitch); m_Predictor = 0; if (predictor) { @@ -816,7 +820,10 @@ void CCodec_FlateScanlineDecoder::Create(const uint8_t* src_buf, m_Colors = Colors; m_BitsPerComponent = BitsPerComponent; m_Columns = Columns; - m_PredictPitch = (m_BitsPerComponent * m_Colors * m_Columns + 7) / 8; + m_PredictPitch = + (static_cast(m_BitsPerComponent) * m_Colors * m_Columns + + 7) / + 8; m_pLastLine = FX_Alloc(uint8_t, m_PredictPitch); m_pPredictRaw = FX_Alloc(uint8_t, m_PredictPitch + 1); m_pPredictBuffer = FX_Alloc(uint8_t, m_PredictPitch); @@ -849,8 +856,9 @@ uint8_t* CCodec_FlateScanlineDecoder::v_GetNextLine() { m_OutputWidth); } } else { - int bytes_to_go = m_Pitch; - int read_leftover = m_LeftOver > bytes_to_go ? bytes_to_go : m_LeftOver; + size_t bytes_to_go = m_Pitch; + size_t read_leftover = + m_LeftOver > bytes_to_go ? bytes_to_go : m_LeftOver; if (read_leftover) { FXSYS_memcpy(m_pScanline, m_pPredictBuffer + m_PredictPitch - m_LeftOver, @@ -869,7 +877,7 @@ uint8_t* CCodec_FlateScanlineDecoder::v_GetNextLine() { TIFF_PredictLine(m_pPredictBuffer, m_PredictPitch, m_BitsPerComponent, m_Colors, m_Columns); } - int read_bytes = + size_t read_bytes = m_PredictPitch > bytes_to_go ? bytes_to_go : m_PredictPitch; FXSYS_memcpy(m_pScanline + m_Pitch - bytes_to_go, m_pPredictBuffer, read_bytes); diff --git a/core/src/fxcodec/codec/fx_codec_jpeg.cpp b/core/src/fxcodec/codec/fx_codec_jpeg.cpp index c06253d785..b59deb5a0d 100644 --- a/core/src/fxcodec/codec/fx_codec_jpeg.cpp +++ b/core/src/fxcodec/codec/fx_codec_jpeg.cpp @@ -427,7 +427,9 @@ FX_BOOL CCodec_JpegDecoder::Create(const uint8_t* src_buf, if ((int)cinfo.image_width < width) { return FALSE; } - m_Pitch = (cinfo.image_width * cinfo.num_components + 3) / 4 * 4; + m_Pitch = + (static_cast(cinfo.image_width) * cinfo.num_components + 3) / + 4 * 4; m_pScanlineBuf = FX_Alloc(uint8_t, m_Pitch); m_nComps = cinfo.num_components; m_bpc = 8; @@ -461,7 +463,7 @@ void CCodec_JpegDecoder::v_DownScale(int dest_width, int dest_height) { FX_GetDownsampleRatio(m_OrigWidth, m_OrigHeight, dest_width, dest_height); m_OutputWidth = (m_OrigWidth + m_DownScale - 1) / m_DownScale; m_OutputHeight = (m_OrigHeight + m_DownScale - 1) / m_DownScale; - m_Pitch = (m_OutputWidth * m_nComps + 3) / 4 * 4; + m_Pitch = (static_cast(m_OutputWidth) * m_nComps + 3) / 4 * 4; if (old_scale != m_DownScale) { m_NextLine = -1; } -- cgit v1.2.3