commit d91ef173878574803000e455d882092cdd4f7104
author Hui Yingst <nigi@chromium.org> Thu Jul 14 18:50:38 2022 +0000
committer Pdfium LUCI CQ <pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com> Thu Jul 14 18:50:38 2022 +0000
tree ea1d43432ed7a249be66eee71468a6339b7c7755
parent ceaa31b826614b69a1f8d2f1149186d9491a9ac0

Avoid integer overflow in CPDF_ToUnicodeMap::HandleBeginBFRange()

In CPDF_ToUnicodeMap::HandleBeginBFRange(), there are 3 for-loops
which increase `code` values in the range between `lowcode` and
`highcode`. If `highcode` is 0xffffffff, the "code++" operation will
cause integer overflow and these for-loops will never be terminated.

Make `code` FX_SAFE_UINT32 type so that its value can be validated
for each for-loop to avoid integer overflow.

Bug: chromium:1343510
Change-Id: Icb8221ce026fade90b01151c6f675b9a4d3b2f2b
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/95310
Commit-Queue: Nigi <nigi@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>

core/fpdfapi/font/cpdf_tounicodemap.cpp

diff --git a/core/fpdfapi/font/cpdf_tounicodemap.cpp b/core/fpdfapi/font/cpdf_tounicodemap.cpp
index 4fa92bf..4ec5a3c 100644
--- a/core/fpdfapi/font/cpdf_tounicodemap.cpp
+++ b/core/fpdfapi/font/cpdf_tounicodemap.cpp
@@ -173,8 +173,10 @@
 
     ByteStringView start = pParser->GetWord();
     if (start == "[") {
-      for (uint32_t code = lowcode; code <= highcode; code++)
-        SetCode(code, StringToWideString(pParser->GetWord()));
+      for (FX_SAFE_UINT32 code = lowcode;
+           code.IsValid() && code.ValueOrDie() <= highcode; code++) {
+        SetCode(code.ValueOrDie(), StringToWideString(pParser->GetWord()));
+      }
       pParser->GetWord();
       continue;
     }
@@ -186,13 +188,17 @@
         return;
 
       uint32_t value = value_or_error.value();
-      for (uint32_t code = lowcode; code <= highcode; code++)
-        m_Multimap.emplace(code, value++);
+      for (FX_SAFE_UINT32 code = lowcode;
+           code.IsValid() && code.ValueOrDie() <= highcode; code++) {
+        m_Multimap.emplace(code.ValueOrDie(), value++);
+      }
     } else {
-      for (uint32_t code = lowcode; code <= highcode; code++) {
+      for (FX_SAFE_UINT32 code = lowcode;
+           code.IsValid() && code.ValueOrDie() <= highcode; code++) {
+        uint32_t code_value = code.ValueOrDie();
         WideString retcode =
-            code == lowcode ? destcode : StringDataAdd(destcode);
-        m_Multimap.emplace(code, GetMultiCharIndexIndicator());
+            code_value == lowcode ? destcode : StringDataAdd(destcode);
+        m_Multimap.emplace(code_value, GetMultiCharIndexIndicator());
         m_MultiCharVec.push_back(retcode);
         destcode = std::move(retcode);
       }

core/fpdfapi/font/cpdf_tounicodemap_unittest.cpp

diff --git a/core/fpdfapi/font/cpdf_tounicodemap_unittest.cpp b/core/fpdfapi/font/cpdf_tounicodemap_unittest.cpp
index 7dacc5a..48e7c2f 100644
--- a/core/fpdfapi/font/cpdf_tounicodemap_unittest.cpp
+++ b/core/fpdfapi/font/cpdf_tounicodemap_unittest.cpp
@@ -4,8 +4,11 @@
 
 #include "core/fpdfapi/font/cpdf_tounicodemap.h"
 
+#include "core/fpdfapi/parser/cpdf_stream.h"
+#include "core/fxcrt/retain_ptr.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/base/span.h"
 
 TEST(cpdf_tounicodemap, StringToCode) {
   EXPECT_THAT(CPDF_ToUnicodeMap::StringToCode("<0001>"), testing::Optional(1u));
@@ -47,3 +50,32 @@
   EXPECT_EQ(res, CPDF_ToUnicodeMap::StringToWideString("<c2abFaAb>"));
   EXPECT_EQ(res, CPDF_ToUnicodeMap::StringToWideString("<c2abFaAb12>"));
 }
+
+TEST(cpdf_tounicodemap, HandleBeginBFRangeAvoidIntegerOverflow) {
+  // Make sure there won't be infinite loops due to integer overflows in
+  // HandleBeginBFRange().
+  {
+    static constexpr uint8_t kInput1[] =
+        "beginbfrange<FFFFFFFF><FFFFFFFF>[<0041>]endbfrange";
+    auto stream = pdfium::MakeRetain<CPDF_Stream>();
+    stream->SetData(pdfium::make_span(kInput1));
+    CPDF_ToUnicodeMap map(stream.Get());
+    EXPECT_STREQ(L"A", map.Lookup(0xffffffff).c_str());
+  }
+  {
+    static constexpr uint8_t kInput2[] =
+        "beginbfrange<FFFFFFFF><FFFFFFFF><0042>endbfrange";
+    auto stream = pdfium::MakeRetain<CPDF_Stream>();
+    stream->SetData(pdfium::make_span(kInput2));
+    CPDF_ToUnicodeMap map(stream.Get());
+    EXPECT_STREQ(L"B", map.Lookup(0xffffffff).c_str());
+  }
+  {
+    static constexpr uint8_t kInput3[] =
+        "beginbfrange<FFFFFFFF><FFFFFFFF><00410042>endbfrange";
+    auto stream = pdfium::MakeRetain<CPDF_Stream>();
+    stream->SetData(pdfium::make_span(kInput3));
+    CPDF_ToUnicodeMap map(stream.Get());
+    EXPECT_STREQ(L"AB", map.Lookup(0xffffffff).c_str());
+  }
+}