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 <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>

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<const uint8_t> 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<CCodec_ScanlineDecoder> pDecoder =
         CreateFaxDecoder(src_span, width, height, pParam);
     return DecodeAllScanlines(std::move(pDecoder), dest_buf, dest_size);
   }
+  std::unique_ptr<uint8_t, FxFreeDeleter> 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<CCodec_ScanlineDecoder> pDecoder =
         CPDF_ModuleMgr::Get()->GetJpegModule()->CreateDecoder(
@@ -93,9 +95,8 @@ uint32_t DecodeInlineStream(pdfium::span<const uint8_t> 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<uint8_t>(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<const uint8_t> src_span,
-                   uint8_t** dest_buf,
+                   std::unique_ptr<uint8_t, FxFreeDeleter>* 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<const uint8_t> 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<const uint8_t> 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<const uint8_t> src_span,
     }
 
     for (size_t i = 0; i < 4; ++i) {
-      (*dest_buf)[(*dest_size)++] = static_cast<uint8_t>(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<const uint8_t> 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<uint8_t>(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<const uint8_t> src_span,
 }
 
 uint32_t HexDecode(pdfium::span<const uint8_t> src_span,
-                   uint8_t** dest_buf,
+                   std::unique_ptr<uint8_t, FxFreeDeleter>* 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<const uint8_t> 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<const uint8_t> 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<const uint8_t> 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<uint8_t, FxFreeDeleter> 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<uint8_t, FxFreeDeleter> 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 <memory>
 
+#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<const uint8_t> src_span,
                          uint32_t* dest_size);
 
 uint32_t A85Decode(pdfium::span<const uint8_t> src_span,
-                   uint8_t** dest_buf,
+                   std::unique_ptr<uint8_t, FxFreeDeleter>* dest_buf,
                    uint32_t* dest_size);
 
 uint32_t HexDecode(pdfium::span<const uint8_t> src_span,
-                   uint8_t** dest_buf,
+                   std::unique_ptr<uint8_t, FxFreeDeleter>* 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<uint8_t, FxFreeDeleter> 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<uint8_t, FxFreeDeleter> 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<uint8_t, FxFreeDeleter> 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) {