Fix the potential integer overflow from "offset + size"

core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp

Index: core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp
diff --git a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp
index f82bf3a861959f31cfed56a905c485b91adb0d28..99afe829f6bc420930b0b73ef28fdbabfcf8f866 100644
--- a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp
+++ b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp
@@ -2864,13 +2864,27 @@ FX_BOOL CPDF_DataAvail::IsObjectsAvail(CFX_PtrArray& obj_array, FX_BOOL bParsePa
                     CPDF_Reference *pRef = (CPDF_Reference*)pObj;
                     FX_DWORD dwNum = pRef->GetRefObjNum();
                     FX_FILESIZE offset;
-                    FX_DWORD size = GetObjectSize(pRef->GetRefObjNum(), offset);
-                    if (!size) {
+                    FX_DWORD original_size = GetObjectSize(dwNum, offset);
+                    base::CheckedNumeric<FX_DWORD> size = original_size;

[palmer, 2014/07/23 17:58:36]

I've already checked in this change
(https://codereview.chromium.org/390983007/).

[jun_fang, 2014/07/24 07:14:39]

Yes. I already merged your change into this fix by resolving the conflicting.

+
+                    if (size.ValueOrDefault(0) == 0 || offset < 0 || offset >= m_dwFileLen)
                         break;
-                    }
-                    size = (FX_DWORD)((FX_FILESIZE)(offset + size + 512) > m_dwFileLen ? m_dwFileLen - offset : size + 512);
-                    if (!m_pFileAvail->IsDataAvail(offset, size)) {
-                        pHints->AddSegment(offset, size);
+                    
+                    size += offset;
+                    size += 512;
+                    if (!size.IsValid())
+                        break;
+
+                    if (size.ValueOrDie() > m_dwFileLen)
+                        size = m_dwFileLen - offset; 
+                    else
+                        size = original_size + 512;
+
+                    if (!size.IsValid())
+                        break;
+
+                    if (!m_pFileAvail->IsDataAvail(offset, size.ValueOrDie())) {
+                        pHints->AddSegment(offset, size.ValueOrDie());
                         ret_array.Add(pObj);
                         count++;
                     } else if (!m_objnum_array.Find(dwNum)) {
@@ -3070,10 +3084,27 @@ CPDF_Object* CPDF_DataAvail::GetObject(FX_DWORD objnum, IFX_DownloadHints* pHint
             *pExistInFile = FALSE;
             return NULL;
         }
-        FX_DWORD size = (FX_DWORD)m_parser.GetObjectSize(objnum);
-        size = (FX_DWORD)(((FX_FILESIZE)(offset + size + 512)) > m_dwFileLen ? m_dwFileLen - offset : size + 512);
-        if (!m_pFileAvail->IsDataAvail(offset, size)) {
-            pHints->AddSegment(offset, size);
+
+        FX_DWORD original_size = (FX_DWORD)m_parser.GetObjectSize(objnum);
+        base::CheckedNumeric<FX_DWORD> size = original_size;
+        if (size.ValueOrDefault(0) == 0 || offset < 0 || offset >= m_dwFileLen)
+            return NULL;
+  
+        size += offset;
+        size += 512;
+        if (!size.IsValid()) 
+            return NULL;
+
+        if (size.ValueOrDie() > m_dwFileLen)
+            size = m_dwFileLen - offset;
+        else
+            size = original_size + 512;
+
+        if (!size.IsValid())
+            return NULL;
+
+        if (!m_pFileAvail->IsDataAvail(offset, size.ValueOrDie())) {
+            pHints->AddSegment(offset, size.ValueOrDie());
             return NULL;
         }
         pRet = m_parser.ParseIndirectObject(NULL, objnum);
@@ -3083,10 +3114,26 @@ CPDF_Object* CPDF_DataAvail::GetObject(FX_DWORD objnum, IFX_DownloadHints* pHint
         return pRet;
     }
     FX_FILESIZE offset = 0;
-    FX_DWORD size = GetObjectSize(objnum, offset);
-    size = (FX_DWORD)((FX_FILESIZE)(offset + size + 512) > m_dwFileLen ? m_dwFileLen - offset : size + 512);
-    if (!m_pFileAvail->IsDataAvail(offset, size)) {
-        pHints->AddSegment(offset, size);
+    FX_DWORD original_size = GetObjectSize(objnum, offset);
+    base::CheckedNumeric<FX_DWORD> size = original_size;

[palmer, 2014/07/23 17:58:36]

So, this same basic chunk is now repeated 3 times. It's time to separate it out
into its own utility function, if that's possible.

[jun_fang, 2014/07/24 07:14:39]

I will reconstruct this function to remove the redundant parts.

+    if (size.ValueOrDefault(0) == 0 || offset < 0 || offset >= m_dwFileLen)
+        return NULL;
+
+    size += offset;
+    size += 512;
+    if (!size.IsValid())
+        return NULL;
+
+    if (size.ValueOrDie() > m_dwFileLen)
+        size = m_dwFileLen - offset;
+    else
+        size = original_size + 512;
+
+    if (!size.IsValid())
+        return NULL;
+ 
+    if (!m_pFileAvail->IsDataAvail(offset, size.ValueOrDie())) {
+        pHints->AddSegment(offset, size.ValueOrDie());
         return NULL;
     }
     CPDF_Parser *pParser = (CPDF_Parser *)(m_pDocument->GetParser());