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

core/src/fxcrt/extension.h

 // Copyright 2014 PDFium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
 #ifndef _FXCRT_EXTENSION_IMP_
 #define _FXCRT_EXTENSION_IMP_
 
-#include "../../../third_party/numerics/safe_math.h"
-
 class IFXCRT_FileAccess
 {
 public:
     virtual ~IFXCRT_FileAccess() {}
     virtual FX_BOOL             Open(FX_BSTR fileName, FX_DWORD dwMode) = 0;
     virtual FX_BOOL             Open(FX_WSTR fileName, FX_DWORD dwMode) = 0;
     virtual void                Close() = 0;
     virtual void                Release() = 0;
     virtual FX_FILESIZE GetSize() const = 0;
     virtual FX_FILESIZE GetPosition() const = 0;
@@ skipping 39 matching lines @@
     virtual FX_FILESIZE                 GetPosition()
     {
         FX_FILESIZE pos = m_pFile->GetPosition();
         if (m_bUseRange) {
             pos -= m_nOffset;
         }
         return pos;
     }
     virtual FX_BOOL                             SetRange(FX_FILESIZE offset, FX_FILESIZE size)
     {
-        if (offset < 0 || offset + size > m_pFile->GetSize()) {
+        FX_SAFE_FILESIZE pos = size;

[Tom Sepez, 2014/07/30 18:50:24]

nit:  I would write this as
if (offset < 0 || size < 0) {
    return FALSE;
}
FX_SAFE_FILESIZE pos = size;
pos += offset;
if (!pos.IsVald() || pos.ValueOrDie() >= m_pFile->GetSize) {
    return FALSE;
}
just to avoid initializing the FX_SAFE_FILESIZE if we don't have to. I think
that reads a litte clearer.

+        pos += offset;
+
+        if (!pos.IsValid() || offset < 0 || size < 0 || pos.ValueOrDie() >= m_pFile->GetSize()) {
             return FALSE;
         }
+
         m_nOffset = offset, m_nSize = size;
         m_bUseRange = TRUE;
         m_pFile->SetPosition(m_nOffset);
         return TRUE;
     }
     virtual void                                ClearRange()
     {
         m_bUseRange = FALSE;
     }
     virtual FX_BOOL                             ReadBlock(void* buffer, FX_FILESIZE offset, size_t size)
     {
+        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)
     {
         if (m_bUseRange) {
             FX_FILESIZE availSize = m_nOffset + m_nSize - m_pFile->GetPosition();
             if ((size_t)availSize < size) {
                 size -= size - (size_t)availSize;
             }
         }
         return m_pFile->Read(buffer, size);
@@ skipping 75 matching lines @@
     virtual FX_FILESIZE                 GetPosition()
     {
         FX_FILESIZE pos = (FX_FILESIZE)m_nCurPos;
         if (m_bUseRange) {
             pos -= (FX_FILESIZE)m_nOffset;
         }
         return pos;
     }
     virtual FX_BOOL                             SetRange(FX_FILESIZE offset, FX_FILESIZE size)
     {
-        base::CheckedNumeric<FX_FILESIZE> range = size;
-        range += size;
- 
-        if (!range.IsValid() || offset <= 0 || size <= 0 || range.ValueOrDie() > m_nCurSize) {
+        FX_SAFE_FILESIZE range = size;
+        range += offset;
+        if (!range.IsValid() || offset <= 0 || size <= 0 || range.ValueOrDie() >= m_nCurSize) {

[Tom Sepez, 2014/07/30 18:50:24]

nit: again, I'd make the <= 0 checks first before initializing the
FX_SAFE_FILESIZE

             return FALSE;
         }
         
         m_nOffset = (size_t)offset, m_nSize = (size_t)size;
         m_bUseRange = TRUE;
         m_nCurPos = m_nOffset;
         return TRUE;
     }
     virtual void                                ClearRange()
     {
         m_bUseRange = FALSE;
     }
     virtual FX_BOOL                             ReadBlock(void* buffer, FX_FILESIZE offset, size_t size)
     {
         if (!buffer || !size) {
             return FALSE;
         }
 
-        base::CheckedNumeric<FX_FILESIZE> safeOffset = offset;
+        FX_SAFE_FILESIZE safeOffset = offset;
         if (m_bUseRange) {
             safeOffset += m_nOffset;
         }
          
         if (!safeOffset.IsValid()) {
             return FALSE;
         }
 
         offset = safeOffset.ValueOrDie();
 
-        base::CheckedNumeric<size_t> newPos = size;
+        FX_SAFE_SIZET newPos = size;
         newPos += offset;
-        if (!newPos.IsValid() || newPos.ValueOrDefault(0) == 0 || newPos.ValueOrDie() > m_nCurSize) {
+        if (!newPos.IsValid() || newPos.ValueOrDefault(0) == 0 || newPos.ValueOrDie() >= m_nCurSize) {
             return FALSE;
         }
 
         m_nCurPos = newPos.ValueOrDie();
         if (m_dwFlags & FX_MEMSTREAM_Consecutive) {
             FXSYS_memcpy32(buffer, (FX_LPBYTE)m_Blocks[0] + (size_t)offset, size);
             return TRUE;
         }
         size_t nStartBlock = (size_t)offset / m_nGrowSize;
         offset -= (FX_FILESIZE)(nStartBlock * m_nGrowSize);
@@ skipping 29 matching lines @@
     }
     virtual     FX_BOOL                         WriteBlock(const void* buffer, FX_FILESIZE offset, size_t size)
     {
         if (!buffer || !size) {
             return FALSE;
         }
         if (m_bUseRange) {
             offset += (FX_FILESIZE)m_nOffset;
         }
         if (m_dwFlags & FX_MEMSTREAM_Consecutive) {
-            base::CheckedNumeric<size_t> newPos = size; 
+            FX_SAFE_SIZET newPos = size; 
             newPos += offset;
             if (!newPos.IsValid())
                 return FALSE;
 
             m_nCurPos = newPos.ValueOrDie();
             if (m_nCurPos > m_nTotalSize) {
                 m_nTotalSize = (m_nCurPos + m_nGrowSize - 1) / m_nGrowSize * m_nGrowSize;
                 if (m_Blocks.GetSize() < 1) {
                     void* block = FX_Alloc(FX_BYTE, m_nTotalSize);
                     m_Blocks.Add(block);
                 } else {
                     m_Blocks[0] = FX_Realloc(FX_BYTE, m_Blocks[0], m_nTotalSize);
                 }
                 if (!m_Blocks[0]) {
                     m_Blocks.RemoveAll();
                     return FALSE;
                 }
             }
             FXSYS_memcpy32((FX_LPBYTE)m_Blocks[0] + (size_t)offset, buffer, size);
             if (m_nCurSize < m_nCurPos) {
                 m_nCurSize = m_nCurPos;
             }
             return TRUE;
         }
 
-        base::CheckedNumeric<size_t> newPos = size;
+        FX_SAFE_SIZET newPos = size;
         newPos += offset;
         if (!newPos.IsValid())

[Tom Sepez, 2014/07/30 18:50:24]

nit: braces here around single-line if to match this file's convention.

             return FALSE;
 
         if (!ExpandBlocks(newPos.ValueOrDie())) {
             return FALSE;
         }
         m_nCurPos = newPos.ValueOrDie();
         size_t nStartBlock = (size_t)offset / m_nGrowSize;
         offset -= (FX_FILESIZE)(nStartBlock * m_nGrowSize);
         while (size) {
             size_t nWrite = m_nGrowSize - (size_t)offset;
@@ skipping 108 matching lines @@
     FX_DWORD mt[MT_N];
 } FX_MTRANDOMCONTEXT, * FX_LPMTRANDOMCONTEXT;
 typedef FX_MTRANDOMCONTEXT const * FX_LPCMTRANDOMCONTEXT;
 #if _FXM_PLATFORM_ == _FXM_PLATFORM_WINDOWS_
 FX_BOOL FX_GenerateCryptoRandom(FX_LPDWORD pBuffer, FX_INT32 iCount);
 #endif
 #ifdef __cplusplus
 }
 #endif
 #endif