From e4f8fda9e75609b1b23882eca288aa4ea62bc433 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Wed, 3 May 2017 14:23:25 -0700 Subject: CPDF_DataAvail: avoid reads into stack buffers. Not a good practice even with correct bounds checks. Same idea for fpdf_edit_create.cpp Change-Id: I90b869ae4a07eb60d59997b9c5afc14830efc076 Reviewed-on: https://pdfium-review.googlesource.com/4830 Reviewed-by: dsinclair Commit-Queue: dsinclair --- core/fpdfapi/edit/fpdf_edit_create.cpp | 8 ++++-- core/fpdfapi/parser/cpdf_data_avail.cpp | 51 +++++++++++++-------------------- 2 files changed, 25 insertions(+), 34 deletions(-) diff --git a/core/fpdfapi/edit/fpdf_edit_create.cpp b/core/fpdfapi/edit/fpdf_edit_create.cpp index 24ec0d7b55..d0482173bb 100644 --- a/core/fpdfapi/edit/fpdf_edit_create.cpp +++ b/core/fpdfapi/edit/fpdf_edit_create.cpp @@ -1424,13 +1424,15 @@ int32_t CPDF_Creator::WriteDoc_Stage1(IFX_Pause* pPause) { if ((m_dwFlags & FPDFCREATE_NO_ORIGINAL) == 0 && m_SavedOffset > 0) { CFX_RetainPtr pSrcFile = m_pParser->GetFileAccess(); - uint8_t buffer[4096]; // TODO(tsepez): don't stack allocate. + std::vector buffer(4096); FX_FILESIZE src_size = m_SavedOffset; while (src_size) { uint32_t block_size = src_size > 4096 ? 4096 : src_size; - if (!pSrcFile->ReadBlock(buffer, m_Offset - src_size, block_size)) + if (!pSrcFile->ReadBlock(buffer.data(), m_Offset - src_size, + block_size)) { return -1; - if (m_File.AppendBlock(buffer, block_size) < 0) + } + if (m_File.AppendBlock(buffer.data(), block_size) < 0) return -1; src_size -= block_size; diff --git a/core/fpdfapi/parser/cpdf_data_avail.cpp b/core/fpdfapi/parser/cpdf_data_avail.cpp index b13b982430..b4da893429 100644 --- a/core/fpdfapi/parser/cpdf_data_avail.cpp +++ b/core/fpdfapi/parser/cpdf_data_avail.cpp @@ -563,21 +563,20 @@ bool CPDF_DataAvail::CheckPages(DownloadHints* pHints) { bool CPDF_DataAvail::CheckHeader(DownloadHints* pHints) { ASSERT(m_dwFileLen >= 0); const uint32_t kReqSize = std::min(static_cast(m_dwFileLen), 1024U); - if (!m_pFileAvail->IsDataAvail(0, kReqSize)) { pHints->AddSegment(0, kReqSize); return false; } - - uint8_t buffer[1024]; - m_pFileRead->ReadBlock(buffer, 0, kReqSize); - if (IsLinearizedFile(buffer, kReqSize)) { + std::vector buffer(kReqSize); + m_pFileRead->ReadBlock(buffer.data(), 0, kReqSize); + if (IsLinearizedFile(buffer.data(), kReqSize)) { m_docStatus = PDF_DATAAVAIL_FIRSTPAGE; - } else { - if (m_docStatus == PDF_DATAAVAIL_ERROR) - return false; - m_docStatus = PDF_DATAAVAIL_END; + return true; } + if (m_docStatus == PDF_DATAAVAIL_ERROR) + return false; + + m_docStatus = PDF_DATAAVAIL_END; return true; } @@ -701,9 +700,9 @@ CPDF_DataAvail::DocLinearizationStatus CPDF_DataAvail::IsLinearizedPDF() { if (dwSize < (FX_FILESIZE)kReqSize) return LinearizationUnknown; - uint8_t buffer[1024]; - m_pFileRead->ReadBlock(buffer, 0, kReqSize); - if (IsLinearizedFile(buffer, kReqSize)) + std::vector buffer(kReqSize); + m_pFileRead->ReadBlock(buffer.data(), 0, kReqSize); + if (IsLinearizedFile(buffer.data(), kReqSize)) return Linearized; return NotLinearized; @@ -753,11 +752,11 @@ bool CPDF_DataAvail::CheckEnd(DownloadHints* pHints) { return false; } - uint8_t buffer[1024]; - m_pFileRead->ReadBlock(buffer, req_pos, dwSize); + std::vector buffer(dwSize); + m_pFileRead->ReadBlock(buffer.data(), req_pos, dwSize); auto file = pdfium::MakeRetain( - buffer, static_cast(dwSize), false); + buffer.data(), static_cast(dwSize), false); m_syntaxParser.InitParser(file, 0); m_syntaxParser.SetPos(dwSize - 1); if (!m_syntaxParser.BackwardsSearchToWord("startxref", dwSize)) { @@ -772,13 +771,11 @@ bool CPDF_DataAvail::CheckEnd(DownloadHints* pHints) { m_docStatus = PDF_DATAAVAIL_ERROR; return false; } - m_dwXRefOffset = (FX_FILESIZE)FXSYS_atoi64(xrefpos_str.c_str()); if (!m_dwXRefOffset || m_dwXRefOffset > m_dwFileLen) { m_docStatus = PDF_DATAAVAIL_LOADALLFILE; return true; } - m_dwLastXRefOffset = m_dwXRefOffset; SetStartOffset(m_dwXRefOffset); m_docStatus = PDF_DATAAVAIL_CROSSREF; @@ -797,13 +794,11 @@ int32_t CPDF_DataAvail::CheckCrossRefStream(DownloadHints* pHints, } int32_t iSize = (int32_t)(m_Pos + req_size - m_dwCurrentXRefSteam); - CFX_BinaryBuf buf(iSize); - uint8_t* pBuf = buf.GetBuffer(); - - m_pFileRead->ReadBlock(pBuf, m_dwCurrentXRefSteam, iSize); + std::vector buf(iSize); + m_pFileRead->ReadBlock(buf.data(), m_dwCurrentXRefSteam, iSize); auto file = pdfium::MakeRetain( - pBuf, static_cast(iSize), false); + buf.data(), static_cast(iSize), false); m_parser.m_pSyntax->InitParser(file, 0); bool bNumber; @@ -1037,18 +1032,12 @@ bool CPDF_DataAvail::CheckTrailer(DownloadHints* pHints) { } int32_t iSize = (int32_t)(m_Pos + iTrailerSize - m_dwTrailerOffset); - CFX_BinaryBuf buf(iSize); - uint8_t* pBuf = buf.GetBuffer(); - if (!pBuf) { - m_docStatus = PDF_DATAAVAIL_ERROR; - return false; - } - - if (!m_pFileRead->ReadBlock(pBuf, m_dwTrailerOffset, iSize)) + std::vector buf(iSize); + if (!m_pFileRead->ReadBlock(buf.data(), m_dwTrailerOffset, iSize)) return false; auto file = pdfium::MakeRetain( - pBuf, static_cast(iSize), false); + buf.data(), static_cast(iSize), false); m_syntaxParser.InitParser(file, 0); std::unique_ptr pTrailer( -- cgit v1.2.3