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

fpdfsdk/src/fpdfview.cpp

Index: fpdfsdk/src/fpdfview.cpp
diff --git a/fpdfsdk/src/fpdfview.cpp b/fpdfsdk/src/fpdfview.cpp
index b950ed8641556345cb8642c3f574933af66a55c7..ab05e33bbb1c082db010b4e98277179464ca5097 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_math.h"
 
 CPDF_CustomAccess::CPDF_CustomAccess(FPDF_FILEACCESS* pFileAccess)
 {
@@ -35,18 +35,21 @@ 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;
+        base::CheckedNumeric<FX_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;
+        base::CheckedNumeric<size_t> newPos = 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
@@ -292,8 +295,13 @@ 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;
+
+                base::CheckedNumeric<size_t> newPos = size;
+                newPos += offset;
+                if (!newPos.IsValid() || newPos.ValueOrDie() >= (FX_DWORD)m_size) return FALSE;

[palmer, 2014/07/25 00:40:14]

Hmm, I wonder if this cast to FX_DWORD makes sense. On some platforms, it is
potentially truncating? I.e. on some platforms FX_FILESIZE is an off_t, which
might be 64 bits even on platforms where size_t and FX_DWORD are 32 bits.

I think it might be better to declare

    base::CheckedNumeric<FX_FILESIZE> newPos = size;

and then not cast |m_size| to FX_DWORD in the comparison expression.

[jun_fang, 2014/07/25 01:04:00]

However, FX_FILESIZE is defined as int on some platforms. It causes potential
truncation if  size_t or FX_DWORD casts to FX_FILESIZE.

 		FXSYS_memcpy(buffer, m_pBuf+offset, size);
+
 		return TRUE;
 	}
 private: