summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom Sepez <tsepez@chromium.org>2017-05-03 14:23:25 -0700
committerChromium commit bot <commit-bot@chromium.org>2017-05-04 13:48:06 +0000
commite4f8fda9e75609b1b23882eca288aa4ea62bc433 (patch)
tree6d1c663095f6c999b7f245ab4d2c9c7426b207b7
parentcbdf926884c4d96a84ff12293fd45b9ec95d5b93 (diff)
downloadpdfium-e4f8fda9e75609b1b23882eca288aa4ea62bc433.tar.xz
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 <dsinclair@chromium.org> Commit-Queue: dsinclair <dsinclair@chromium.org>
-rw-r--r--core/fpdfapi/edit/fpdf_edit_create.cpp8
-rw-r--r--core/fpdfapi/parser/cpdf_data_avail.cpp51
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<IFX_SeekableReadStream> pSrcFile =
m_pParser->GetFileAccess();
- uint8_t buffer[4096]; // TODO(tsepez): don't stack allocate.
+ std::vector<uint8_t> 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<uint32_t>(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<uint8_t> 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<uint8_t> 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<uint8_t> buffer(dwSize);
+ m_pFileRead->ReadBlock(buffer.data(), req_pos, dwSize);
auto file = pdfium::MakeRetain<CFX_MemoryStream>(
- buffer, static_cast<size_t>(dwSize), false);
+ buffer.data(), static_cast<size_t>(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<uint8_t> buf(iSize);
+ m_pFileRead->ReadBlock(buf.data(), m_dwCurrentXRefSteam, iSize);
auto file = pdfium::MakeRetain<CFX_MemoryStream>(
- pBuf, static_cast<size_t>(iSize), false);
+ buf.data(), static_cast<size_t>(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<uint8_t> buf(iSize);
+ if (!m_pFileRead->ReadBlock(buf.data(), m_dwTrailerOffset, iSize))
return false;
auto file = pdfium::MakeRetain<CFX_MemoryStream>(
- pBuf, static_cast<size_t>(iSize), false);
+ buf.data(), static_cast<size_t>(iSize), false);
m_syntaxParser.InitParser(file, 0);
std::unique_ptr<CPDF_Object> pTrailer(