Make GIF decoder more standards complaint

Fixed issue with unit tests that was causing raw data to be backwards
and reverted related LSB -> MSB change that was introduced due to
this.

If global palette not set then the background colour index should be
0.

Check that background colour index is valid when global palette
exists.

Check if transparency index is valid for the palette of the frame it
is being applied to.

BUG=chromium:770337

Change-Id: I5d9b648f45bbb4c93218664a7035e4d01dbeb627
Reviewed-on: https://pdfium-review.googlesource.com/15453
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: dsinclair <dsinclair@chromium.org>

2 files changed, 54 insertions, 60 deletions

diff --git a/core/fxcodec/gif/cfx_gifcontext.cpp b/core/fxcodec/gif/cfx_gifcontext.cpp
index 28bcad9ceb..ca2e7c3c88 100644
--- a/core/fxcodec/gif/cfx_gifcontext.cpp
+++ b/core/fxcodec/gif/cfx_gifcontext.cpp
@@ -390,9 +390,14 @@ CFX_GifDecodeStatus CFX_GifContext::ReadLogicalScreenDescriptor() {
   }
 
   if (lsd->global_flags.global_pal) {
-    uint32_t global_pal_size = unsigned(2 << lsd->global_flags.pal_bits) * 3u;
-    uint8_t* global_pal = nullptr;
-    if (!ReadData(&global_pal, global_pal_size)) {
+    uint32_t palette_count = unsigned(2 << lsd->global_flags.pal_bits);
+    if (lsd->bc_index >= palette_count)
+      return CFX_GifDecodeStatus::Error;
+    bc_index_ = lsd->bc_index;
+
+    uint32_t palette_size = palette_count * 3u;
+    uint8_t* palette = nullptr;
+    if (!ReadData(&palette, palette_size)) {
       skip_size_ = skip_size_org;
       return CFX_GifDecodeStatus::Unfinished;
     }
@@ -400,15 +405,15 @@ CFX_GifDecodeStatus CFX_GifContext::ReadLogicalScreenDescriptor() {
     global_pal_exp_ = lsd->global_flags.pal_bits;
     global_sort_flag_ = lsd->global_flags.sort_flag;
     global_color_resolution_ = lsd->global_flags.color_resolution;
-    global_palette_.resize(global_pal_size / 3);
-    memcpy(global_palette_.data(), global_pal, global_pal_size);
+    global_palette_.resize(palette_count);
+    memcpy(global_palette_.data(), palette, palette_size);
   }
 
   width_ = static_cast<int>(
-      FXWORD_GET_MSBFIRST(reinterpret_cast<uint8_t*>(&lsd->width)));
+      FXWORD_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&lsd->width)));
   height_ = static_cast<int>(
-      FXWORD_GET_MSBFIRST(reinterpret_cast<uint8_t*>(&lsd->height)));
-  bc_index_ = lsd->bc_index;
+      FXWORD_GET_LSBFIRST(reinterpret_cast<uint8_t*>(&lsd->height)));
+
   pixel_aspect_ = lsd->pixel_aspect;
   return CFX_GifDecodeStatus::Success;
 }
@@ -544,9 +549,19 @@ CFX_GifDecodeStatus CFX_GifContext::DecodeImageInfo() {
   gif_image->data_pos += skip_size_;
   gif_image->image_GCE = nullptr;
   if (graphic_control_extension_.get()) {
+    if (graphic_control_extension_->gce_flags.transparency) {
+      // Need to test that the color that is going to be transparent is actually
+      // in the palette being used.
+      if (graphic_control_extension_->trans_index >=
+          2 << (gif_image->local_palettes.empty()
+                    ? global_pal_exp_
+                    : gif_image->local_pallette_exp))
+        return CFX_GifDecodeStatus::Error;
+    }
     gif_image->image_GCE = std::move(graphic_control_extension_);
     graphic_control_extension_ = nullptr;
   }
+
   images_.push_back(std::move(gif_image));
   SaveDecodingStatus(GIF_D_STATUS_IMG_DATA);
   return CFX_GifDecodeStatus::Success;
diff --git a/core/fxcodec/gif/cfx_gifcontext_unittest.cpp b/core/fxcodec/gif/cfx_gifcontext_unittest.cpp
index 98b8ec5cd0..c5d3ac90c7 100644
--- a/core/fxcodec/gif/cfx_gifcontext_unittest.cpp
+++ b/core/fxcodec/gif/cfx_gifcontext_unittest.cpp
@@ -168,10 +168,9 @@ TEST(CFX_GifContext, ReadLocalScreenDescriptor) {
   }
   // LSD with all the values zero'd
   {
-    CFX_GifLocalScreenDescriptor lsd;
+    uint8_t lsd[sizeof(CFX_GifLocalScreenDescriptor)];
     memset(&lsd, 0, sizeof(CFX_GifLocalScreenDescriptor));
-    context.SetInputBuffer(reinterpret_cast<uint8_t*>(&lsd),
-                           sizeof(CFX_GifLocalScreenDescriptor));
+    context.SetInputBuffer(lsd, sizeof(CFX_GifLocalScreenDescriptor));
 
     EXPECT_EQ(CFX_GifDecodeStatus::Success,
               context.ReadLogicalScreenDescriptor());
@@ -184,9 +183,9 @@ TEST(CFX_GifContext, ReadLocalScreenDescriptor) {
   }
   // LSD with no global palette
   {
-    CFX_GifLocalScreenDescriptor lsd{0x0A00, 0x000F, {0, 0, 0, 0}, 1, 2};
-    context.SetInputBuffer(reinterpret_cast<uint8_t*>(&lsd),
-                           sizeof(CFX_GifLocalScreenDescriptor));
+    uint8_t lsd[sizeof(CFX_GifLocalScreenDescriptor)] = {0x0A, 0x00, 0x00, 0x0F,
+                                                         0x00, 0x01, 0x02};
+    context.SetInputBuffer(lsd, sizeof(CFX_GifLocalScreenDescriptor));
 
     EXPECT_EQ(CFX_GifDecodeStatus::Success,
               context.ReadLogicalScreenDescriptor());
@@ -194,14 +193,14 @@ TEST(CFX_GifContext, ReadLocalScreenDescriptor) {
     EXPECT_EQ(sizeof(CFX_GifLocalScreenDescriptor), context.skip_size_);
     EXPECT_EQ(0x000A, context.width_);
     EXPECT_EQ(0x0F00, context.height_);
-    EXPECT_EQ(1u, context.bc_index_);
+    EXPECT_EQ(0u, context.bc_index_);  // bc_index_ is 0 if no global palette
     EXPECT_EQ(2u, context.pixel_aspect_);
   }
   // LSD with global palette bit set, but no global palette
   {
-    CFX_GifLocalScreenDescriptor lsd{0x0A00, 0x000F, {0, 0, 0, 1}, 1, 2};
-    context.SetInputBuffer(reinterpret_cast<uint8_t*>(&lsd),
-                           sizeof(CFX_GifLocalScreenDescriptor));
+    uint8_t lsd[sizeof(CFX_GifLocalScreenDescriptor)] = {0x0A, 0x00, 0x00, 0x0F,
+                                                         0x80, 0x01, 0x02};
+    context.SetInputBuffer(lsd, sizeof(CFX_GifLocalScreenDescriptor));
 
     EXPECT_EQ(CFX_GifDecodeStatus::Unfinished,
               context.ReadLogicalScreenDescriptor());
@@ -210,14 +209,11 @@ TEST(CFX_GifContext, ReadLocalScreenDescriptor) {
   }
   // LSD with global palette
   {
-    CFX_GifLocalScreenDescriptor lsd = {0x0A00, 0x000F, {1, 1, 2, 1}, 1, 2};
-    CFX_GifPalette palette[4] = {{0, 0, 0}, {1, 1, 1}, {1, 0, 0}, {0, 0, 1}};
     struct {
-      CFX_GifLocalScreenDescriptor lsd;
-      CFX_GifPalette palette[4];
-    } data;
-    memcpy(&data.lsd, &lsd, sizeof(data.lsd));
-    memcpy(&data.palette, &palette, sizeof(data.lsd));
+      uint8_t lsd[sizeof(CFX_GifLocalScreenDescriptor)];
+      uint8_t palette[4 * sizeof(CFX_GifPalette)];
+    } data = {{0x0A, 0x00, 0x00, 0x0F, 0xA9, 0x01, 0x02},
+              {0, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0, 1}};
     context.SetInputBuffer(reinterpret_cast<uint8_t*>(&data), sizeof(data));
 
     EXPECT_EQ(CFX_GifDecodeStatus::Success,
@@ -228,6 +224,7 @@ TEST(CFX_GifContext, ReadLocalScreenDescriptor) {
     EXPECT_EQ(0x0F00, context.height_);
     EXPECT_EQ(1u, context.bc_index_);
     EXPECT_EQ(2u, context.pixel_aspect_);
+
     EXPECT_EQ(1u, context.global_pal_exp_);
     EXPECT_EQ(1, context.global_sort_flag_);
     EXPECT_EQ(2, context.global_color_resolution_);
@@ -240,19 +237,16 @@ TEST(CFX_GifContext, ReadHeader) {
   CFX_GifContextForTest context(nullptr, nullptr);
   // Bad signature
   {
-    uint8_t signature[] = {'N', 'O', 'T', 'G', 'I', 'F'};
-    CFX_GifLocalScreenDescriptor lsd{0x0A00, 0x000F, {0, 0, 0, 0}, 1, 2};
     struct {
       uint8_t signature[6];
-      CFX_GifLocalScreenDescriptor lsd;
-    } data;
-    memcpy(&data.signature, signature, sizeof(data.signature));
-    memcpy(&data.lsd, &lsd, sizeof(data.lsd));
+      uint8_t lsd[sizeof(CFX_GifLocalScreenDescriptor)];
+    } data = {{'N', 'O', 'T', 'G', 'I', 'F'},
+              {0x0A, 0x00, 0x00, 0x0F, 0x00, 0x01, 0x02}};
     context.SetInputBuffer(reinterpret_cast<uint8_t*>(&data), sizeof(data));
 
     EXPECT_EQ(CFX_GifDecodeStatus::Error, context.ReadHeader());
 
-    EXPECT_EQ(sizeof(signature), context.skip_size_);
+    EXPECT_EQ(sizeof(data.signature), context.skip_size_);
   }
   // Short after signature
   {
@@ -266,15 +260,11 @@ TEST(CFX_GifContext, ReadHeader) {
   }
   // Success without global palette
   {
-    uint8_t signature[] = {'G', 'I', 'F', '8', '7', 'a'};
-    CFX_GifLocalScreenDescriptor lsd{0x0A00, 0x000F, {0, 0, 0, 0}, 1, 2};
     struct {
       uint8_t signature[6];
-      CFX_GifLocalScreenDescriptor lsd;
-    } data;
-    memcpy(&data.signature, signature, sizeof(data.signature));
-    memcpy(&data.lsd, &lsd, sizeof(data.lsd));
-
+      uint8_t lsd[sizeof(CFX_GifLocalScreenDescriptor)];
+    } data = {{'G', 'I', 'F', '8', '7', 'a'},
+              {0x0A, 0x00, 0x00, 0x0F, 0x00, 0x01, 0x02}};
     context.SetInputBuffer(reinterpret_cast<uint8_t*>(&data), sizeof(data));
 
     EXPECT_EQ(CFX_GifDecodeStatus::Success, context.ReadHeader());
@@ -282,42 +272,31 @@ TEST(CFX_GifContext, ReadHeader) {
     EXPECT_EQ(sizeof(data), context.skip_size_);
     EXPECT_EQ(0x000A, context.width_);
     EXPECT_EQ(0x0F00, context.height_);
-    EXPECT_EQ(1u, context.bc_index_);
+    EXPECT_EQ(0u, context.bc_index_);  // bc_index_ is 0 if no global palette
     EXPECT_EQ(2u, context.pixel_aspect_);
   }
   // Missing Global Palette
   {
-    uint8_t signature[] = {'G', 'I', 'F', '8', '7', 'a'};
-    CFX_GifLocalScreenDescriptor lsd{0x0A00, 0x000F, {0, 0, 0, 1}, 1, 2};
-
     struct {
       uint8_t signature[6];
-      CFX_GifLocalScreenDescriptor lsd;
-    } data;
-    memcpy(&data.signature, signature, sizeof(data.signature));
-    memcpy(&data.lsd, &lsd, sizeof(data.lsd));
-
+      uint8_t lsd[sizeof(CFX_GifLocalScreenDescriptor)];
+    } data = {{'G', 'I', 'F', '8', '7', 'a'},
+              {0x0A, 0x00, 0x00, 0x0F, 0x80, 0x01, 0x02}};
     context.SetInputBuffer(reinterpret_cast<uint8_t*>(&data), sizeof(data));
 
     EXPECT_EQ(CFX_GifDecodeStatus::Unfinished, context.ReadHeader());
 
-    EXPECT_EQ(sizeof(signature), context.skip_size_);
+    EXPECT_EQ(sizeof(data.signature), context.skip_size_);
   }
   // Success with global palette
   {
-    uint8_t signature[] = {'G', 'I', 'F', '8', '7', 'a'};
-    CFX_GifLocalScreenDescriptor lsd = {0x0A00, 0x000F, {1, 1, 2, 1}, 1, 2};
-    CFX_GifPalette palette[4] = {{0, 0, 0}, {1, 1, 1}, {1, 0, 0}, {0, 0, 1}};
-
     struct {
       uint8_t signature[6];
-      CFX_GifLocalScreenDescriptor lsd;
-      CFX_GifPalette palette[4];
-    } data;
-    memcpy(&data.signature, signature, sizeof(data.signature));
-    memcpy(&data.lsd, &lsd, sizeof(data.lsd));
-    memcpy(&data.palette, &palette, sizeof(data.lsd));
-
+      uint8_t lsd[sizeof(CFX_GifLocalScreenDescriptor)];
+      uint8_t palette[4 * sizeof(CFX_GifPalette)];
+    } data = {{'G', 'I', 'F', '8', '7', 'a'},
+              {0x0A, 0x00, 0x00, 0x0F, 0xA9, 0x01, 0x02},
+              {0, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0, 1}};
     context.SetInputBuffer(reinterpret_cast<uint8_t*>(&data), sizeof(data));
 
     EXPECT_EQ(CFX_GifDecodeStatus::Success, context.ReadHeader());