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 926117722f7d93eb63a1cbe33e9c4a13ce6b98f6..b27ddd21a4a8b4f7a7259e2fbc13410fa2e7b7af 100644
--- a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp
+++ b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp
@@ -2865,10 +2865,21 @@ FX_BOOL CPDF_DataAvail::IsObjectsAvail(CFX_PtrArray& obj_array, FX_BOOL bParsePa
                     FX_DWORD dwNum = pRef->GetRefObjNum();
                     FX_FILESIZE offset;
                     FX_DWORD size = GetObjectSize(pRef->GetRefObjNum(), offset);
-                    if (!size) {
+
+                    if (size == 0 || offset < 0 || offset >= m_dwFileLen)
                         break;
-                    }
-                    size = (FX_DWORD)((FX_FILESIZE)(offset + size + 512) > m_dwFileLen ? m_dwFileLen - offset : size + 512);
+                    
+                    base::CheckedNumeric<FX_DWORD> safe_size = size;

[palmer, 2014/07/09 18:29:14]

You should check the value that you actually use, i.e. |size|. You don't need
|safe_size|.

base::CheckedNumeric<FX_DWORD> size = GetObjectSize(pRef->GetRefObjNum(),
offset);
if (size.ValueOrDefault(0) == 0 || offset < 0 || offset >= m_dwFileLen) {
    break;
}

safe_size += offset;
safe_size += 512;
if (!safe_size.IsValid()) {
    break;
}

if (safe_size.ValueOrDie() > m_dwFileLen) {
    size = m_dwFileLen - offset; 
} else {
    size = size + 512;
}
if (!safe_size.IsValid()) {
    break;
}

Then use size.ValidOrDie() on lines 2883 and 2884.

[jun_fang, 2014/07/10 22:21:54]

safe_size is a temporary variable which is used to judge whether the result of
|size+offset+512| overflows. Even we use the method you suggested:
base::CheckedNumeric<FX_DWORD> size = GetObjectSize(pRef->GetRefObjNum(). We
still need another temporary variable to check the sum of integer. 

+                    safe_size += offset;
+                    safe_size += 512;
+                    if (!safe_size.IsValid())
+                        break;
+
+                    if (safe_size.ValueOrDie() > m_dwFileLen)
+                        size = m_dwFileLen - offset; 

[jun_fang, 2014/07/10 22:21:54]

size = m_dwFileLen - offset; |size| should be valid because we check |offset <
m_dwFileLen| at the line of 2869.

+                    else
+                        size = size + 512;

[jun_fang, 2014/07/10 22:21:54]

because safe_size is valid, we can make sure size is valid here.
safe_size = size + offset + 512;

+
                     if (!m_pFileAvail->IsDataAvail(offset, size)) {
                         pHints->AddSegment(offset, size);
                         ret_array.Add(pObj);
@@ -3070,8 +3081,22 @@ 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 (size == 0 || offset < 0 || offset >= m_dwFileLen)
+            return NULL;
+  
+        base::CheckedNumeric<FX_DWORD> safe_size = size;

[palmer, 2014/07/09 18:29:14]

Same as above.

+        safe_size += offset;
+        safe_size += 512;
+        if (!safe_size.IsValid()) 
+            return NULL;
+
+        if (safe_size.ValueOrDie() > m_dwFileLen)
+            size = m_dwFileLen - offset;
+        else
+            size = size + 512;
+
         if (!m_pFileAvail->IsDataAvail(offset, size)) {
             pHints->AddSegment(offset, size);
             return NULL;
@@ -3084,7 +3109,20 @@ CPDF_Object* CPDF_DataAvail::GetObject(FX_DWORD objnum, IFX_DownloadHints* pHint
     }
     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 (size == 0 || offset < 0 || offset >= m_dwFileLen)
+        return NULL;
+
+    base::CheckedNumeric<FX_DWORD> safe_size = size;

[palmer, 2014/07/09 18:29:14]

Same as above.

+    safe_size += offset;
+    safe_size += 512;
+    if (!safe_size.IsValid()) 
+        return NULL;
+
+    if (safe_size.ValueOrDie() > m_dwFileLen)
+        size = m_dwFileLen - offset;
+    else
+        size = size + 512;
+ 
     if (!m_pFileAvail->IsDataAvail(offset, size)) {
         pHints->AddSegment(offset, size);
         return NULL;