Prevent an OOB access in CPDF_DIBSource::TranslateScanline24bpp

core/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp

 // Copyright 2014 PDFium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
 #include "core/fpdfapi/fpdf_render/render_int.h"
 
 #include <algorithm>
 #include <memory>
@@ skipping 899 matching lines @@
     m_bpc = 0;
 }
 
 void CPDF_DIBSource::TranslateScanline24bpp(uint8_t* dest_scan,
                                             const uint8_t* src_scan) const {
   if (m_bpc == 0) {
     return;
   }
   unsigned int max_data = (1 << m_bpc) - 1;
   if (m_bDefaultDecode) {
-    if (m_Family == PDFCS_DEVICERGB || m_Family == PDFCS_CALRGB) {
+    if ((m_Family == PDFCS_DEVICERGB || m_Family == PDFCS_CALRGB) &&
+        m_nComponents == 3) {

[Tom Sepez, 2016/04/12 00:39:27]

Do we just want to return in error if m_nComponents is not 3 for these families?
 Or do we get good output if we continue?

[Oliver Chang, 2016/04/12 00:48:59]

I'm not sure if we get good output, but it doesn't look like the rest of the
code in this function makes similarly incorrect assumptions. Would it be better
to just return early to be safe? 

[Oliver Chang, 2016/04/12 16:36:22]

Done.

       const uint8_t* src_pos = src_scan;
       switch (m_bpc) {
         case 16:
           for (int col = 0; col < m_Width; col++) {
             *dest_scan++ = src_pos[4];
             *dest_scan++ = src_pos[2];
             *dest_scan++ = *src_pos;
             src_pos += 6;
           }
           break;
@@ skipping 637 matching lines @@
                                    IFX_Pause* pPause) {
   return LoadHandle->Continue(pPause);
 }
 
 CPDF_ImageLoader::~CPDF_ImageLoader() {
   if (!m_bCached) {
     delete m_pBitmap;
     delete m_pMask;
   }
 }