Get rid of some longjmp() usage in CFX_BmpDecompressor. Stop using longjmp() (via CFX_BmpDecompressor::Error()) in CFX_BmpDecompressor::DecodeImage() and related methods. Return |kFail| instead, which is ultimately what the longjmp() does anyway. Along the way, push longjmp() calls out of CFX_BmpDecompressor::SetHeight(), and into its callers, and simplify ProgressiveDecoder::BmpContinueDecode(), which calls into CFX_BmpDecompressor. Bug: chromium:1017253 Change-Id: I707b929fda4bb9c6c5c7f2bda68da6ee45c0dcdb Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/61650 Commit-Queue: Lei Zhang <thestig@chromium.org> Reviewed-by: Nicolás Peña Moreno <npm@chromium.org>
diff --git a/core/fxcodec/bmp/bmpmodule.cpp b/core/fxcodec/bmp/bmpmodule.cpp index 1987f08..d7472a8 100644 --- a/core/fxcodec/bmp/bmpmodule.cpp +++ b/core/fxcodec/bmp/bmpmodule.cpp
@@ -54,11 +54,7 @@ } BmpModule::Status BmpModule::LoadImage(Context* pContext) { - auto* ctx = static_cast<CFX_BmpContext*>(pContext); - if (setjmp(*ctx->m_Bmp.jmpbuf())) - return Status::kFail; - - return ctx->m_Bmp.DecodeImage(); + return static_cast<CFX_BmpContext*>(pContext)->m_Bmp.DecodeImage(); } FX_FILESIZE BmpModule::GetAvailInput(Context* pContext) const {
diff --git a/core/fxcodec/bmp/cfx_bmpdecompressor.cpp b/core/fxcodec/bmp/cfx_bmpdecompressor.cpp index 0713429..3adda59 100644 --- a/core/fxcodec/bmp/cfx_bmpdecompressor.cpp +++ b/core/fxcodec/bmp/cfx_bmpdecompressor.cpp
@@ -166,7 +166,10 @@ reinterpret_cast<uint8_t*>(&bmp_info_header.biXPelsPerMeter))); dpi_y_ = static_cast<int32_t>(FXDWORD_GET_LSBFIRST( reinterpret_cast<uint8_t*>(&bmp_info_header.biYPelsPerMeter))); - SetHeight(signed_height); + if (!SetHeight(signed_height)) { + Error(); + NOTREACHED(); + } return true; } @@ -208,7 +211,10 @@ reinterpret_cast<uint8_t*>(&bmp_info_header.biXPelsPerMeter)); dpi_y_ = FXDWORD_GET_LSBFIRST( reinterpret_cast<uint8_t*>(&bmp_info_header.biYPelsPerMeter)); - SetHeight(signed_height); + if (!SetHeight(signed_height)) { + Error(); + NOTREACHED(); + } if (compress_flag_ != kBmpRgb || bi_planes != 1 || color_used_ != 0) { Error(); NOTREACHED(); @@ -354,17 +360,14 @@ input_buffer_->Seek(0); if (!GetDataPosition(header_offset_)) { decode_status_ = DecodeStatus::kTail; - Error(); - NOTREACHED(); + return BmpModule::Status::kFail; } row_num_ = 0; SaveDecodingStatus(DecodeStatus::kData); } - if (decode_status_ != DecodeStatus::kData || !ValidateFlag()) { - Error(); - NOTREACHED(); - } + if (decode_status_ != DecodeStatus::kData || !ValidateFlag()) + return BmpModule::Status::kFail; switch (compress_flag_) { case kBmpRgb: @@ -379,13 +382,8 @@ } } -bool CFX_BmpDecompressor::ValidateColorIndex(uint8_t val) { - if (val >= pal_num_) { - Error(); - NOTREACHED(); - } - - return true; +bool CFX_BmpDecompressor::ValidateColorIndex(uint8_t val) const { + return val < pal_num_; } BmpModule::Status CFX_BmpDecompressor::DecodeRGB() { @@ -477,8 +475,7 @@ case kRleEol: { if (row_num_ >= height_) { SaveDecodingStatus(DecodeStatus::kTail); - Error(); - NOTREACHED(); + return BmpModule::Status::kFail; } ReadNextScanline(); @@ -500,10 +497,8 @@ col_num_ += delta[0]; size_t bmp_row_num__next = row_num_ + delta[1]; - if (col_num_ >= out_row_bytes_ || bmp_row_num__next >= height_) { - Error(); - NOTREACHED(); - } + if (col_num_ >= out_row_bytes_ || bmp_row_num__next >= height_) + return BmpModule::Status::kFail; while (row_num_ < bmp_row_num__next) { std::fill(out_row_buffer_.begin(), out_row_buffer_.end(), 0); @@ -513,10 +508,8 @@ } default: { int32_t avail_size = out_row_bytes_ - col_num_; - if (!avail_size || static_cast<int32_t>(first_part) > avail_size) { - Error(); - NOTREACHED(); - } + if (!avail_size || static_cast<int32_t>(first_part) > avail_size) + return BmpModule::Status::kFail; size_t second_part_size = first_part & 1 ? first_part + 1 : first_part; @@ -538,10 +531,8 @@ } default: { int32_t avail_size = out_row_bytes_ - col_num_; - if (!avail_size || static_cast<int32_t>(first_part) > avail_size) { - Error(); - NOTREACHED(); - } + if (!avail_size || static_cast<int32_t>(first_part) > avail_size) + return BmpModule::Status::kFail; uint8_t second_part; if (!ReadData(&second_part, sizeof(second_part))) @@ -555,8 +546,7 @@ } } } - Error(); - NOTREACHED(); + return BmpModule::Status::kFail; } BmpModule::Status CFX_BmpDecompressor::DecodeRLE4() { @@ -575,8 +565,7 @@ case kRleEol: { if (row_num_ >= height_) { SaveDecodingStatus(DecodeStatus::kTail); - Error(); - NOTREACHED(); + return BmpModule::Status::kFail; } ReadNextScanline(); @@ -598,10 +587,8 @@ col_num_ += delta[0]; size_t bmp_row_num__next = row_num_ + delta[1]; - if (col_num_ >= out_row_bytes_ || bmp_row_num__next >= height_) { - Error(); - NOTREACHED(); - } + if (col_num_ >= out_row_bytes_ || bmp_row_num__next >= height_) + return BmpModule::Status::kFail; while (row_num_ < bmp_row_num__next) { std::fill(out_row_buffer_.begin(), out_row_buffer_.end(), 0); @@ -611,16 +598,12 @@ } default: { int32_t avail_size = out_row_bytes_ - col_num_; - if (!avail_size) { - Error(); - NOTREACHED(); - } + if (!avail_size) + return BmpModule::Status::kFail; uint8_t size = HalfRoundUp(first_part); if (static_cast<int32_t>(first_part) > avail_size) { - if (size + (col_num_ >> 1) > src_row_bytes_) { - Error(); - NOTREACHED(); - } + if (size + (col_num_ >> 1) > src_row_bytes_) + return BmpModule::Status::kFail; first_part = avail_size - 1; } @@ -644,17 +627,13 @@ } default: { int32_t avail_size = out_row_bytes_ - col_num_; - if (!avail_size) { - Error(); - NOTREACHED(); - } + if (!avail_size) + return BmpModule::Status::kFail; if (static_cast<int32_t>(first_part) > avail_size) { uint8_t size = HalfRoundUp(first_part); - if (size + (col_num_ >> 1) > src_row_bytes_) { - Error(); - NOTREACHED(); - } + if (size + (col_num_ >> 1) > src_row_bytes_) + return BmpModule::Status::kFail; first_part = avail_size - 1; } @@ -674,8 +653,7 @@ } } } - Error(); - NOTREACHED(); + return BmpModule::Status::kFail; } bool CFX_BmpDecompressor::ReadData(uint8_t* destination, uint32_t size) { @@ -698,17 +676,17 @@ return input_buffer_->GetSize() - input_buffer_->GetPosition(); } -void CFX_BmpDecompressor::SetHeight(int32_t signed_height) { +bool CFX_BmpDecompressor::SetHeight(int32_t signed_height) { if (signed_height >= 0) { height_ = signed_height; - return; + return true; } - if (signed_height == std::numeric_limits<int>::min()) { - Error(); - NOTREACHED(); + if (signed_height != std::numeric_limits<int>::min()) { + height_ = -signed_height; + img_tb_flag_ = true; + return true; } - height_ = -signed_height; - img_tb_flag_ = true; + return false; } } // namespace fxcodec
diff --git a/core/fxcodec/bmp/cfx_bmpdecompressor.h b/core/fxcodec/bmp/cfx_bmpdecompressor.h index 7d2fd03..e9761b5 100644 --- a/core/fxcodec/bmp/cfx_bmpdecompressor.h +++ b/core/fxcodec/bmp/cfx_bmpdecompressor.h
@@ -64,9 +64,9 @@ BmpModule::Status DecodeRLE4(); bool ReadData(uint8_t* destination, uint32_t size); void SaveDecodingStatus(DecodeStatus status); - bool ValidateColorIndex(uint8_t val); + bool ValidateColorIndex(uint8_t val) const; bool ValidateFlag() const; - void SetHeight(int32_t signed_height); + bool SetHeight(int32_t signed_height); jmp_buf jmpbuf_; UnownedPtr<CFX_BmpContext> const context_;
diff --git a/core/fxcodec/progressivedecoder.cpp b/core/fxcodec/progressivedecoder.cpp index 87223d6..d6e65f9 100644 --- a/core/fxcodec/progressivedecoder.cpp +++ b/core/fxcodec/progressivedecoder.cpp
@@ -797,29 +797,25 @@ m_status = FXCODEC_STATUS_ERR_MEMORY; return m_status; } - while (true) { - BmpModule::Status read_res = pBmpModule->LoadImage(m_pBmpContext.get()); - while (read_res == BmpModule::Status::kContinue) { - FXCODEC_STATUS error_status = FXCODEC_STATUS_DECODE_FINISH; - if (!BmpReadMoreData(pBmpModule, m_pBmpContext.get(), error_status)) { - m_pDeviceBitmap = nullptr; - m_pFile = nullptr; - m_status = error_status; - return m_status; - } - read_res = pBmpModule->LoadImage(m_pBmpContext.get()); - } - if (read_res == BmpModule::Status::kSuccess) { + + BmpModule::Status read_res = pBmpModule->LoadImage(m_pBmpContext.get()); + while (read_res == BmpModule::Status::kContinue) { + FXCODEC_STATUS error_status = FXCODEC_STATUS_DECODE_FINISH; + if (!BmpReadMoreData(pBmpModule, m_pBmpContext.get(), error_status)) { m_pDeviceBitmap = nullptr; m_pFile = nullptr; - m_status = FXCODEC_STATUS_DECODE_FINISH; + m_status = error_status; return m_status; } - m_pDeviceBitmap = nullptr; - m_pFile = nullptr; - m_status = FXCODEC_STATUS_ERROR; - return m_status; + read_res = pBmpModule->LoadImage(m_pBmpContext.get()); } + + m_pDeviceBitmap = nullptr; + m_pFile = nullptr; + m_status = read_res == BmpModule::Status::kSuccess + ? FXCODEC_STATUS_DECODE_FINISH + : FXCODEC_STATUS_ERROR; + return m_status; } #endif // PDF_ENABLE_XFA_BMP