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
