Fix the potential integer overflow from 'offset+size' in extension.h and fpdfview.cpp

BUG=397258
R=tsepez@chromium.org

Review URL: https://codereview.chromium.org/419063002

4 files changed, 84 insertions, 16 deletions

diff --git a/core/include/fxcrt/fx_stream.h b/core/include/fxcrt/fx_stream.h
index 869797f7e9..033913f780 100644
--- a/core/include/fxcrt/fx_stream.h
+++ b/core/include/fxcrt/fx_stream.h
@@ -30,6 +30,7 @@ FX_DEFINEHANDLE(FX_HFILE)
 #endif
 #define FX_FILESIZE			off_t
 #endif
+typedef base::CheckedNumeric<FX_FILESIZE>    FX_SAFE_FILESIZE;
 #define FX_GETBYTEOFFSET32(a)	0
 #define FX_GETBYTEOFFSET40(a)	0
 #define FX_GETBYTEOFFSET48(a)	0
diff --git a/core/include/fxcrt/fx_system.h b/core/include/fxcrt/fx_system.h
index 9f43360fa7..391380304c 100644
--- a/core/include/fxcrt/fx_system.h
+++ b/core/include/fxcrt/fx_system.h
@@ -275,5 +275,18 @@ int			FXSYS_round(FX_FLOAT f);
 #define		FXSYS_sqrt2(a, b) (FX_FLOAT)FXSYS_sqrt((a)*(a) + (b)*(b))
 #ifdef __cplusplus
 };
+
+#include "../../../third_party/numerics/safe_math.h"
+typedef base::CheckedNumeric<FX_DWORD> 		FX_SAFE_DWORD;
+typedef base::CheckedNumeric<FX_INT32> 		FX_SAFE_INT;
+typedef base::CheckedNumeric<size_t>            FX_SAFE_SIZET;
+#if defined(__clang__) || _MSC_VER >= 1700
+#define FX_FINAL final
+#elif defined(__GNUC__) && __cplusplus >= 201103 && \
+      (__GNUC__ * 10000 + __GNUC_MINOR__ * 100) >= 40700
+#define FX_FINAL final
+#else
+#define FX_FINAL
+#endif
 #endif
 #endif
diff --git a/core/src/fxcrt/extension.h b/core/src/fxcrt/extension.h
index 671b4736ef..d5659e5d29 100644
--- a/core/src/fxcrt/extension.h
+++ b/core/src/fxcrt/extension.h
@@ -6,6 +6,7 @@
 
 #ifndef _FXCRT_EXTENSION_IMP_
 #define _FXCRT_EXTENSION_IMP_
+
 class IFXCRT_FileAccess
 {
 public:
@@ -69,9 +70,17 @@ public:
     }
     virtual FX_BOOL				SetRange(FX_FILESIZE offset, FX_FILESIZE size)
     {
-        if (offset < 0 || offset + size > m_pFile->GetSize()) {
+        if (offset < 0 || size < 0) {
             return FALSE;
         }
+     
+        FX_SAFE_FILESIZE pos = size;
+        pos += offset;
+
+        if (!pos.IsValid() || pos.ValueOrDie() >= m_pFile->GetSize()) {
+            return FALSE;
+        }
+
         m_nOffset = offset, m_nSize = size;
         m_bUseRange = TRUE;
         m_pFile->SetPosition(m_nOffset);
@@ -83,13 +92,18 @@ public:
     }
     virtual FX_BOOL				ReadBlock(void* buffer, FX_FILESIZE offset, size_t size)
     {
+        if (m_bUseRange && offset < 0) {
+            return FALSE;
+        }
+        FX_SAFE_FILESIZE pos = offset;
+
         if (m_bUseRange) {
-            if (offset + size > (size_t)GetSize()) {
+            pos += m_nOffset;
+            if (!pos.IsValid() || pos.ValueOrDie() >= (size_t)GetSize()) {
                 return FALSE;
             }
-            offset += m_nOffset;
         }
-        return (FX_BOOL)m_pFile->ReadPos(buffer, size, offset);
+        return (FX_BOOL)m_pFile->ReadPos(buffer, size, pos.ValueOrDie());
     }
     virtual size_t				ReadBlock(void* buffer, size_t size)
     {
@@ -194,7 +208,12 @@ public:
     }
     virtual FX_BOOL				SetRange(FX_FILESIZE offset, FX_FILESIZE size)
     {
-        if (offset < 0 || (size_t)(offset + size) > m_nCurSize) {
+        if (offset < 0 || size < 0) {
+            return FALSE;
+        }
+        FX_SAFE_FILESIZE range = size;
+        range += offset;
+        if (!range.IsValid() || range.ValueOrDie() >= m_nCurSize) {
             return FALSE;
         }
         m_nOffset = (size_t)offset, m_nSize = (size_t)size;
@@ -211,10 +230,17 @@ public:
         if (!buffer || !size) {
             return FALSE;
         }
+
+        FX_SAFE_FILESIZE safeOffset = offset;
         if (m_bUseRange) {
             offset += (FX_FILESIZE)m_nOffset;
         }
-        if ((size_t)offset + size > m_nCurSize) {
+
+        offset = safeOffset.ValueOrDie();
+
+        FX_SAFE_SIZET newPos = size;
+        newPos += offset;
+        if (!newPos.IsValid() || newPos.ValueOrDefault(0) == 0 || newPos.ValueOrDie() >= m_nCurSize) {
             return FALSE;
         }
         m_nCurPos = (size_t)offset + size;
@@ -263,7 +289,12 @@ public:
             offset += (FX_FILESIZE)m_nOffset;
         }
         if (m_dwFlags & FX_MEMSTREAM_Consecutive) {
-            m_nCurPos = (size_t)offset + size;
+            FX_SAFE_SIZET newPos = size; 
+            newPos += offset;
+            if (!newPos.IsValid())
+                return FALSE;
+
+            m_nCurPos = newPos.ValueOrDie();
             if (m_nCurPos > m_nTotalSize) {
                 IFX_Allocator* pAllocator = m_Blocks.m_pAllocator;
                 m_nTotalSize = (m_nCurPos + m_nGrowSize - 1) / m_nGrowSize * m_nGrowSize;
@@ -284,7 +315,14 @@ public:
             }
             return TRUE;
         }
-        if (!ExpandBlocks((size_t)offset + size)) {
+
+        FX_SAFE_SIZET newPos = size;
+        newPos += offset;
+        if (!newPos.IsValid()) {
+            return FALSE;
+        }
+
+        if (!ExpandBlocks(newPos.ValueOrDie())) {
             return FALSE;
         }
         m_nCurPos = (size_t)offset + size;
diff --git a/fpdfsdk/src/fpdfview.cpp b/fpdfsdk/src/fpdfview.cpp
index 1ca66f389d..232c7ddec2 100644
--- a/fpdfsdk/src/fpdfview.cpp
+++ b/fpdfsdk/src/fpdfview.cpp
@@ -9,7 +9,7 @@
 #include "../include/fsdk_rendercontext.h"
 #include "../include/fpdf_progressive.h"
 #include "../include/fpdf_ext.h"
-
+#include "../../third_party/numerics/safe_conversions_impl.h"
 
 CPDF_CustomAccess::CPDF_CustomAccess(FPDF_FILEACCESS* pFileAccess)
 {
@@ -35,18 +35,27 @@ FX_BOOL CPDF_CustomAccess::GetByte(FX_DWORD pos, FX_BYTE& ch)
 
 FX_BOOL CPDF_CustomAccess::GetBlock(FX_DWORD pos, FX_LPBYTE pBuf, FX_DWORD size)
 {
-	if (pos + size > m_FileAccess.m_FileLen) return FALSE;
+        FX_SAFE_DWORD newPos = size;
+        newPos += pos;
+	if (!newPos.IsValid() || newPos.ValueOrDie() >= m_FileAccess.m_FileLen) {
+            return FALSE;
+        }
+
 	return m_FileAccess.m_GetBlock(m_FileAccess.m_Param, pos, pBuf, size);
 }
 
 FX_BOOL CPDF_CustomAccess::ReadBlock(void* buffer, FX_FILESIZE offset, size_t size)
 {
-	//	m_FileAccess = *pFileAccess;
-	//	m_BufferOffset = (FX_DWORD)-1;
-	if (offset + size > m_FileAccess.m_FileLen) return FALSE;
-	return m_FileAccess.m_GetBlock(m_FileAccess.m_Param, offset,(FX_LPBYTE) buffer, size);
+        if (offset < 0) {
+            return FALSE;
+        }
+        FX_SAFE_FILESIZE newPos = base::checked_cast<FX_FILESIZE, size_t>(size);
+        newPos += offset;
+	if (!newPos.IsValid() || newPos.ValueOrDie() >= m_FileAccess.m_FileLen) {
+            return FALSE;
+        }
 
-	//	return FALSE;
+	return m_FileAccess.m_GetBlock(m_FileAccess.m_Param, offset,(FX_LPBYTE) buffer, size);
 }
 
 //0 bit: FPDF_POLICY_MACHINETIME_ACCESS
@@ -298,8 +307,15 @@ public:
 	virtual FX_FILESIZE		GetSize() {return m_size;}
 	virtual FX_BOOL			ReadBlock(void* buffer, FX_FILESIZE offset, size_t size) 
 	{
-		if(offset+size > (FX_DWORD)m_size) return FALSE;
+                if (offset < 0) {
+                    return FALSE;
+                }
+
+                FX_SAFE_FILESIZE newPos = base::checked_cast<FX_FILESIZE, size_t>(size);
+                newPos += offset;
+                if (!newPos.IsValid() || newPos.ValueOrDie() >= (FX_DWORD)m_size) return FALSE;
 		FXSYS_memcpy(buffer, m_pBuf+offset, size);
+
 		return TRUE;
 	}
 private: