commit 6500c6faf82f636d55c9ca5682711022890bef1d
author Nicolas Pena <npm@chromium.org> Tue Jun 13 12:14:11 2017 -0400
committer Chromium commit bot <commit-bot@chromium.org> Tue Jun 13 16:50:49 2017 +0000
tree ac688a6fd0571c4afe0aba43004c48c516ea830f
parent 3516256c28c29d13e9092e7bb3ea3b417d3bb6df

Check validity of color indices in bmp_decode_rgb

The pal_num member of bmp_ptr indicates the number of color indices
used by the bitmap. This CL returns an error when an invalid index is
found, since otherwise a heap-buffer-overflow can occur since the size
of m_pSrcPalette is calculated based on pal_num.

Bug: chromium:616670
Change-Id: I397958704bed1aa1ae259016ffd5033c07a801ee
Reviewed-on: https://pdfium-review.googlesource.com/6470
Reviewed-by: dsinclair <dsinclair@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: dsinclair <dsinclair@chromium.org>

core/fxcodec/lbmp/fx_bmp.cpp

diff --git a/core/fxcodec/lbmp/fx_bmp.cpp b/core/fxcodec/lbmp/fx_bmp.cpp
index e4fa17d..5611329 100644
--- a/core/fxcodec/lbmp/fx_bmp.cpp
+++ b/core/fxcodec/lbmp/fx_bmp.cpp
@@ -335,10 +335,19 @@
   bmp_error(bmp_ptr, "Any Uncontrol Error");
   return 0;
 }
+
+bool validateColorIndex(uint8_t val, bmp_decompress_struct_p bmp_ptr) {
+  if (val >= bmp_ptr->pal_num) {
+    bmp_error(bmp_ptr, "A color index exceeds range determined by pal_num");
+    return false;
+  }
+  return true;
+}
+
 int32_t bmp_decode_rgb(bmp_decompress_struct_p bmp_ptr) {
-  uint8_t* row_buf = bmp_ptr->out_row_buffer;
   uint8_t* des_buf = nullptr;
   while (bmp_ptr->row_num < bmp_ptr->height) {
+    uint8_t* row_buf = bmp_ptr->out_row_buffer;
     if (!bmp_read_data(bmp_ptr, &des_buf, bmp_ptr->src_row_bytes))
       return 2;
 
@@ -389,9 +398,13 @@
       case 24:
       case 32:
         memcpy(bmp_ptr->out_row_buffer, des_buf, bmp_ptr->src_row_bytes);
+        row_buf += bmp_ptr->src_row_bytes;
         break;
     }
-    row_buf = bmp_ptr->out_row_buffer;
+    for (uint8_t* buf = bmp_ptr->out_row_buffer; buf < row_buf; ++buf) {
+      if (!validateColorIndex(*buf, bmp_ptr))
+        return 0;
+    }
     bmp_ptr->bmp_get_row_fn(bmp_ptr,
                             bmp_ptr->imgTB_flag
                                 ? bmp_ptr->row_num++
@@ -479,8 +492,12 @@
               bmp_ptr->skip_size = skip_size_org;
               return 2;
             }
-            memcpy(bmp_ptr->out_row_buffer + bmp_ptr->col_num, second_byte_ptr,
-                   *first_byte_ptr);
+            uint8_t* first_buf = bmp_ptr->out_row_buffer + bmp_ptr->col_num;
+            memcpy(first_buf, second_byte_ptr, *first_byte_ptr);
+            for (size_t i = 0; i < *first_byte_ptr; ++i) {
+              if (!validateColorIndex(first_buf[i], bmp_ptr))
+                return 0;
+            }
             bmp_ptr->col_num += (int32_t)(*first_byte_ptr);
           }
         }
@@ -495,8 +512,12 @@
           bmp_ptr->skip_size = skip_size_org;
           return 2;
         }
-        memset(bmp_ptr->out_row_buffer + bmp_ptr->col_num, *second_byte_ptr,
-               *first_byte_ptr);
+        uint8_t* first_buf = bmp_ptr->out_row_buffer + bmp_ptr->col_num;
+        memset(first_buf, *second_byte_ptr, *first_byte_ptr);
+        for (size_t i = 0; i < *first_byte_ptr; ++i) {
+          if (!validateColorIndex(first_buf[i], bmp_ptr))
+            return 0;
+        }
         bmp_ptr->col_num += (int32_t)(*first_byte_ptr);
       }
     }
@@ -590,13 +611,12 @@
               return 2;
             }
             for (uint8_t i = 0; i < *first_byte_ptr; i++) {
-              if (i & 0x01) {
-                *(bmp_ptr->out_row_buffer + bmp_ptr->col_num++) =
-                    (*second_byte_ptr++ & 0x0F);
-              } else {
-                *(bmp_ptr->out_row_buffer + bmp_ptr->col_num++) =
-                    ((*second_byte_ptr & 0xF0) >> 4);
-              }
+              uint8_t color = (i & 0x01) ? (*second_byte_ptr++ & 0x0F)
+                                         : (*second_byte_ptr & 0xF0) >> 4;
+              if (!validateColorIndex(color, bmp_ptr))
+                return 0;
+
+              *(bmp_ptr->out_row_buffer + bmp_ptr->col_num++) = color;
             }
           }
         }
@@ -623,6 +643,9 @@
           uint8_t second_byte = *second_byte_ptr;
           second_byte =
               i & 0x01 ? (second_byte & 0x0F) : (second_byte & 0xF0) >> 4;
+          if (!validateColorIndex(second_byte, bmp_ptr))
+            return 0;
+
           *(bmp_ptr->out_row_buffer + bmp_ptr->col_num++) = second_byte;
         }
       }