Resolve unsafe buffer issues in CPDF_TextObject
Bug: pdfium:2155
Change-Id: Ie8d2d8bf8be8303f3489362845384ee6d17aabe9
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/118953
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Thomas Sepez <tsepez@google.com>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fpdfapi/page/cpdf_streamcontentparser.cpp b/core/fpdfapi/page/cpdf_streamcontentparser.cpp
index d6257a3..58f2e36 100644
--- a/core/fpdfapi/page/cpdf_streamcontentparser.cpp
+++ b/core/fpdfapi/page/cpdf_streamcontentparser.cpp
@@ -1244,24 +1244,26 @@
->GetShading(std::move(pPattern), m_pCurStates->parent_matrix());
}
-void CPDF_StreamContentParser::AddTextObject(const ByteString* pStrs,
- float fInitKerning,
- const std::vector<float>& kernings,
- size_t nSegs) {
+void CPDF_StreamContentParser::AddTextObject(
+ pdfium::span<const ByteString> strings,
+ pdfium::span<const float> kernings,
+ float initial_kerning) {
RetainPtr<CPDF_Font> pFont = m_pCurStates->text_state().GetFont();
- if (!pFont)
+ if (!pFont) {
return;
-
- if (fInitKerning != 0) {
- if (pFont->IsVertWriting())
- m_pCurStates->IncrementTextPositionY(-GetVerticalTextSize(fInitKerning));
- else
- m_pCurStates->IncrementTextPositionX(
- -GetHorizontalTextSize(fInitKerning));
}
- if (nSegs == 0)
+ if (initial_kerning != 0) {
+ if (pFont->IsVertWriting()) {
+ m_pCurStates->IncrementTextPositionY(
+ -GetVerticalTextSize(initial_kerning));
+ } else {
+ m_pCurStates->IncrementTextPositionX(
+ -GetHorizontalTextSize(initial_kerning));
+ }
+ }
+ if (strings.empty()) {
return;
-
+ }
const TextRenderingMode text_mode =
pFont->IsType3Font() ? TextRenderingMode::MODE_FILL
: m_pCurStates->text_state().GetTextMode();
@@ -1278,7 +1280,7 @@
text_ctm[2] = ctm.b;
text_ctm[3] = ctm.d;
}
- pText->SetSegments(pStrs, kernings, nSegs);
+ pText->SetSegments(strings, kernings);
pText->SetPosition(m_mtContentToUser.Transform(
m_pCurStates->GetTransformedTextPosition()));
@@ -1290,13 +1292,13 @@
m_ClipTextList.push_back(pText->Clone());
m_pObjectHolder->AppendPageObject(std::move(pText));
}
- if (!kernings.empty() && kernings[nSegs - 1] != 0) {
+ if (!kernings.empty() && kernings.back() != 0) {
if (pFont->IsVertWriting())
m_pCurStates->IncrementTextPositionY(
- -GetVerticalTextSize(kernings[nSegs - 1]));
+ -GetVerticalTextSize(kernings.back()));
else
m_pCurStates->IncrementTextPositionX(
- -GetHorizontalTextSize(kernings[nSegs - 1]));
+ -GetHorizontalTextSize(kernings.back()));
}
}
@@ -1317,8 +1319,9 @@
void CPDF_StreamContentParser::Handle_ShowText() {
ByteString str = GetString(0);
- if (!str.IsEmpty())
- AddTextObject(&str, 0, std::vector<float>(), 1);
+ if (!str.IsEmpty()) {
+ AddTextObject(pdfium::span_from_ref(str), pdfium::span<float>(), 0.0f);
+ }
}
void CPDF_StreamContentParser::Handle_ShowText_Positioning() {
@@ -1364,7 +1367,8 @@
kernings[iSegment - 1] += num;
}
}
- AddTextObject(strs.data(), fInitKerning, kernings, iSegment);
+ AddTextObject(pdfium::make_span(strs).first(iSegment), kernings,
+ fInitKerning);
}
void CPDF_StreamContentParser::Handle_SetTextLeading() {
diff --git a/core/fpdfapi/page/cpdf_streamcontentparser.h b/core/fpdfapi/page/cpdf_streamcontentparser.h
index 6f8d8b9..1a9a0f3 100644
--- a/core/fpdfapi/page/cpdf_streamcontentparser.h
+++ b/core/fpdfapi/page/cpdf_streamcontentparser.h
@@ -105,10 +105,9 @@
// Makes a matrix from {GetNumber(5), ..., GetNumber(0)}.
CFX_Matrix GetMatrix() const;
void OnOperator(ByteStringView op);
- void AddTextObject(const ByteString* pStrs,
- float fInitKerning,
- const std::vector<float>& kernings,
- size_t nSegs);
+ void AddTextObject(pdfium::span<const ByteString> strings,
+ pdfium::span<const float> kernings,
+ float initial_kerning);
float GetHorizontalTextSize(float fKerning) const;
float GetVerticalTextSize(float fKerning) const;
diff --git a/core/fpdfapi/page/cpdf_textobject.cpp b/core/fpdfapi/page/cpdf_textobject.cpp
index 4fb1b5f..f552f40 100644
--- a/core/fpdfapi/page/cpdf_textobject.cpp
+++ b/core/fpdfapi/page/cpdf_textobject.cpp
@@ -4,11 +4,6 @@
// Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
-#if defined(UNSAFE_BUFFERS_BUILD)
-// TODO(crbug.com/pdfium/2153): resolve buffer safety issues.
-#pragma allow_unsafe_buffers
-#endif
-
#include "core/fpdfapi/page/cpdf_textobject.h"
#include <algorithm>
@@ -18,6 +13,7 @@
#include "core/fxcrt/check.h"
#include "core/fxcrt/fx_coordinates.h"
#include "core/fxcrt/span.h"
+#include "core/fxcrt/span_util.h"
#define ISLATINWORD(u) (u != 0x20 && u <= 0x28FF)
@@ -198,23 +194,23 @@
CalcPositionDataInternal(GetFont());
}
-void CPDF_TextObject::SetSegments(const ByteString* pStrs,
- const std::vector<float>& kernings,
- size_t nSegs) {
+void CPDF_TextObject::SetSegments(pdfium::span<const ByteString> strings,
+ pdfium::span<const float> kernings) {
+ size_t nSegs = strings.size();
CHECK(nSegs);
m_CharCodes.clear();
m_CharPos.clear();
RetainPtr<CPDF_Font> pFont = GetFont();
size_t nChars = nSegs - 1;
- for (size_t i = 0; i < nSegs; ++i)
- nChars += pFont->CountChar(pStrs[i].AsStringView());
-
+ for (const auto& str : strings) {
+ nChars += pFont->CountChar(str.AsStringView());
+ }
CHECK(nChars);
m_CharCodes.resize(nChars);
m_CharPos.resize(nChars - 1);
size_t index = 0;
for (size_t i = 0; i < nSegs; ++i) {
- ByteStringView segment = pStrs[i].AsStringView();
+ ByteStringView segment = strings[i].AsStringView();
size_t offset = 0;
while (offset < segment.GetLength()) {
DCHECK(index < m_CharCodes.size());
@@ -228,7 +224,7 @@
}
void CPDF_TextObject::SetText(const ByteString& str) {
- SetSegments(&str, std::vector<float>(), 1);
+ SetSegments(pdfium::span_from_ref(str), pdfium::span<float>());
CalcPositionDataInternal(GetFont());
SetDirty(true);
}
diff --git a/core/fpdfapi/page/cpdf_textobject.h b/core/fpdfapi/page/cpdf_textobject.h
index 4a936bc..374bc8e 100644
--- a/core/fpdfapi/page/cpdf_textobject.h
+++ b/core/fpdfapi/page/cpdf_textobject.h
@@ -17,6 +17,7 @@
#include "core/fxcrt/fx_coordinates.h"
#include "core/fxcrt/fx_string.h"
#include "core/fxcrt/retain_ptr.h"
+#include "core/fxcrt/span.h"
class CPDF_TextObject final : public CPDF_PageObject {
public:
@@ -70,9 +71,9 @@
// Caller is expected to call SetDirty(true) when done changing the object.
void SetTextMatrix(const CFX_Matrix& matrix);
- void SetSegments(const ByteString* pStrs,
- const std::vector<float>& kernings,
- size_t nSegs);
+ void SetSegments(pdfium::span<const ByteString> strings,
+ pdfium::span<const float> kernings);
+
CFX_PointF CalcPositionData(float horz_scale);
private:
