From 8365e76fe8a311eaf77c4b5d5c0b199de0238f07 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Tue, 11 Sep 2018 07:57:48 +0000 Subject: Make HexDecode()'s buffer out parameter a unique_ptr. Do the same for A85Decode(). Change-Id: Ic6a0e5e8a63defa104e35e97eb9ce4223aac23a7 Reviewed-on: https://pdfium-review.googlesource.com/41851 Commit-Queue: Lei Zhang Reviewed-by: Tom Sepez --- core/fpdfapi/page/cpdf_streamparser.cpp | 15 ++++---- core/fpdfapi/parser/fpdf_parser_decode.cpp | 44 ++++++++++++++-------- core/fpdfapi/parser/fpdf_parser_decode.h | 5 ++- .../fpdfapi/parser/fpdf_parser_decode_unittest.cpp | 24 ++++++------ fpdfsdk/fpdf_attachment.cpp | 6 +-- 5 files changed, 54 insertions(+), 40 deletions(-) diff --git a/core/fpdfapi/page/cpdf_streamparser.cpp b/core/fpdfapi/page/cpdf_streamparser.cpp index 9533bb2a17..fe67634196 100644 --- a/core/fpdfapi/page/cpdf_streamparser.cpp +++ b/core/fpdfapi/page/cpdf_streamparser.cpp @@ -68,22 +68,24 @@ uint32_t DecodeInlineStream(pdfium::span src_span, CPDF_Dictionary* pParam, uint8_t** dest_buf, uint32_t* dest_size) { + *dest_buf = nullptr; + *dest_size = 0; if (decoder == "CCITTFaxDecode" || decoder == "CCF") { std::unique_ptr pDecoder = CreateFaxDecoder(src_span, width, height, pParam); return DecodeAllScanlines(std::move(pDecoder), dest_buf, dest_size); } + std::unique_ptr ignored_result; if (decoder == "ASCII85Decode" || decoder == "A85") - return A85Decode(src_span, dest_buf, dest_size); + return A85Decode(src_span, &ignored_result, dest_size); if (decoder == "ASCIIHexDecode" || decoder == "AHx") - return HexDecode(src_span, dest_buf, dest_size); + return HexDecode(src_span, &ignored_result, dest_size); if (decoder == "FlateDecode" || decoder == "Fl") { return FlateOrLZWDecode(false, src_span, pParam, *dest_size, dest_buf, dest_size); } - if (decoder == "LZWDecode" || decoder == "LZW") { + if (decoder == "LZWDecode" || decoder == "LZW") return FlateOrLZWDecode(true, src_span, pParam, 0, dest_buf, dest_size); - } if (decoder == "DCTDecode" || decoder == "DCT") { std::unique_ptr pDecoder = CPDF_ModuleMgr::Get()->GetJpegModule()->CreateDecoder( @@ -93,9 +95,8 @@ uint32_t DecodeInlineStream(pdfium::span src_span, } if (decoder == "RunLengthDecode" || decoder == "RL") return RunLengthDecode(src_span, dest_buf, dest_size); - *dest_size = 0; - *dest_buf = 0; - return 0xFFFFFFFF; + + return FX_INVALID_OFFSET; } } // namespace diff --git a/core/fpdfapi/parser/fpdf_parser_decode.cpp b/core/fpdfapi/parser/fpdf_parser_decode.cpp index 410c9a9b8d..efa60d8e3b 100644 --- a/core/fpdfapi/parser/fpdf_parser_decode.cpp +++ b/core/fpdfapi/parser/fpdf_parser_decode.cpp @@ -50,6 +50,10 @@ bool CheckFlateDecodeParams(int Colors, int BitsPerComponent, int Columns) { return check.ValueOrDie() <= INT_MAX - 7; } +uint8_t GetA85Result(uint32_t res, size_t i) { + return static_cast(res >> (3 - i) * 8); +} + } // namespace const uint16_t PDFDocEncoding[256] = { @@ -84,12 +88,13 @@ const uint16_t PDFDocEncoding[256] = { 0x00fc, 0x00fd, 0x00fe, 0x00ff}; uint32_t A85Decode(pdfium::span src_span, - uint8_t** dest_buf, + std::unique_ptr* dest_buf, uint32_t* dest_size) { *dest_size = 0; - *dest_buf = nullptr; - if (src_span.empty()) + if (src_span.empty()) { + dest_buf->reset(); return 0; + } // Count legal characters and zeros. uint32_t zcount = 0; @@ -111,10 +116,14 @@ uint32_t A85Decode(pdfium::span src_span, // Count the space needed to contain non-zero characters. The encoding ratio // of Ascii85 is 4:5. uint32_t space_for_non_zeroes = (pos - zcount) / 5 * 4 + 4; - if (zcount > (UINT_MAX - space_for_non_zeroes) / 4) + FX_SAFE_UINT32 size = zcount; + size *= 4; + size += space_for_non_zeroes; + if (!size.IsValid()) return FX_INVALID_OFFSET; - *dest_buf = FX_Alloc(uint8_t, zcount * 4 + space_for_non_zeroes); + dest_buf->reset(FX_Alloc(uint8_t, size.ValueOrDie())); + uint8_t* dest_buf_ptr = dest_buf->get(); size_t state = 0; uint32_t res = 0; pos = 0; @@ -124,7 +133,7 @@ uint32_t A85Decode(pdfium::span src_span, continue; if (ch == 'z') { - memset(*dest_buf + *dest_size, 0, 4); + memset(dest_buf_ptr + *dest_size, 0, 4); state = 0; res = 0; *dest_size += 4; @@ -142,7 +151,7 @@ uint32_t A85Decode(pdfium::span src_span, } for (size_t i = 0; i < 4; ++i) { - (*dest_buf)[(*dest_size)++] = static_cast(res >> (3 - i) * 8); + dest_buf_ptr[(*dest_size)++] = GetA85Result(res, i); } state = 0; res = 0; @@ -152,7 +161,7 @@ uint32_t A85Decode(pdfium::span src_span, for (size_t i = state; i < 5; ++i) res = res * 85 + 84; for (size_t i = 0; i < state - 1; ++i) - (*dest_buf)[(*dest_size)++] = static_cast(res >> (3 - i) * 8); + dest_buf_ptr[(*dest_size)++] = GetA85Result(res, i); } if (pos < src_span.size() && src_span[pos] == '>') ++pos; @@ -160,11 +169,11 @@ uint32_t A85Decode(pdfium::span src_span, } uint32_t HexDecode(pdfium::span src_span, - uint8_t** dest_buf, + std::unique_ptr* dest_buf, uint32_t* dest_size) { *dest_size = 0; if (src_span.empty()) { - *dest_buf = nullptr; + dest_buf->reset(); return 0; } @@ -173,7 +182,8 @@ uint32_t HexDecode(pdfium::span src_span, while (i < src_span.size() && src_span[i] != '>') ++i; - *dest_buf = FX_Alloc(uint8_t, i / 2 + 1); + dest_buf->reset(FX_Alloc(uint8_t, i / 2 + 1)); + uint8_t* dest_buf_ptr = dest_buf->get(); bool bFirst = true; for (i = 0; i < src_span.size(); ++i) { uint8_t ch = src_span[i]; @@ -189,9 +199,9 @@ uint32_t HexDecode(pdfium::span src_span, int digit = FXSYS_HexCharToInt(ch); if (bFirst) - (*dest_buf)[*dest_size] = digit * 16; + dest_buf_ptr[*dest_size] = digit * 16; else - (*dest_buf)[(*dest_size)++] += digit; + dest_buf_ptr[(*dest_size)++] += digit; bFirst = !bFirst; } if (!bFirst) @@ -384,9 +394,13 @@ bool PDF_DataDecode(pdfium::span src_span, offset = FlateOrLZWDecode(true, last_span, pParam, estimated_size, &new_buf, &new_size); } else if (decoder == "ASCII85Decode" || decoder == "A85") { - offset = A85Decode(last_span, &new_buf, &new_size); + std::unique_ptr result; + offset = A85Decode(last_span, &result, &new_size); + new_buf = result.release(); } else if (decoder == "ASCIIHexDecode" || decoder == "AHx") { - offset = HexDecode(last_span, &new_buf, &new_size); + std::unique_ptr result; + offset = HexDecode(last_span, &result, &new_size); + new_buf = result.release(); } else if (decoder == "RunLengthDecode" || decoder == "RL") { if (bImageAcc && i == nSize - 1) { *ImageEncoding = "RunLengthDecode"; diff --git a/core/fpdfapi/parser/fpdf_parser_decode.h b/core/fpdfapi/parser/fpdf_parser_decode.h index 1484dd7cf0..eb7beaccdf 100644 --- a/core/fpdfapi/parser/fpdf_parser_decode.h +++ b/core/fpdfapi/parser/fpdf_parser_decode.h @@ -9,6 +9,7 @@ #include +#include "core/fxcrt/fx_memory.h" #include "core/fxcrt/fx_string.h" #include "core/fxcrt/unowned_ptr.h" #include "third_party/base/span.h" @@ -51,11 +52,11 @@ uint32_t RunLengthDecode(pdfium::span src_span, uint32_t* dest_size); uint32_t A85Decode(pdfium::span src_span, - uint8_t** dest_buf, + std::unique_ptr* dest_buf, uint32_t* dest_size); uint32_t HexDecode(pdfium::span src_span, - uint8_t** dest_buf, + std::unique_ptr* dest_buf, uint32_t* dest_size); uint32_t FlateOrLZWDecode(bool bLZW, diff --git a/core/fpdfapi/parser/fpdf_parser_decode_unittest.cpp b/core/fpdfapi/parser/fpdf_parser_decode_unittest.cpp index aa37b796e2..b6d7679ef5 100644 --- a/core/fpdfapi/parser/fpdf_parser_decode_unittest.cpp +++ b/core/fpdfapi/parser/fpdf_parser_decode_unittest.cpp @@ -8,7 +8,7 @@ #include "testing/test_support.h" TEST(fpdf_parser_decode, A85Decode) { - pdfium::DecodeTestData test_data[] = { + const pdfium::DecodeTestData test_data[] = { // Empty src string. STR_IN_OUT_CASE("", "", 0), // Empty content in src string. @@ -27,23 +27,23 @@ TEST(fpdf_parser_decode, A85Decode) { STR_IN_OUT_CASE("FCfN8FCfN8vw", "testtest", 11), }; for (size_t i = 0; i < FX_ArraySize(test_data); ++i) { - pdfium::DecodeTestData* ptr = &test_data[i]; - uint8_t* result = nullptr; + const pdfium::DecodeTestData* ptr = &test_data[i]; + std::unique_ptr result; uint32_t result_size = 0; EXPECT_EQ(ptr->processed_size, A85Decode({ptr->input, ptr->input_size}, &result, &result_size)) << "for case " << i; ASSERT_EQ(ptr->expected_size, result_size); + const uint8_t* result_ptr = result.get(); for (size_t j = 0; j < result_size; ++j) { - EXPECT_EQ(ptr->expected[j], result[j]) << "for case " << i << " char " - << j; + EXPECT_EQ(ptr->expected[j], result_ptr[j]) + << "for case " << i << " char " << j; } - FX_Free(result); } } TEST(fpdf_parser_decode, HexDecode) { - pdfium::DecodeTestData test_data[] = { + const pdfium::DecodeTestData test_data[] = { // Empty src string. STR_IN_OUT_CASE("", "", 0), // Empty content in src string. @@ -62,18 +62,18 @@ TEST(fpdf_parser_decode, HexDecode) { STR_IN_OUT_CASE("12AcED3c3456", "\x12\xac\xed\x3c\x34\x56", 12), }; for (size_t i = 0; i < FX_ArraySize(test_data); ++i) { - pdfium::DecodeTestData* ptr = &test_data[i]; - uint8_t* result = nullptr; + const pdfium::DecodeTestData* ptr = &test_data[i]; + std::unique_ptr result; uint32_t result_size = 0; EXPECT_EQ(ptr->processed_size, HexDecode({ptr->input, ptr->input_size}, &result, &result_size)) << "for case " << i; ASSERT_EQ(ptr->expected_size, result_size); + const uint8_t* result_ptr = result.get(); for (size_t j = 0; j < result_size; ++j) { - EXPECT_EQ(ptr->expected[j], result[j]) << "for case " << i << " char " - << j; + EXPECT_EQ(ptr->expected[j], result_ptr[j]) + << "for case " << i << " char " << j; } - FX_Free(result); } } diff --git a/fpdfsdk/fpdf_attachment.cpp b/fpdfsdk/fpdf_attachment.cpp index f3c1c83cf9..7bc168025a 100644 --- a/fpdfsdk/fpdf_attachment.cpp +++ b/fpdfsdk/fpdf_attachment.cpp @@ -27,12 +27,10 @@ namespace { constexpr char kChecksumKey[] = "CheckSum"; ByteString CFXByteStringHexDecode(const ByteString& bsHex) { - uint8_t* result = nullptr; + std::unique_ptr result; uint32_t size = 0; HexDecode(bsHex.AsRawSpan(), &result, &size); - ByteString bsDecoded(result, size); - FX_Free(result); - return bsDecoded; + return ByteString(result.get(), size); } ByteString GenerateMD5Base16(const void* contents, const unsigned long len) { -- cgit v1.2.3