Relax colorspace checks in CPDF_DIBSource::CreateDecoder().

core/fpdfapi/render/fpdf_render_loadimage.cpp

Index: core/fpdfapi/render/fpdf_render_loadimage.cpp
diff --git a/core/fpdfapi/render/fpdf_render_loadimage.cpp b/core/fpdfapi/render/fpdf_render_loadimage.cpp
index c3fe99018c575e577f70044a1460f517b6bc18fe..f5a101e186edf55f03410103cda361c721b711c3 100644
--- a/core/fpdfapi/render/fpdf_render_loadimage.cpp
+++ b/core/fpdfapi/render/fpdf_render_loadimage.cpp
@@ -58,6 +58,10 @@ bool IsAllowedBPCValue(int bpc) {
   return bpc == 1 || bpc == 2 || bpc == 4 || bpc == 8 || bpc == 16;
 }
 
+bool IsAllowedICCComponents(int nComp) {
+  return nComp == 1 || nComp == 3 || nComp == 4;
+}
+
 template <typename T>
 T ClampValue(T value, T max_value) {
   value = std::min(value, max_value);
@@ -545,9 +549,34 @@ int CPDF_DIBSource::CreateDecoder() {
   const uint8_t* src_data = m_pStreamAcc->GetData();

[Tom Sepez, 2016/11/09 17:26:37]

nit: can these move down some?

[Lei Zhang, 2016/11/09 22:23:49]

Done.

   uint32_t src_size = m_pStreamAcc->GetSize();
   const CPDF_Dictionary* pParams = m_pStreamAcc->GetImageParam();
+  if (decoder == "JPXDecode") {
+    LoadJpxBitmap();
+    return m_pCachedBitmap ? 1 : 0;
+  }
+  if (decoder == "JBIG2Decode") {
+    m_pCachedBitmap = pdfium::MakeUnique<CFX_DIBitmap>();
+    if (!m_pCachedBitmap->Create(
+            m_Width, m_Height, m_bImageMask ? FXDIB_1bppMask : FXDIB_1bppRgb)) {
+      m_pCachedBitmap.reset();
+      return 0;
+    }
+    m_Status = 1;
+    return 2;
+  }
+
   if (decoder == "CCITTFaxDecode") {
     m_pDecoder.reset(FPDFAPI_CreateFaxDecoder(src_data, src_size, m_Width,
                                               m_Height, pParams));
+  } else if (decoder == "FlateDecode") {
+    m_pDecoder.reset(FPDFAPI_CreateFlateDecoder(
+        src_data, src_size, m_Width, m_Height, m_nComponents, m_bpc, pParams));
+  } else if (decoder == "RunLengthDecode") {
+    m_pDecoder.reset(CPDF_ModuleMgr::Get()
+                         ->GetCodecModule()
+                         ->GetBasicModule()
+                         ->CreateRunLengthDecoder(src_data, src_size, m_Width,
+                                                  m_Height, m_nComponents,
+                                                  m_bpc));
   } else if (decoder == "DCTDecode") {
     m_pDecoder.reset(CPDF_ModuleMgr::Get()->GetJpegModule()->CreateDecoder(
         src_data, src_size, m_Width, m_Height, m_nComponents,
@@ -563,12 +592,41 @@ int CPDF_DIBSource::CreateDecoder() {
           FX_Free(m_pCompData);
           m_pCompData = nullptr;
           m_nComponents = static_cast<uint32_t>(comps);
-          if (m_pColorSpace &&
-              m_pColorSpace->CountComponents() != m_nComponents) {
-            return 0;
+          if (m_pColorSpace) {
+            switch (m_Family) {

[Tom Sepez, 2016/11/09 17:26:37]

Do we want to bound the components here to the expected number?  I wonder if
there's code dowstream that does indexing by component into some fixed sized
array of the expected component size ...

[Lei Zhang, 2016/11/09 22:27:56]

I don't know what is correct for all the cases. I'm trying to make sure for some
easy to understand ones, the colorspace has at least enough components to
accommodate what's in the image. If there is a mismatch, previously the image
would not render at all, and now it would render, possibly poorly.

I'll try more PDFs outside of our public corpus to see if there are some that
have colorspaces.

In the elephant example, it's family is PDFCS_ICCBASED, the colorspace has 3
components, and the image only has 1.

+              case PDFCS_DEVICEGRAY:
+              case PDFCS_DEVICERGB:
+              case PDFCS_DEVICECMYK: {
+                uint32_t dwMinComps = ComponentsForFamily(m_Family);
+                if (m_pColorSpace->CountComponents() < dwMinComps ||
+                    m_nComponents < dwMinComps) {
+                  return 0;
+                }
+                break;
+              }
+              case PDFCS_LAB: {
+                if (m_nComponents != 3 || m_pColorSpace->CountComponents() < 3)
+                  return 0;
+                break;
+              }
+              case PDFCS_ICCBASED: {
+                if (!IsAllowedICCComponents(m_nComponents) ||
+                    !IsAllowedICCComponents(m_pColorSpace->CountComponents()) ||
+                    m_pColorSpace->CountComponents() < m_nComponents) {
+                  return 0;
+                }
+                break;
+              }
+              default: {
+                if (m_pColorSpace->CountComponents() != m_nComponents)
+                  return 0;
+                break;
+              }
+            }
+          } else {
+            if (m_Family == PDFCS_LAB && m_nComponents != 3)
+              return 0;
           }
-          if (m_Family == PDFCS_LAB && m_nComponents != 3)
-            return 0;
           m_pCompData = GetDecodeAndMaskArray(m_bDefaultDecode, m_bColorKey);
           if (!m_pCompData)
             return 0;
@@ -578,28 +636,6 @@ int CPDF_DIBSource::CreateDecoder() {
             src_data, src_size, m_Width, m_Height, m_nComponents, bTransform));
       }
     }
-  } else if (decoder == "FlateDecode") {
-    m_pDecoder.reset(FPDFAPI_CreateFlateDecoder(
-        src_data, src_size, m_Width, m_Height, m_nComponents, m_bpc, pParams));
-  } else if (decoder == "JPXDecode") {
-    LoadJpxBitmap();
-    return m_pCachedBitmap ? 1 : 0;
-  } else if (decoder == "JBIG2Decode") {
-    m_pCachedBitmap.reset(new CFX_DIBitmap);
-    if (!m_pCachedBitmap->Create(
-            m_Width, m_Height, m_bImageMask ? FXDIB_1bppMask : FXDIB_1bppRgb)) {
-      m_pCachedBitmap.reset();
-      return 0;
-    }
-    m_Status = 1;
-    return 2;
-  } else if (decoder == "RunLengthDecode") {
-    m_pDecoder.reset(CPDF_ModuleMgr::Get()
-                         ->GetCodecModule()
-                         ->GetBasicModule()
-                         ->CreateRunLengthDecoder(src_data, src_size, m_Width,
-                                                  m_Height, m_nComponents,
-                                                  m_bpc));
   }
   if (!m_pDecoder)
     return 0;