From 3414a6447fcd7a8338beb8d641c8f50c2e60a2b0 Mon Sep 17 00:00:00 2001 From: JUN FANG Date: Thu, 24 Jul 2014 12:19:57 -0700 Subject: This change is for fixing the potential integer overflow from "offset + size" BUG=382667 R=palmer@chromium.org Review URL: https://codereview.chromium.org/322333002 --- .../src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp | 74 ++++++++++++++-------- core/src/fxcrt/extension.h | 44 +++++++++++-- 2 files changed, 85 insertions(+), 33 deletions(-) diff --git a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp index 14597d989c..caf40544dc 100644 --- a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp +++ b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp @@ -2869,7 +2869,7 @@ FX_BOOL CPDF_DataAvail::IsObjectsAvail(CFX_PtrArray& obj_array, FX_BOOL bParsePa if (size.ValueOrDefault(0) == 0 || offset < 0 || offset >= m_dwFileLen) { break; } - + size += offset; size += 512; if (!size.IsValid()) { @@ -3074,42 +3074,64 @@ FX_BOOL CPDF_DataAvail::LoadAllXref(IFX_DownloadHints* pHints) } CPDF_Object* CPDF_DataAvail::GetObject(FX_DWORD objnum, IFX_DownloadHints* pHints, FX_BOOL *pExistInFile) { - CPDF_Object *pRet = NULL; - if (pExistInFile) { + CPDF_Object *pRet = NULL; + FX_DWORD original_size = 0; + FX_FILESIZE offset = 0; + CPDF_Parser *pParser = NULL; + + if (pExistInFile) { *pExistInFile = TRUE; } + if (m_pDocument == NULL) { - FX_FILESIZE offset = m_parser.GetObjectOffset(objnum); - if (offset < 0) { - *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); - return NULL; - } - pRet = m_parser.ParseIndirectObject(NULL, objnum); - if (!pRet && pExistInFile) { - *pExistInFile = FALSE; - } - return pRet; + original_size = (FX_DWORD)m_parser.GetObjectSize(objnum); + offset = m_parser.GetObjectOffset(objnum); + pParser = &m_parser; + } else { + original_size = GetObjectSize(objnum, offset); + pParser = (CPDF_Parser *)(m_pDocument->GetParser()); } - 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); + + base::CheckedNumeric size = original_size; + if (size.ValueOrDefault(0) == 0 || offset < 0 || offset >= m_dwFileLen) { + if (pExistInFile) + *pExistInFile = FALSE; + return NULL; } - CPDF_Parser *pParser = (CPDF_Parser *)(m_pDocument->GetParser()); - pRet = pParser->ParseIndirectObject(NULL, objnum, 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; + } + + if (pParser) { + pRet = pParser->ParseIndirectObject(NULL, objnum, NULL); + } + if (!pRet && pExistInFile) { *pExistInFile = FALSE; } + return pRet; } + FX_BOOL CPDF_DataAvail::CheckInfo(IFX_DownloadHints* pHints) { FX_BOOL bExist = FALSE; diff --git a/core/src/fxcrt/extension.h b/core/src/fxcrt/extension.h index a2d0a1462f..b8dce7c97d 100644 --- a/core/src/fxcrt/extension.h +++ b/core/src/fxcrt/extension.h @@ -6,6 +6,9 @@ #ifndef _FXCRT_EXTENSION_IMP_ #define _FXCRT_EXTENSION_IMP_ + +#include "../../../third_party/numerics/safe_math.h" + class IFXCRT_FileAccess { public: @@ -181,9 +184,13 @@ public: } virtual FX_BOOL SetRange(FX_FILESIZE offset, FX_FILESIZE size) { - if (offset < 0 || (size_t)(offset + size) > m_nCurSize) { + base::CheckedNumeric range = size; + range += size; + + if (!range.IsValid() || offset <= 0 || size <= 0 || range.ValueOrDie() > m_nCurSize) { return FALSE; } + m_nOffset = (size_t)offset, m_nSize = (size_t)size; m_bUseRange = TRUE; m_nCurPos = m_nOffset; @@ -198,13 +205,25 @@ public: if (!buffer || !size) { return FALSE; } + + base::CheckedNumeric safeOffset = offset; if (m_bUseRange) { - offset += (FX_FILESIZE)m_nOffset; + safeOffset += m_nOffset; } - if ((size_t)offset + size > m_nCurSize) { + + if (!safeOffset.IsValid()) { + return FALSE; + } + + offset = safeOffset.ValueOrDie(); + + base::CheckedNumeric newPos = size; + newPos += offset; + if (!newPos.IsValid() || newPos.ValueOrDefault(0) == 0 || newPos.ValueOrDie() > m_nCurSize) { return FALSE; } - m_nCurPos = (size_t)offset + size; + + m_nCurPos = newPos.ValueOrDie(); if (m_dwFlags & FX_MEMSTREAM_Consecutive) { FXSYS_memcpy32(buffer, (FX_LPBYTE)m_Blocks[0] + (size_t)offset, size); return TRUE; @@ -250,7 +269,12 @@ public: offset += (FX_FILESIZE)m_nOffset; } if (m_dwFlags & FX_MEMSTREAM_Consecutive) { - m_nCurPos = (size_t)offset + size; + base::CheckedNumeric 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) { @@ -270,10 +294,16 @@ public: } return TRUE; } - if (!ExpandBlocks((size_t)offset + size)) { + + base::CheckedNumeric newPos = size; + newPos += offset; + if (!newPos.IsValid()) + return FALSE; + + if (!ExpandBlocks(newPos.ValueOrDie())) { return FALSE; } - m_nCurPos = (size_t)offset + size; + m_nCurPos = newPos.ValueOrDie(); size_t nStartBlock = (size_t)offset / m_nGrowSize; offset -= (FX_FILESIZE)(nStartBlock * m_nGrowSize); while (size) { -- cgit v1.2.3