From 2772a2d944c8573aa187339b0b5ea059d1293c36 Mon Sep 17 00:00:00 2001
From: Lei Zhang <thestig@chromium.org>
Date: Thu, 12 Apr 2018 20:58:56 +0000
Subject: More CPDF_Color improvements.

Make Copy() take a const-ref parameter. As is, the parameter is a
pointer and it is dereferenced without any checks, and there are no
crash reports as a result of that.

Also mention GetPattern() should only be called when IsPattern() returns
true.

Change-Id: Ice3b7c941532d5a312fdd8f0c032e08d1ee1c6b5
Reviewed-on: https://pdfium-review.googlesource.com/30430
Reviewed-by: Henrique Nakashima <hnakashima@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
---
 core/fpdfapi/page/cpdf_color.cpp          | 10 ++++++----
 core/fpdfapi/page/cpdf_color.h            |  5 ++++-
 core/fpdfapi/page/cpdf_colorstate.cpp     |  4 ++--
 core/fpdfapi/render/cpdf_renderstatus.cpp |  4 ++--
 4 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/core/fpdfapi/page/cpdf_color.cpp b/core/fpdfapi/page/cpdf_color.cpp
index 73e3d36ec4..080df2fa28 100644
--- a/core/fpdfapi/page/cpdf_color.cpp
+++ b/core/fpdfapi/page/cpdf_color.cpp
@@ -119,10 +119,10 @@ void CPDF_Color::SetValue(CPDF_Pattern* pPattern,
   }
 }
 
-void CPDF_Color::Copy(const CPDF_Color* pSrc) {
+void CPDF_Color::Copy(const CPDF_Color& src) {
   ReleaseBuffer();
   ReleaseColorSpace();
-  m_pCS = pSrc->m_pCS;
+  m_pCS = src.m_pCS;
   if (!m_pCS)
     return;
 
@@ -134,7 +134,7 @@ void CPDF_Color::Copy(const CPDF_Color* pSrc) {
       return;
   }
   m_pBuffer = m_pCS->CreateBuf();
-  memcpy(m_pBuffer, pSrc->m_pBuffer, m_pCS->GetBufSize());
+  memcpy(m_pBuffer, src.m_pBuffer, m_pCS->GetBufSize());
   if (!IsPatternInternal())
     return;
 
@@ -164,7 +164,9 @@ bool CPDF_Color::GetRGB(int* R, int* G, int* B) const {
 }
 
 CPDF_Pattern* CPDF_Color::GetPattern() const {
-  if (!m_pBuffer || !IsPatternInternal())
+  ASSERT(IsPattern());
+
+  if (!m_pBuffer)
     return nullptr;
 
   PatternValue* pvalue = reinterpret_cast<PatternValue*>(m_pBuffer);
diff --git a/core/fpdfapi/page/cpdf_color.h b/core/fpdfapi/page/cpdf_color.h
index 2eac7cfacb..31bc125d83 100644
--- a/core/fpdfapi/page/cpdf_color.h
+++ b/core/fpdfapi/page/cpdf_color.h
@@ -20,14 +20,17 @@ class CPDF_Color {
   bool IsNull() const { return !m_pBuffer; }
   bool IsPattern() const;
 
-  void Copy(const CPDF_Color* pSrc);
+  void Copy(const CPDF_Color& src);
 
   void SetColorSpace(CPDF_ColorSpace* pCS);
   void SetValue(const float* comp);
   void SetValue(CPDF_Pattern* pPattern, const float* comp, uint32_t ncomps);
 
   bool GetRGB(int* R, int* G, int* B) const;
+
+  // Should only be called if IsPattern() returns true.
   CPDF_Pattern* GetPattern() const;
+
   const CPDF_ColorSpace* GetColorSpace() const { return m_pCS; }
 
  protected:
diff --git a/core/fpdfapi/page/cpdf_colorstate.cpp b/core/fpdfapi/page/cpdf_colorstate.cpp
index 227fd62045..b1f14c88b1 100644
--- a/core/fpdfapi/page/cpdf_colorstate.cpp
+++ b/core/fpdfapi/page/cpdf_colorstate.cpp
@@ -147,8 +147,8 @@ CPDF_ColorState::ColorData::ColorData()
 CPDF_ColorState::ColorData::ColorData(const ColorData& src)
     : m_FillColorRef(src.m_FillColorRef),
       m_StrokeColorRef(src.m_StrokeColorRef) {
-  m_FillColor.Copy(&src.m_FillColor);
-  m_StrokeColor.Copy(&src.m_StrokeColor);
+  m_FillColor.Copy(src.m_FillColor);
+  m_StrokeColor.Copy(src.m_StrokeColor);
 }
 
 CPDF_ColorState::ColorData::~ColorData() {}
diff --git a/core/fpdfapi/render/cpdf_renderstatus.cpp b/core/fpdfapi/render/cpdf_renderstatus.cpp
index 1f9e60d29d..8ebced72f7 100644
--- a/core/fpdfapi/render/cpdf_renderstatus.cpp
+++ b/core/fpdfapi/render/cpdf_renderstatus.cpp
@@ -1051,13 +1051,13 @@ bool CPDF_RenderStatus::Initialize(CPDF_RenderContext* pContext,
         m_InitialStates.m_ColorState.SetFillColorRef(
             pParentState->m_InitialStates.m_ColorState.GetFillColorRef());
         m_InitialStates.m_ColorState.GetMutableFillColor()->Copy(
-            pParentState->m_InitialStates.m_ColorState.GetFillColor());
+            *pParentState->m_InitialStates.m_ColorState.GetFillColor());
       }
       if (!m_InitialStates.m_ColorState.HasStrokeColor()) {
         m_InitialStates.m_ColorState.SetStrokeColorRef(
             pParentState->m_InitialStates.m_ColorState.GetFillColorRef());
         m_InitialStates.m_ColorState.GetMutableStrokeColor()->Copy(
-            pParentState->m_InitialStates.m_ColorState.GetStrokeColor());
+            *pParentState->m_InitialStates.m_ColorState.GetStrokeColor());
       }
     }
   } else {
-- 
cgit v1.2.3