commit 7876609b3540137663d48282ad94ba42a3749e73
author Nicolas Pena <npm@chromium.org> Tue May 16 18:30:24 2017 -0400
committer Chromium commit bot <commit-bot@chromium.org> Wed May 17 00:56:02 2017 +0000
tree 81c594651cb6a048ee61699a876dd12d5e471082
parent b332581e185760597e8f0160011b1e6094634ed8

Gif: Detect string decoding errors

This CL adds some checks to make sure the DecodeString method does not go out
out control:
If code is equal to code_table[code].prefix, it will try to loop forever.
Even if that's not the case, avoid reading a negative position from the stack.

Bug: chromium:722672
Change-Id: I638f91542ba21f3a9915198fef853cc3cf94f4f1
Reviewed-on: https://pdfium-review.googlesource.com/5513
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Nicolás Peña <npm@chromium.org>

core/fxcodec/lgif/fx_gif.cpp

diff --git a/core/fxcodec/lgif/fx_gif.cpp b/core/fxcodec/lgif/fx_gif.cpp
index 0b56f3a..5e257cc 100644
--- a/core/fxcodec/lgif/fx_gif.cpp
+++ b/core/fxcodec/lgif/fx_gif.cpp
@@ -242,14 +242,18 @@
     code_table[i].suffix = static_cast<uint8_t>(i);
 }
 
-void CGifLZWDecoder::DecodeString(uint16_t code) {
+bool CGifLZWDecoder::DecodeString(uint16_t code) {
   stack_size = 0;
   while (code >= code_clear && code <= code_next) {
+    if (code == code_table[code].prefix || stack_size == GIF_MAX_LZW_CODE - 1)
+      return false;
+
     stack[GIF_MAX_LZW_CODE - 1 - stack_size++] = code_table[code].suffix;
     code = code_table[code].prefix;
   }
   stack[GIF_MAX_LZW_CODE - 1 - stack_size++] = static_cast<uint8_t>(code);
   code_first = static_cast<uint8_t>(code);
+  return true;
 }
 
 void CGifLZWDecoder::AddCode(uint16_t prefix_code, uint8_t append_char) {
@@ -319,19 +323,30 @@
         if (code_next < GIF_MAX_LZW_CODE) {
           if (code == code_next) {
             AddCode(code_old, code_first);
-            DecodeString(code);
+            if (!DecodeString(code)) {
+              strncpy(err_msg_ptr, "String Decoding Error",
+                      GIF_MAX_ERROR_SIZE - 1);
+              return GifDecodeStatus::Error;
+            }
           } else if (code > code_next) {
             strncpy(err_msg_ptr, "Decode Error, Out Of Range",
                     GIF_MAX_ERROR_SIZE - 1);
             return GifDecodeStatus::Error;
           } else {
-            DecodeString(code);
+            if (!DecodeString(code)) {
+              strncpy(err_msg_ptr, "String Decoding Error",
+                      GIF_MAX_ERROR_SIZE - 1);
+              return GifDecodeStatus::Error;
+            }
             uint8_t append_char = stack[GIF_MAX_LZW_CODE - stack_size];
             AddCode(code_old, append_char);
           }
         }
       } else {
-        DecodeString(code);
+        if (!DecodeString(code)) {
+          strncpy(err_msg_ptr, "String Decoding Error", GIF_MAX_ERROR_SIZE - 1);
+          return GifDecodeStatus::Error;
+        }
       }
       code_old = code;
       if (i + stack_size > *des_size) {

core/fxcodec/lgif/fx_gif.h

diff --git a/core/fxcodec/lgif/fx_gif.h b/core/fxcodec/lgif/fx_gif.h
index 5df1f81..d7cd5d8 100644
--- a/core/fxcodec/lgif/fx_gif.h
+++ b/core/fxcodec/lgif/fx_gif.h
@@ -142,7 +142,7 @@
  private:
   void ClearTable();
   void AddCode(uint16_t prefix_code, uint8_t append_char);
-  void DecodeString(uint16_t code);
+  bool DecodeString(uint16_t code);
 
   uint8_t code_size;
   uint8_t code_size_cur;