commit bafc408dc3010c48b105f852901b2a2a9659f89e
author Tom Sepez <tsepez@chromium.org> Wed Dec 15 21:33:15 2021 +0000
committer Pdfium LUCI CQ <pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Dec 15 21:33:15 2021 +0000
tree bac0cba6544d603336169a3cea17233a80511b67
parent 8436313566e6ad9c82e73888db468b315026ce8f

Fix NULL segv in {Byte,Wide}String::MakeLower.

It is possible to enter MakeLower with an allocated buffer of zero
length, because strings are greedy about holding onto their buffers.
This won't be converted to nullptr until ReallocBeforeWrite() makes
a private copy for update, which being a new zero-length string, gets
no buffer.

-- Add tests that checks strings in this state.

Bug: b/208309865
Change-Id: I1243bed16b2ec386ac66ca1704998c602aa16e6d
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/88130
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>

core/fxcrt/bytestring.cpp

diff --git a/core/fxcrt/bytestring.cpp b/core/fxcrt/bytestring.cpp
index ef3abb6..48a92e4 100644
--- a/core/fxcrt/bytestring.cpp
+++ b/core/fxcrt/bytestring.cpp
@@ -577,7 +577,7 @@
 }
 
 void ByteString::MakeLower() {
-  if (!m_pData)
+  if (IsEmpty())
     return;
 
   ReallocBeforeWrite(m_pData->m_nDataLength);
@@ -585,7 +585,7 @@
 }
 
 void ByteString::MakeUpper() {
-  if (!m_pData)
+  if (IsEmpty())
     return;
 
   ReallocBeforeWrite(m_pData->m_nDataLength);
@@ -593,7 +593,7 @@
 }
 
 size_t ByteString::Remove(char chRemove) {
-  if (!m_pData || m_pData->m_nDataLength == 0)
+  if (IsEmpty())
     return 0;
 
   char* pstrSource = m_pData->m_String;

core/fxcrt/bytestring_unittest.cpp

diff --git a/core/fxcrt/bytestring_unittest.cpp b/core/fxcrt/bytestring_unittest.cpp
index 7cbbd2e..8056b6f 100644
--- a/core/fxcrt/bytestring_unittest.cpp
+++ b/core/fxcrt/bytestring_unittest.cpp
@@ -803,6 +803,17 @@
   EXPECT_EQ("", empty);
   empty.MakeUpper();
   EXPECT_EQ("", empty);
+
+  ByteString empty_with_buffer("x");
+  empty_with_buffer.Delete(0);
+
+  ByteString additional_empty_with_buffer_ref = empty_with_buffer;
+  additional_empty_with_buffer_ref.MakeLower();
+  EXPECT_EQ("", additional_empty_with_buffer_ref);
+
+  additional_empty_with_buffer_ref = empty_with_buffer;
+  additional_empty_with_buffer_ref.MakeUpper();
+  EXPECT_EQ("", additional_empty_with_buffer_ref);
 }
 
 TEST(ByteString, Trim) {

core/fxcrt/widestring.cpp

diff --git a/core/fxcrt/widestring.cpp b/core/fxcrt/widestring.cpp
index 38f56d1..2fc31bc 100644
--- a/core/fxcrt/widestring.cpp
+++ b/core/fxcrt/widestring.cpp
@@ -809,7 +809,7 @@
 }
 
 void WideString::MakeLower() {
-  if (!m_pData)
+  if (IsEmpty())
     return;
 
   ReallocBeforeWrite(m_pData->m_nDataLength);
@@ -817,7 +817,7 @@
 }
 
 void WideString::MakeUpper() {
-  if (!m_pData)
+  if (IsEmpty())
     return;
 
   ReallocBeforeWrite(m_pData->m_nDataLength);
@@ -825,7 +825,7 @@
 }
 
 size_t WideString::Remove(wchar_t chRemove) {
-  if (!m_pData || m_pData->m_nDataLength == 0)
+  if (IsEmpty())
     return 0;
 
   wchar_t* pstrSource = m_pData->m_String;

core/fxcrt/widestring_unittest.cpp

diff --git a/core/fxcrt/widestring_unittest.cpp b/core/fxcrt/widestring_unittest.cpp
index b05f26b..a1c331e 100644
--- a/core/fxcrt/widestring_unittest.cpp
+++ b/core/fxcrt/widestring_unittest.cpp
@@ -809,6 +809,17 @@
   EXPECT_EQ(L"", empty);
   empty.MakeUpper();
   EXPECT_EQ(L"", empty);
+
+  WideString empty_with_buffer(L"x");
+  empty_with_buffer.Delete(0);
+
+  WideString additional_empty_with_buffer_ref = empty_with_buffer;
+  additional_empty_with_buffer_ref.MakeLower();
+  EXPECT_EQ(L"", additional_empty_with_buffer_ref);
+
+  additional_empty_with_buffer_ref = empty_with_buffer;
+  additional_empty_with_buffer_ref.MakeUpper();
+  EXPECT_EQ(L"", additional_empty_with_buffer_ref);
 }
 
 TEST(WideString, Trim) {