From 7876609b3540137663d48282ad94ba42a3749e73 Mon Sep 17 00:00:00 2001
From: Nicolas Pena <npm@chromium.org>
Date: Tue, 16 May 2017 18:30:24 -0400
Subject: Gif: Detect string decoding errors
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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 | 23 +++++++++++++++++++----
 core/fxcodec/lgif/fx_gif.h   |  2 +-
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/core/fxcodec/lgif/fx_gif.cpp b/core/fxcodec/lgif/fx_gif.cpp
index 0b56f3a509..5e257cc2f5 100644
--- a/core/fxcodec/lgif/fx_gif.cpp
+++ b/core/fxcodec/lgif/fx_gif.cpp
@@ -242,14 +242,18 @@ void CGifLZWDecoder::ClearTable() {
     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 @@ GifDecodeStatus CGifLZWDecoder::Decode(uint8_t* des_buf, uint32_t* des_size) {
         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) {
diff --git a/core/fxcodec/lgif/fx_gif.h b/core/fxcodec/lgif/fx_gif.h
index 5df1f817ef..d7cd5d83fb 100644
--- a/core/fxcodec/lgif/fx_gif.h
+++ b/core/fxcodec/lgif/fx_gif.h
@@ -142,7 +142,7 @@ class CGifLZWDecoder {
  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;
-- 
cgit v1.2.3