From ef73cf5838ab3a902872d9fc57a90621cc3d7f21 Mon Sep 17 00:00:00 2001 From: Nicolas Pena Date: Fri, 12 May 2017 14:36:06 -0400 Subject: Rename ErrorData and fix potential leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This CL fixes a leak that can be caused by a longjmp in ErrorData. The method is renamed to express the fact that it includes such, and a followup should remove the jmps altogether. Bug: chromium:721488 Change-Id: Iefcc82a77a30ff77b7973b05611440a8d5bf275e Reviewed-on: https://pdfium-review.googlesource.com/5450 Commit-Queue: Nicolás Peña Reviewed-by: Tom Sepez --- core/fxcodec/codec/fx_codec_progress.cpp | 3 +-- core/fxcodec/lgif/cgifcontext.cpp | 2 +- core/fxcodec/lgif/cgifcontext.h | 3 ++- core/fxcodec/lgif/fx_gif.cpp | 29 ++++++++++++++++++++--------- 4 files changed, 24 insertions(+), 13 deletions(-) (limited to 'core') diff --git a/core/fxcodec/codec/fx_codec_progress.cpp b/core/fxcodec/codec/fx_codec_progress.cpp index aa97ac4bb2..4cbae5cc75 100644 --- a/core/fxcodec/codec/fx_codec_progress.cpp +++ b/core/fxcodec/codec/fx_codec_progress.cpp @@ -1804,8 +1804,7 @@ FXCODEC_STATUS CCodec_ProgressiveDecoder::GetFrames(int32_t& frames) { m_status = FXCODEC_STATUS_DECODE_READY; return m_status; } - if (m_pGifContext.get()) - m_pGifContext = nullptr; + m_pGifContext = nullptr; m_status = FXCODEC_STATUS_ERROR; return m_status; } diff --git a/core/fxcodec/lgif/cgifcontext.cpp b/core/fxcodec/lgif/cgifcontext.cpp index aa2aba6075..b7a513c7e8 100644 --- a/core/fxcodec/lgif/cgifcontext.cpp +++ b/core/fxcodec/lgif/cgifcontext.cpp @@ -33,7 +33,7 @@ CGifContext::CGifContext(CCodec_GifModule* gif_module, char* error_string) CGifContext::~CGifContext() {} -void CGifContext::ErrorData(const char* err_msg) { +void CGifContext::ThrowError(const char* err_msg) { strncpy(err_ptr, err_msg, GIF_MAX_ERROR_SIZE - 1); longjmp(jmpbuf, 1); } diff --git a/core/fxcodec/lgif/cgifcontext.h b/core/fxcodec/lgif/cgifcontext.h index 2ee8fbe396..e65d6c7b0b 100644 --- a/core/fxcodec/lgif/cgifcontext.h +++ b/core/fxcodec/lgif/cgifcontext.h @@ -21,7 +21,8 @@ class CGifContext { CGifContext(CCodec_GifModule* gif_module, char* error_string); ~CGifContext(); - void ErrorData(const char* err_msg); + // TODO(npm): Remove longjmp from this method!!! + void ThrowError(const char* err_msg); void RecordCurrentPosition(uint32_t* cur_pos_ptr); void ReadScanline(int32_t row_num, uint8_t* row_buf); bool GetRecordPosition(uint32_t cur_pos, diff --git a/core/fxcodec/lgif/fx_gif.cpp b/core/fxcodec/lgif/fx_gif.cpp index b9ff41cc2e..dd80dc65f7 100644 --- a/core/fxcodec/lgif/fx_gif.cpp +++ b/core/fxcodec/lgif/fx_gif.cpp @@ -11,6 +11,7 @@ #include "core/fxcodec/lbmp/fx_bmp.h" #include "core/fxcodec/lgif/cgifcontext.h" +#include "third_party/base/logging.h" #include "third_party/base/ptr_util.h" #include "third_party/base/stl_util.h" @@ -118,7 +119,8 @@ GifDecodeStatus gif_decode_extension(CGifContext* context) { GifDecodeStatus gif_decode_image_info(CGifContext* context) { if (context->width == 0 || context->height == 0) { - context->ErrorData("No Image Header Info"); + context->ThrowError("No Image Header Info"); + NOTREACHED(); return GifDecodeStatus::Error; } uint32_t skip_size_org = context->skip_size; @@ -141,7 +143,9 @@ GifDecodeStatus gif_decode_image_info(CGifContext* context) { context->width || gif_image->m_ImageInfo.top + gif_image->m_ImageInfo.height > context->height) { - context->ErrorData("Image Data Out Of LSD, The File May Be Corrupt"); + gif_image = nullptr; + context->ThrowError("Image Data Out Of LSD, The File May Be Corrupt"); + NOTREACHED(); return GifDecodeStatus::Error; } GifLF* gif_img_info_lf_ptr = (GifLF*)&gif_img_info_ptr->local_flag; @@ -178,7 +182,8 @@ void gif_decoding_failure_at_tail_cleanup(CGifContext* context, GifImage* gif_image_ptr) { gif_image_ptr->m_ImageRowBuf.clear(); gif_save_decoding_status(context, GIF_D_STATUS_TAIL); - context->ErrorData("Decode Image Data Error"); + context->ThrowError("Decode Image Data Error"); + NOTREACHED(); } } // namespace @@ -361,7 +366,8 @@ GifDecodeStatus gif_read_header(CGifContext* context) { if (strncmp(gif_header_ptr->signature, GIF_SIGNATURE, 3) != 0 || gif_header_ptr->version[0] != '8' || gif_header_ptr->version[2] != 'a') { - context->ErrorData("Not A Gif Image"); + context->ThrowError("Not A Gif Image"); + NOTREACHED(); return GifDecodeStatus::Error; } GifLSD* gif_lsd_ptr = nullptr; @@ -499,7 +505,8 @@ GifDecodeStatus gif_load_frame(CGifContext* context, int32_t frame_num) { GifImage* gif_image_ptr = context->m_Images[frame_num].get(); uint32_t gif_img_row_bytes = gif_image_ptr->m_ImageInfo.width; if (gif_img_row_bytes == 0) { - context->ErrorData("Error Invalid Number of Row Bytes"); + context->ThrowError("Error Invalid Number of Row Bytes"); + NOTREACHED(); return GifDecodeStatus::Error; } if (context->decode_status == GIF_D_STATUS_TAIL) { @@ -522,7 +529,8 @@ GifDecodeStatus gif_load_frame(CGifContext* context, int32_t frame_num) { (bool)((GifLF*)&gif_image_ptr->m_ImageInfo.local_flag)->interlace); if (!bRes) { gif_image_ptr->m_ImageRowBuf.clear(); - context->ErrorData("Error Read Record Position Data"); + context->ThrowError("Error Read Record Position Data"); + NOTREACHED(); return GifDecodeStatus::Error; } } else { @@ -540,13 +548,15 @@ GifDecodeStatus gif_load_frame(CGifContext* context, int32_t frame_num) { (bool)((GifLF*)&gif_image_ptr->m_ImageInfo.local_flag)->interlace); if (!bRes) { gif_image_ptr->m_ImageRowBuf.clear(); - context->ErrorData("Error Read Record Position Data"); + context->ThrowError("Error Read Record Position Data"); + NOTREACHED(); return GifDecodeStatus::Error; } } if (gif_image_ptr->image_code_size >= 32) { gif_image_ptr->m_ImageRowBuf.clear(); - context->ErrorData("Error Invalid Code Size"); + context->ThrowError("Error Invalid Code Size"); + NOTREACHED(); return GifDecodeStatus::Error; } if (!context->m_ImgDecoder.get()) @@ -643,7 +653,8 @@ GifDecodeStatus gif_load_frame(CGifContext* context, int32_t frame_num) { } gif_save_decoding_status(context, GIF_D_STATUS_TAIL); } - context->ErrorData("Decode Image Data Error"); + context->ThrowError("Decode Image Data Error"); + NOTREACHED(); return GifDecodeStatus::Error; } -- cgit v1.2.3