Enforce unsafe buffers on 2-arg StringViewTemplate constructor.
Prepare for a future compiler which will enforce this by making the
constructor private and forcing callers through a static method that
is enforced. Once the compiler is updated, it is a simple matter to
change, for example, ByteStringView::Create(..., ...) back to
ByteStringView(..., ...).
Change-Id: I6aceebbb7865514f41f801d57225f0ea171c298a
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/119677
Reviewed-by: Thomas Sepez <tsepez@google.com>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfapi/font/cpdf_cidfont.cpp b/core/fpdfapi/font/cpdf_cidfont.cpp
index 9bcc84e..8e11285 100644
--- a/core/fpdfapi/font/cpdf_cidfont.cpp
+++ b/core/fpdfapi/font/cpdf_cidfont.cpp
@@ -321,7 +321,8 @@
}
size_t ret = FX_MultiByteToWideChar(
UNSAFE_TODO(kCharsetCodePages[static_cast<size_t>(m_pCMap->GetCoding())]),
- ByteStringView(reinterpret_cast<const char*>(&charcode), charsize),
+ UNSAFE_TODO(ByteStringView::Create(
+ reinterpret_cast<const char*>(&charcode), charsize)),
pdfium::span_from_ref(unicode));
return ret == 1 ? unicode : 0;
#else
@@ -368,7 +369,7 @@
uint8_t buffer[32];
size_t ret = FX_WideCharToMultiByte(
UNSAFE_TODO(kCharsetCodePages[static_cast<size_t>(m_pCMap->GetCoding())]),
- WideStringView(&unicode, 1),
+ WideStringView(unicode),
pdfium::as_writable_chars(pdfium::make_span(buffer).first(4u)));
if (ret == 1)
return buffer[0];
diff --git a/core/fpdfapi/parser/fpdf_parser_decode_unittest.cpp b/core/fpdfapi/parser/fpdf_parser_decode_unittest.cpp
index 2ec773e..f416aac 100644
--- a/core/fpdfapi/parser/fpdf_parser_decode_unittest.cpp
+++ b/core/fpdfapi/parser/fpdf_parser_decode_unittest.cpp
@@ -33,7 +33,8 @@
// Converts a string literal into a `uint8_t` span.
template <size_t N>
pdfium::span<const uint8_t> ToSpan(const char (&array)[N]) {
- return pdfium::as_bytes(ByteStringView(array, N - 1).span());
+ return pdfium::as_bytes(
+ UNSAFE_BUFFERS(ByteStringView::Create(array, N - 1).span()));
}
// Converts a string literal into a `ByteString`.
diff --git a/core/fxcrt/bytestring_unittest.cpp b/core/fxcrt/bytestring_unittest.cpp
index 95ccfe1..c2b84e6 100644
--- a/core/fxcrt/bytestring_unittest.cpp
+++ b/core/fxcrt/bytestring_unittest.cpp
@@ -1221,12 +1221,16 @@
TEST(ByteStringView, NotNull) {
ByteStringView string3("abc");
ByteStringView string6("abcdef");
- ByteStringView alternate_string3("abcdef", 3);
+ // SAFETY: known fixed-length string.
+ auto alternate_string3 = UNSAFE_BUFFERS(ByteStringView::Create("abcdef", 3));
const char abcd[] = "abcd";
ByteStringView span_string4(
pdfium::as_bytes(pdfium::make_span(abcd).first(4u)));
- ByteStringView embedded_nul_string7("abc\0def", 7);
- ByteStringView illegal_string7("abcdef", 7);
+ // SAFETY: known fixed-length string.
+ auto embedded_nul_string7 =
+ UNSAFE_BUFFERS(ByteStringView::Create("abc\0def", 7));
+ // SAFETY: known fixed-length string.
+ auto illegal_string7 = UNSAFE_BUFFERS(ByteStringView::Create("abcdef", 7));
EXPECT_EQ(3u, string3.GetLength());
EXPECT_EQ(6u, string6.GetLength());
@@ -1961,8 +1965,9 @@
// a C++-style string.
{
std::ostringstream stream;
- char stringWithNulls[]{'x', 'y', '\0', 'z'};
- ByteStringView str(stringWithNulls, 4);
+ char stringWithNulls[] = {'x', 'y', '\0', 'z'};
+ // SAFETY: known array above.
+ auto str = UNSAFE_BUFFERS(ByteStringView::Create(stringWithNulls, 4));
EXPECT_EQ(4u, str.GetLength());
stream << str;
EXPECT_EQ(4u, stream.tellp());
diff --git a/core/fxcrt/string_view_template.h b/core/fxcrt/string_view_template.h
index 76df947..6c1bf44 100644
--- a/core/fxcrt/string_view_template.h
+++ b/core/fxcrt/string_view_template.h
@@ -42,6 +42,22 @@
using const_iterator = const CharType*;
using const_reverse_iterator = std::reverse_iterator<const_iterator>;
+ UNSAFE_BUFFER_USAGE static constexpr StringViewTemplate Create(
+ const CharType* ptr,
+ size_t size) {
+ // SAFETY: required from caller, enforced via UNSAFE_BUFFER_USAGE.
+ return UNSAFE_BUFFERS(StringViewTemplate(ptr, size));
+ }
+
+ template <typename E = typename std::enable_if_t<
+ !std::is_same_v<UnsignedType, CharType>>>
+ UNSAFE_BUFFER_USAGE static constexpr StringViewTemplate Create(
+ const UnsignedType* ptr,
+ size_t size) {
+ // SAFETY: required from caller, enforced via UNSAFE_BUFFER_USAGE.
+ return UNSAFE_BUFFERS(StringViewTemplate(ptr, size));
+ }
+
constexpr StringViewTemplate() noexcept = default;
constexpr StringViewTemplate(const StringViewTemplate& src) noexcept =
default;
@@ -54,20 +70,6 @@
reinterpret_cast<const UnsignedType*>(ptr),
ptr ? std::char_traits<CharType>::length(ptr) : 0))) {}
- UNSAFE_BUFFER_USAGE
- constexpr StringViewTemplate(const CharType* ptr, size_t size) noexcept
- // SAFETY: propagated to caller via UNSAFE_BUFFER_USAGE.
- : m_Span(UNSAFE_BUFFERS(
- pdfium::make_span(reinterpret_cast<const UnsignedType*>(ptr),
- size))) {}
-
- template <typename E = typename std::enable_if<
- !std::is_same<UnsignedType, CharType>::value>::type>
- UNSAFE_BUFFER_USAGE constexpr StringViewTemplate(const UnsignedType* ptr,
- size_t size) noexcept
- // SAFETY: propagated to caller via UNSAFE_BUFFER_USAGE.
- : m_Span(UNSAFE_BUFFERS(pdfium::make_span(ptr, size))) {}
-
explicit constexpr StringViewTemplate(
const pdfium::span<const CharType>& other) noexcept {
if (!other.empty()) {
@@ -291,6 +293,20 @@
}
protected:
+ UNSAFE_BUFFER_USAGE
+ constexpr StringViewTemplate(const CharType* ptr, size_t size) noexcept
+ // SAFETY: propagated to caller via UNSAFE_BUFFER_USAGE.
+ : m_Span(UNSAFE_BUFFERS(
+ pdfium::make_span(reinterpret_cast<const UnsignedType*>(ptr),
+ size))) {}
+
+ template <typename E = typename std::enable_if<
+ !std::is_same<UnsignedType, CharType>::value>::type>
+ UNSAFE_BUFFER_USAGE constexpr StringViewTemplate(const UnsignedType* ptr,
+ size_t size) noexcept
+ // SAFETY: propagated to caller via UNSAFE_BUFFER_USAGE.
+ : m_Span(UNSAFE_BUFFERS(pdfium::make_span(ptr, size))) {}
+
// This is not a raw_span<> because StringViewTemplates must be passed by
// value without introducing BackupRefPtr churn. Also, repeated re-assignment
// of substrings of a StringViewTemplate to itself must avoid the same issue.
diff --git a/core/fxcrt/widestring_unittest.cpp b/core/fxcrt/widestring_unittest.cpp
index 7b89ca2..727ffc4 100644
--- a/core/fxcrt/widestring_unittest.cpp
+++ b/core/fxcrt/widestring_unittest.cpp
@@ -2136,8 +2136,9 @@
// a C++-style string.
{
wchar_t stringWithNulls[]{'x', 'y', '\0', 'z'};
+ // SAFETY: known array above.
+ auto str = UNSAFE_BUFFERS(WideStringView::Create(stringWithNulls, 4));
std::ostringstream stream;
- WideStringView str(stringWithNulls, 4);
EXPECT_EQ(4u, str.GetLength());
stream << str;
EXPECT_EQ(4u, stream.tellp());
@@ -2214,8 +2215,9 @@
// a C++-style string.
{
wchar_t stringWithNulls[]{'x', 'y', '\0', 'z'};
+ // SAFETY: known array above.
+ auto str = UNSAFE_BUFFERS(WideStringView::Create(stringWithNulls, 4));
std::wostringstream stream;
- WideStringView str(stringWithNulls, 4);
EXPECT_EQ(4u, str.GetLength());
stream << str;
EXPECT_EQ(4u, stream.tellp());
diff --git a/fxjs/fxv8.cpp b/fxjs/fxv8.cpp
index eafd5e2..95b0dfe 100644
--- a/fxjs/fxv8.cpp
+++ b/fxjs/fxv8.cpp
@@ -6,6 +6,7 @@
#include "fxjs/fxv8.h"
+#include "core/fxcrt/compiler_specific.h"
#include "core/fxcrt/numerics/safe_conversions.h"
#include "v8/include/v8-container.h"
#include "v8/include/v8-date.h"
@@ -120,7 +121,9 @@
WideString ToWideString(v8::Isolate* pIsolate, v8::Local<v8::String> pValue) {
v8::String::Utf8Value s(pIsolate, pValue);
- return WideString::FromUTF8(ByteStringView(*s, s.length()));
+ // SAFETY: required from V8.
+ return WideString::FromUTF8(
+ UNSAFE_BUFFERS(ByteStringView::Create(*s, s.length())));
}
ByteString ToByteString(v8::Isolate* pIsolate, v8::Local<v8::String> pValue) {
diff --git a/fxjs/xfa/cfxjse_class.cpp b/fxjs/xfa/cfxjse_class.cpp
index da8053e..adb3d48 100644
--- a/fxjs/xfa/cfxjse_class.cpp
+++ b/fxjs/xfa/cfxjse_class.cpp
@@ -11,6 +11,7 @@
#include "core/fxcrt/check.h"
#include "core/fxcrt/check_op.h"
+#include "core/fxcrt/compiler_specific.h"
#include "fxjs/cjs_result.h"
#include "fxjs/fxv8.h"
#include "fxjs/js_resources.h"
@@ -203,7 +204,9 @@
v8::HandleScope scope(info.GetIsolate());
v8::String::Utf8Value szPropName(info.GetIsolate(), property);
- ByteStringView szFxPropName(*szPropName, szPropName.length());
+ // SAFETY: required from V8.
+ auto szFxPropName =
+ UNSAFE_BUFFERS(ByteStringView::Create(*szPropName, szPropName.length()));
if (DynPropQueryAdapter(info.GetIsolate(), pClass, info.Holder(),
szFxPropName)) {
info.GetReturnValue().Set(v8::DontDelete);
@@ -223,7 +226,9 @@
}
v8::String::Utf8Value szPropName(info.GetIsolate(), property);
- ByteStringView szFxPropName(*szPropName, szPropName.length());
+ // SAFETY: required from V8.
+ auto szFxPropName =
+ UNSAFE_BUFFERS(ByteStringView::Create(*szPropName, szPropName.length()));
std::unique_ptr<CFXJSE_Value> pNewValue = DynPropGetterAdapter(
info.GetIsolate(), pClass, info.Holder(), szFxPropName);
info.GetReturnValue().Set(pNewValue->DirectGetValue());
@@ -241,7 +246,9 @@
}
v8::String::Utf8Value szPropName(info.GetIsolate(), property);
- ByteStringView szFxPropName(*szPropName, szPropName.length());
+ // SAFETY: required from V8.
+ auto szFxPropName =
+ UNSAFE_BUFFERS(ByteStringView::Create(*szPropName, szPropName.length()));
auto pNewValue = std::make_unique<CFXJSE_Value>(info.GetIsolate(), value);
DynPropSetterAdapter(info.GetIsolate(), pClass, info.Holder(), szFxPropName,
pNewValue.get());
diff --git a/testing/fuzzers/pdf_cfgas_stringformatter_fuzzer.cc b/testing/fuzzers/pdf_cfgas_stringformatter_fuzzer.cc
index 12f6206..ba9d538 100644
--- a/testing/fuzzers/pdf_cfgas_stringformatter_fuzzer.cc
+++ b/testing/fuzzers/pdf_cfgas_stringformatter_fuzzer.cc
@@ -9,6 +9,7 @@
#include <iterator>
#include "core/fxcrt/cfx_datetime.h"
+#include "core/fxcrt/compiler_specific.h"
#include "core/fxcrt/fx_string.h"
#include "public/fpdfview.h"
#include "testing/fuzzers/pdfium_fuzzer_util.h"
@@ -43,10 +44,10 @@
size_t pattern_len = size / 2;
size_t value_len = size - pattern_len;
- WideString pattern =
- WideString::FromLatin1(ByteStringView(data, pattern_len));
- WideString value =
- WideString::FromLatin1(ByteStringView(data + pattern_len, value_len));
+ WideString pattern = WideString::FromLatin1(
+ UNSAFE_TODO(ByteStringView::Create(data, pattern_len)));
+ WideString value = WideString::FromLatin1(
+ UNSAFE_TODO(ByteStringView::Create(data + pattern_len, value_len)));
auto fmt = std::make_unique<CFGAS_StringFormatter>(pattern);
diff --git a/testing/fuzzers/pdf_css_fuzzer.cc b/testing/fuzzers/pdf_css_fuzzer.cc
index f11705c..2f6c200 100644
--- a/testing/fuzzers/pdf_css_fuzzer.cc
+++ b/testing/fuzzers/pdf_css_fuzzer.cc
@@ -7,8 +7,9 @@
#include "core/fxcrt/fx_string.h"
extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
- WideString input =
- WideString::FromUTF8(ByteStringView(data, static_cast<size_t>(size)));
+ // SAFETY: required from fuzzer.
+ WideString input = WideString::FromUTF8(
+ UNSAFE_BUFFERS(ByteStringView::Create(data, static_cast<size_t>(size))));
// If we convert the input into an empty string bail out.
if (input.IsEmpty())
diff --git a/testing/fuzzers/pdf_formcalc_context_fuzzer.cc b/testing/fuzzers/pdf_formcalc_context_fuzzer.cc
index c1255b0..14ccff1 100644
--- a/testing/fuzzers/pdf_formcalc_context_fuzzer.cc
+++ b/testing/fuzzers/pdf_formcalc_context_fuzzer.cc
@@ -6,6 +6,7 @@
#include <memory>
+#include "core/fxcrt/compiler_specific.h"
#include "core/fxcrt/fx_string.h"
#include "fpdfsdk/cpdfsdk_helpers.h"
#include "fpdfsdk/fpdfxfa/cpdfxfa_context.h"
@@ -525,7 +526,7 @@
params.m_bCancelAction = false;
CFXJSE_Engine::EventParamScope param_scope(script_context, nullptr,
¶ms);
- ByteStringView data_view(data_, size_);
+ auto data_view = UNSAFE_TODO(ByteStringView::Create(data_, size_));
script_context->RunScript(CXFA_Script::Type::Formcalc,
WideString::FromUTF8(data_view).AsStringView(),
xfa_document->GetRoot());
diff --git a/testing/fuzzers/pdf_formcalc_fuzzer.cc b/testing/fuzzers/pdf_formcalc_fuzzer.cc
index d8a2a895..b03ac0b 100644
--- a/testing/fuzzers/pdf_formcalc_fuzzer.cc
+++ b/testing/fuzzers/pdf_formcalc_fuzzer.cc
@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include "core/fxcrt/compiler_specific.h"
#include "core/fxcrt/fx_string.h"
#include "testing/fuzzers/pdfium_fuzzer_util.h"
#include "testing/fuzzers/xfa_process_state.h"
@@ -9,7 +10,9 @@
extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
auto* state = static_cast<XFAProcessState*>(FPDF_GetFuzzerPerProcessState());
- WideString input = WideString::FromUTF8(ByteStringView(data, size));
+ // SAFETY: required from fuzzer.
+ WideString input =
+ WideString::FromUTF8(UNSAFE_BUFFERS(ByteStringView::Create(data, size)));
CXFA_FMLexer lexer(input.AsStringView());
CXFA_FMParser parser(state->GetHeap(), &lexer);
parser.Parse();
diff --git a/testing/fuzzers/pdf_formcalc_translate_fuzzer.cc b/testing/fuzzers/pdf_formcalc_translate_fuzzer.cc
index 82b8188..d3c8e63 100644
--- a/testing/fuzzers/pdf_formcalc_translate_fuzzer.cc
+++ b/testing/fuzzers/pdf_formcalc_translate_fuzzer.cc
@@ -5,6 +5,7 @@
#include <stddef.h>
#include <stdint.h>
+#include "core/fxcrt/compiler_specific.h"
#include "core/fxcrt/fx_safe_types.h"
#include "core/fxcrt/fx_string.h"
#include "fxjs/xfa/cfxjse_formcalc_context.h"
@@ -13,7 +14,9 @@
extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
auto* state = static_cast<XFAProcessState*>(FPDF_GetFuzzerPerProcessState());
- WideString input = WideString::FromUTF8(ByteStringView(data, size));
+ // SAFETY: required from fuzzer.
+ WideString input =
+ WideString::FromUTF8(UNSAFE_BUFFERS(ByteStringView::Create(data, size)));
CFXJSE_FormCalcContext::Translate(state->GetHeap(), input.AsStringView());
state->ForceGCAndPump();
return 0;
diff --git a/xfa/fgas/font/cfgas_defaultfontmanager.cpp b/xfa/fgas/font/cfgas_defaultfontmanager.cpp
index 7ff89d6..ec57a66 100644
--- a/xfa/fgas/font/cfgas_defaultfontmanager.cpp
+++ b/xfa/fgas/font/cfgas_defaultfontmanager.cpp
@@ -45,8 +45,8 @@
UNSAFE_TODO(pNameText++);
iLength--;
}
- WideString wsReplace =
- WideString::FromASCII(ByteStringView(pReplace, pNameText - pReplace));
+ WideString wsReplace = WideString::FromASCII(
+ UNSAFE_TODO(ByteStringView::Create(pReplace, pNameText - pReplace)));
pFont =
pFontMgr->LoadFont(wsReplace.c_str(), dwStyle, FX_CodePage::kFailure);
if (pFont)
diff --git a/xfa/fgas/layout/cfgas_rtfbreak_unittest.cpp b/xfa/fgas/layout/cfgas_rtfbreak_unittest.cpp
index 22421f3..43519ab 100644
--- a/xfa/fgas/layout/cfgas_rtfbreak_unittest.cpp
+++ b/xfa/fgas/layout/cfgas_rtfbreak_unittest.cpp
@@ -80,7 +80,9 @@
rtf_break->SetLineBreakTolerance(1);
rtf_break->SetFontSize(12);
- WideString input = WideString::FromUTF8(ByteStringView("\xa\x0\xa\xa", 4));
+ // SAFETY: known fixed-length string.
+ WideString input = WideString::FromUTF8(
+ UNSAFE_BUFFERS(ByteStringView::Create("\xa\x0\xa\xa", 4)));
for (wchar_t ch : input)
rtf_break->AppendChar(ch);
diff --git a/xfa/fgas/layout/cfgas_txtbreak_unittest.cpp b/xfa/fgas/layout/cfgas_txtbreak_unittest.cpp
index ff472ac..6fed525 100644
--- a/xfa/fgas/layout/cfgas_txtbreak_unittest.cpp
+++ b/xfa/fgas/layout/cfgas_txtbreak_unittest.cpp
@@ -37,7 +37,9 @@
txt_break->SetLineBreakTolerance(1);
txt_break->SetFontSize(12);
- WideString input = WideString::FromUTF8(ByteStringView("\xa\x0\xa\xa", 4));
+ // SAFETY: known fixed-length string.
+ WideString input = WideString::FromUTF8(
+ UNSAFE_BUFFERS(ByteStringView::Create("\xa\x0\xa\xa", 4)));
for (wchar_t ch : input)
txt_break->AppendChar(ch);
diff --git a/xfa/fxfa/cxfa_textlayout.cpp b/xfa/fxfa/cxfa_textlayout.cpp
index 575e946..696324c 100644
--- a/xfa/fxfa/cxfa_textlayout.cpp
+++ b/xfa/fxfa/cxfa_textlayout.cpp
@@ -12,6 +12,7 @@
#include <utility>
#include "core/fxcrt/check.h"
+#include "core/fxcrt/compiler_specific.h"
#include "core/fxcrt/css/cfx_csscomputedstyle.h"
#include "core/fxcrt/css/cfx_cssstyleselector.h"
#include "core/fxcrt/notreached.h"
@@ -802,10 +803,11 @@
wsText = WideString(L'\n');
} else if (wsName.EqualsASCII("li")) {
bCurLi = true;
- if (bIsOl)
+ if (bIsOl) {
wsText = WideString::Format(L"%d. ", iLiCount);
- else
- wsText = 0x00B7 + WideStringView(L" ", 1);
+ } else {
+ wsText = 0x00B7 + WideStringView(L" ");
+ }
} else if (!bContentNode) {
if (iTabCount > 0) {
while (iTabCount-- > 0)
diff --git a/xfa/fxfa/formcalc/cxfa_fmlexer_unittest.cpp b/xfa/fxfa/formcalc/cxfa_fmlexer_unittest.cpp
index aaeaa43..5224fca 100644
--- a/xfa/fxfa/formcalc/cxfa_fmlexer_unittest.cpp
+++ b/xfa/fxfa/formcalc/cxfa_fmlexer_unittest.cpp
@@ -308,7 +308,8 @@
}
TEST(CXFA_FMLexerTest, NullData) {
- CXFA_FMLexer lexer(WideStringView(L"\x2d\x32\x00\x2d\x32", 5));
+ CXFA_FMLexer lexer(
+ UNSAFE_TODO(WideStringView::Create(L"\x2d\x32\x00\x2d\x32", 5)));
CXFA_FMLexer::Token token = lexer.NextToken();
EXPECT_EQ(TOKminus, token.GetType());
