From f6f68c75ce54a5865fb19dcb075e7734f1639663 Mon Sep 17 00:00:00 2001 From: Ryan Harrison Date: Fri, 7 Jul 2017 14:03:55 -0400 Subject: Check that there is enough data remaining in source BMP before reading When reading in a BMP, after processing the header, make sure that there is enough data remaining in the source before proceeding. If not signal that the BMP is improperly formatted. BUG=chromium:738635 Change-Id: I506bc0e6db7dcd4b5984fd91a1f39516320a2037 Reviewed-on: https://pdfium-review.googlesource.com/7280 Reviewed-by: Tom Sepez Commit-Queue: Ryan Harrison --- core/fxcodec/codec/fx_codec_progress.cpp | 89 +++++++++++++++++++++----------- core/fxge/dib/cfx_dibitmap.cpp | 53 ++++++++++++------- core/fxge/dib/cfx_dibitmap.h | 8 ++- 3 files changed, 101 insertions(+), 49 deletions(-) diff --git a/core/fxcodec/codec/fx_codec_progress.cpp b/core/fxcodec/codec/fx_codec_progress.cpp index 9f74ac3624..e6832f2773 100644 --- a/core/fxcodec/codec/fx_codec_progress.cpp +++ b/core/fxcodec/codec/fx_codec_progress.cpp @@ -1015,16 +1015,18 @@ bool CCodec_ProgressiveDecoder::DetectImageType(FXCODEC_IMAGE_TYPE imageType, m_status = FXCODEC_STATUS_ERR_MEMORY; return false; } + m_pBmpContext = pBmpModule->Start(this); if (!m_pBmpContext) { m_status = FXCODEC_STATUS_ERR_MEMORY; return false; } - bool bResult = m_pFile->ReadBlock(m_pSrcBuf, 0, size); - if (!bResult) { + + if (!m_pFile->ReadBlock(m_pSrcBuf, 0, size)) { m_status = FXCODEC_STATUS_ERR_READ; return false; } + m_offSet += size; pBmpModule->Input(m_pBmpContext.get(), m_pSrcBuf, size); std::vector palette; @@ -1041,22 +1043,61 @@ bool CCodec_ProgressiveDecoder::DetectImageType(FXCODEC_IMAGE_TYPE imageType, m_pBmpContext.get(), &m_SrcWidth, &m_SrcHeight, &m_BmpIsTopBottom, &m_SrcComponents, &m_SrcPaletteNumber, &palette, pAttribute); } - if (readResult == 1) { - m_SrcBPC = 8; - m_clipBox = FX_RECT(0, 0, m_SrcWidth, m_SrcHeight); - FX_Free(m_pSrcPalette); - if (m_SrcPaletteNumber) { - m_pSrcPalette = FX_Alloc(FX_ARGB, m_SrcPaletteNumber); - memcpy(m_pSrcPalette, palette.data(), - m_SrcPaletteNumber * sizeof(uint32_t)); - } else { - m_pSrcPalette = nullptr; - } - return true; + + if (readResult != 1) { + m_pBmpContext.reset(); + m_status = FXCODEC_STATUS_ERR_FORMAT; + return false; } - m_pBmpContext.reset(); - m_status = FXCODEC_STATUS_ERR_FORMAT; - return false; + + FXDIB_Format format = FXDIB_Invalid; + switch (m_SrcComponents) { + case 1: + m_SrcFormat = FXCodec_8bppRgb; + format = FXDIB_8bppRgb; + break; + case 3: + m_SrcFormat = FXCodec_Rgb; + format = FXDIB_Rgb; + break; + case 4: + m_SrcFormat = FXCodec_Rgb32; + format = FXDIB_Rgb32; + break; + default: + m_pBmpContext.reset(); + m_status = FXCODEC_STATUS_ERR_FORMAT; + return false; + break; + } + + uint32_t pitch = 0; + uint32_t neededData = 0; + if (!CFX_DIBitmap::CalculatePitchAndSize(m_SrcWidth, m_SrcHeight, format, + &pitch, &neededData)) { + m_pBmpContext.reset(); + m_status = FXCODEC_STATUS_ERR_FORMAT; + return false; + } + + uint32_t availableData = m_SrcSize > m_offSet ? m_SrcSize - m_offSet : 0; + if (neededData > availableData) { + m_pBmpContext.reset(); + m_status = FXCODEC_STATUS_ERR_FORMAT; + return false; + } + + m_SrcBPC = 8; + m_clipBox = FX_RECT(0, 0, m_SrcWidth, m_SrcHeight); + FX_Free(m_pSrcPalette); + if (m_SrcPaletteNumber) { + m_pSrcPalette = FX_Alloc(FX_ARGB, m_SrcPaletteNumber); + memcpy(m_pSrcPalette, palette.data(), + m_SrcPaletteNumber * sizeof(FX_ARGB)); + } else { + m_pSrcPalette = nullptr; + } + return true; } case FXCODEC_IMAGE_JPG: { CCodec_JpegModule* pJpegModule = m_pCodecMgr->GetJpegModule(); @@ -1065,8 +1106,7 @@ bool CCodec_ProgressiveDecoder::DetectImageType(FXCODEC_IMAGE_TYPE imageType, m_status = FXCODEC_STATUS_ERR_MEMORY; return false; } - bool bResult = m_pFile->ReadBlock(m_pSrcBuf, 0, size); - if (!bResult) { + if (!m_pFile->ReadBlock(m_pSrcBuf, 0, size)) { m_status = FXCODEC_STATUS_ERR_READ; return false; } @@ -1965,17 +2005,6 @@ FXCODEC_STATUS CCodec_ProgressiveDecoder::StartDecode( m_status = FXCODEC_STATUS_ERR_MEMORY; return m_status; } - switch (m_SrcComponents) { - case 1: - m_SrcFormat = FXCodec_8bppRgb; - break; - case 3: - m_SrcFormat = FXCodec_Rgb; - break; - case 4: - m_SrcFormat = FXCodec_Rgb32; - break; - } GetTransMethod(m_pDeviceBitmap->GetFormat(), m_SrcFormat); m_ScanlineSize = (m_SrcWidth * m_SrcComponents + 3) / 4 * 4; FX_Free(m_pDecodeBuf); diff --git a/core/fxge/dib/cfx_dibitmap.cpp b/core/fxge/dib/cfx_dibitmap.cpp index 48cbc3291c..43b0da0edf 100644 --- a/core/fxge/dib/cfx_dibitmap.cpp +++ b/core/fxge/dib/cfx_dibitmap.cpp @@ -34,36 +34,32 @@ bool CFX_DIBitmap::Create(int width, int height, FXDIB_Format format, uint8_t* pBuffer, - int pitch) { + uint32_t pitch) { m_pBuffer = nullptr; m_bpp = static_cast(format); m_AlphaFlag = static_cast(format >> 8); - m_Width = m_Height = m_Pitch = 0; - if (width <= 0 || height <= 0 || pitch < 0) - return false; - - if ((INT_MAX - 31) / width < (format & 0xff)) - return false; - - if (!pitch) - pitch = (width * (format & 0xff) + 31) / 32 * 4; + m_Width = 0; + m_Height = 0; + m_Pitch = 0; - if ((1 << 30) / pitch < height) + uint32_t calculatedSize; + if (!CFX_DIBitmap::CalculatePitchAndSize(height, width, format, &pitch, + &calculatedSize)) return false; if (pBuffer) { m_pBuffer.Reset(pBuffer); } else { - int size = pitch * height + 4; - int oomlimit = MAX_OOM_LIMIT; - if (oomlimit >= 0 && size >= oomlimit) { - m_pBuffer = - std::unique_ptr(FX_TryAlloc(uint8_t, size)); + size_t bufferSize = calculatedSize + 4; + size_t oomlimit = MAX_OOM_LIMIT; + if (bufferSize >= oomlimit) { + m_pBuffer = std::unique_ptr( + FX_TryAlloc(uint8_t, bufferSize)); if (!m_pBuffer) return false; } else { - m_pBuffer = - std::unique_ptr(FX_Alloc(uint8_t, size)); + m_pBuffer = std::unique_ptr( + FX_Alloc(uint8_t, bufferSize)); } } m_Width = width; @@ -816,6 +812,27 @@ bool CFX_DIBitmap::ConvertColorScale(uint32_t forecolor, uint32_t backcolor) { return true; } +bool CFX_DIBitmap::CalculatePitchAndSize(int height, + int width, + FXDIB_Format format, + uint32_t* pitch, + uint32_t* size) { + if (width <= 0 || height <= 0) + return false; + + if ((INT_MAX - 31) / width < (format & 0xFF)) + return false; + + if (!*pitch) + *pitch = static_cast((width * (format & 0xff) + 31) / 32 * 4); + + if ((1 << 30) / *pitch < static_cast(height)) + return false; + + *size = *pitch * static_cast(height); + return true; +} + bool CFX_DIBitmap::CompositeBitmap( int dest_left, int dest_top, diff --git a/core/fxge/dib/cfx_dibitmap.h b/core/fxge/dib/cfx_dibitmap.h index df751db3df..2b5555befc 100644 --- a/core/fxge/dib/cfx_dibitmap.h +++ b/core/fxge/dib/cfx_dibitmap.h @@ -25,7 +25,7 @@ class CFX_DIBitmap : public CFX_DIBSource { int height, FXDIB_Format format, uint8_t* pBuffer = nullptr, - int pitch = 0); + uint32_t pitch = 0); bool Copy(const CFX_RetainPtr& pSrc); @@ -96,6 +96,12 @@ class CFX_DIBitmap : public CFX_DIBSource { bool ConvertColorScale(uint32_t forecolor, uint32_t backcolor); + static bool CalculatePitchAndSize(int height, + int width, + FXDIB_Format format, + uint32_t* pitch, + uint32_t* size); + #if defined _SKIA_SUPPORT_ || _SKIA_SUPPORT_PATHS_ void PreMultiply(); #endif -- cgit v1.2.3