Apply UNSAFE_BUFFERS() to two-arg span ctor in fpdfsdk/
There isn't any reason to trust a programmer-provided length vs.
those deduced by the compiler.
-- Re-write some cases to use subspan where possible.
-- Flag the rest as UNSAFE_BUFFERS().
Change-Id: I8d2fded1c473c5320fca034e0d66b110071076a7
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/118250
Reviewed-by: Thomas Sepez <tsepez@google.com>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/fpdfsdk/cpdfsdk_formfillenvironment.cpp b/fpdfsdk/cpdfsdk_formfillenvironment.cpp
index 8e146b6..ec4ca48 100644
--- a/fpdfsdk/cpdfsdk_formfillenvironment.cpp
+++ b/fpdfsdk/cpdfsdk_formfillenvironment.cpp
@@ -155,7 +155,7 @@
return WideString();
return WideString::FromUTF16LE(
- {pBuff.data(), static_cast<size_t>(nActualLen)});
+ pdfium::make_span(pBuff).first(static_cast<size_t>(nActualLen)));
#else // PDF_ENABLE_XFA
return WideString();
#endif // PDF_ENABLE_XFA
@@ -177,7 +177,7 @@
return WideString();
return WideString::FromUTF16LE(
- {pBuff.data(), static_cast<size_t>(nActualLen)});
+ pdfium::make_span(pBuff).first(static_cast<size_t>(nActualLen)));
#else // PDF_ENABLE_XFA
return WideString();
#endif // PDF_ENABLE_XFA
@@ -569,9 +569,10 @@
AsFPDFWideString(&bsContentType), AsFPDFWideString(&bsEncode),
AsFPDFWideString(&bsHeader), &response);
- WideString wsRet =
- WideString::FromUTF16LE({reinterpret_cast<const uint8_t*>(response.str),
- static_cast<size_t>(response.len)});
+ // SAFETY: required from FFI callback.
+ WideString wsRet = WideString::FromUTF16LE(UNSAFE_BUFFERS(
+ pdfium::make_span(reinterpret_cast<const uint8_t*>(response.str),
+ static_cast<size_t>(response.len))));
FPDF_BStr_Clear(&response);
return wsRet;
diff --git a/fpdfsdk/cpdfsdk_helpers.cpp b/fpdfsdk/cpdfsdk_helpers.cpp
index d2288c1..e9bc2d8 100644
--- a/fpdfsdk/cpdfsdk_helpers.cpp
+++ b/fpdfsdk/cpdfsdk_helpers.cpp
@@ -224,9 +224,12 @@
return WideStringFromFPDFWideString(wide_string).ToUTF8();
}
+// TOOO(tsepez): should be UNSAFE_BUFFER_USAGE.
WideString WideStringFromFPDFWideString(FPDF_WIDESTRING wide_string) {
- return WideString::FromUTF16LE({reinterpret_cast<const uint8_t*>(wide_string),
- FPDFWideStringLength(wide_string) * 2});
+ // SAFETY: caller ensures `wide_string` is NUL-terminated.
+ return WideString::FromUTF16LE(UNSAFE_BUFFERS(
+ pdfium::make_span(reinterpret_cast<const uint8_t*>(wide_string),
+ FPDFWideStringLength(wide_string) * 2)));
}
#ifdef PDF_ENABLE_XFA
diff --git a/fpdfsdk/fpdf_annot_embeddertest.cpp b/fpdfsdk/fpdf_annot_embeddertest.cpp
index a9d83df..3d41f25 100644
--- a/fpdfsdk/fpdf_annot_embeddertest.cpp
+++ b/fpdfsdk/fpdf_annot_embeddertest.cpp
@@ -1766,9 +1766,8 @@
EXPECT_EQ(kExpectNormalAPLength,
FPDFAnnot_GetAP(annot.get(), FPDF_ANNOT_APPEARANCEMODE_NORMAL,
buf.data(), normal_length_bytes));
- EXPECT_EQ(kMd5NormalAP,
- GenerateMD5Base16({reinterpret_cast<uint8_t*>(buf.data()),
- normal_length_bytes}));
+ EXPECT_EQ(kMd5NormalAP, GenerateMD5Base16(pdfium::as_byte_span(buf).first(
+ normal_length_bytes)));
// Check that the string value of an AP is returned through a buffer that is
// larger than necessary.
@@ -1776,9 +1775,8 @@
EXPECT_EQ(kExpectNormalAPLength,
FPDFAnnot_GetAP(annot.get(), FPDF_ANNOT_APPEARANCEMODE_NORMAL,
buf.data(), normal_length_bytes + 2));
- EXPECT_EQ(kMd5NormalAP,
- GenerateMD5Base16({reinterpret_cast<uint8_t*>(buf.data()),
- normal_length_bytes}));
+ EXPECT_EQ(kMd5NormalAP, GenerateMD5Base16(pdfium::as_byte_span(buf).first(
+ normal_length_bytes)));
// Check that getting an AP for a mode that does not have an AP returns an
// empty string.
@@ -1819,9 +1817,8 @@
EXPECT_EQ(kExpectNormalAPLength,
FPDFAnnot_GetAP(annot.get(), FPDF_ANNOT_APPEARANCEMODE_NORMAL,
buf.data(), normal_length_bytes));
- EXPECT_EQ(kMd5NormalAP,
- GenerateMD5Base16({reinterpret_cast<uint8_t*>(buf.data()),
- normal_length_bytes}));
+ EXPECT_EQ(kMd5NormalAP, GenerateMD5Base16(pdfium::as_byte_span(buf).first(
+ normal_length_bytes)));
}
// Save the modified document, then reopen it.
diff --git a/fpdfsdk/fpdf_attachment.cpp b/fpdfsdk/fpdf_attachment.cpp
index 8fc5e54..4293c57 100644
--- a/fpdfsdk/fpdf_attachment.cpp
+++ b/fpdfsdk/fpdf_attachment.cpp
@@ -40,13 +40,19 @@
return ByteString(result.get(), size);
}
+// TODO(tsepez): should be UNSAFE_BUFFER_USAGE.
ByteString GenerateMD5Base16(const void* contents, const unsigned long len) {
uint8_t digest[16];
- CRYPT_MD5Generate({static_cast<const uint8_t*>(contents), len}, digest);
- char buf[32];
- for (int i = 0; i < 16; ++i)
- FXSYS_IntToTwoHexChars(digest[i], &buf[i * 2]);
+ // SAFETY: caller ensures `contents` points to at least `len` bytes.
+ CRYPT_MD5Generate(UNSAFE_BUFFERS(pdfium::make_span(
+ static_cast<const uint8_t*>(contents), len)),
+ digest);
+
+ char buf[32];
+ for (int i = 0; i < 16; ++i) {
+ FXSYS_IntToTwoHexChars(digest[i], &buf[i * 2]);
+ }
return ByteString(buf, 32);
}
@@ -274,8 +280,10 @@
if (!pFileStream)
return false;
+ // SAFETY: required from caller.
*out_buflen = DecodeStreamMaybeCopyAndReturnLength(
std::move(pFileStream),
- {static_cast<uint8_t*>(buffer), static_cast<size_t>(buflen)});
+ UNSAFE_BUFFERS(pdfium::make_span(static_cast<uint8_t*>(buffer),
+ static_cast<size_t>(buflen))));
return true;
}
diff --git a/fpdfsdk/fpdf_editimg.cpp b/fpdfsdk/fpdf_editimg.cpp
index 286aa10..5d86554 100644
--- a/fpdfsdk/fpdf_editimg.cpp
+++ b/fpdfsdk/fpdf_editimg.cpp
@@ -342,9 +342,11 @@
if (!pImgStream)
return 0;
+ // SAFETY: caller ensures `buffer` points to at least `buflen` bytes.
return DecodeStreamMaybeCopyAndReturnLength(
std::move(pImgStream),
- {static_cast<uint8_t*>(buffer), static_cast<size_t>(buflen)});
+ UNSAFE_BUFFERS(pdfium::make_span(static_cast<uint8_t*>(buffer),
+ static_cast<size_t>(buflen))));
}
FPDF_EXPORT unsigned long FPDF_CALLCONV
@@ -363,9 +365,11 @@
if (!pImgStream)
return 0;
+ // SAFETY: caller ensures `buffer` points to at least `buflen` bytes.
return GetRawStreamMaybeCopyAndReturnLength(
std::move(pImgStream),
- {static_cast<uint8_t*>(buffer), static_cast<size_t>(buflen)});
+ UNSAFE_BUFFERS(pdfium::make_span(static_cast<uint8_t*>(buffer),
+ static_cast<size_t>(buflen))));
}
FPDF_EXPORT int FPDF_CALLCONV
diff --git a/fpdfsdk/fpdf_editpage.cpp b/fpdfsdk/fpdf_editpage.cpp
index ffc6a11..b4c0868 100644
--- a/fpdfsdk/fpdf_editpage.cpp
+++ b/fpdfsdk/fpdf_editpage.cpp
@@ -202,7 +202,11 @@
return false;
}
- return doc->MovePages({page_indices, page_indices_len}, dest_page_index);
+ // SAFETY: caller ensures `page_indices` points to at least
+ // `page_indices_len` ints.
+ return doc->MovePages(
+ UNSAFE_BUFFERS(pdfium::make_span(page_indices, page_indices_len)),
+ dest_page_index);
}
FPDF_EXPORT FPDF_PAGE FPDF_CALLCONV FPDFPage_New(FPDF_DOCUMENT document,
diff --git a/fpdfsdk/fpdf_edittext.cpp b/fpdfsdk/fpdf_edittext.cpp
index 25c3b04..62a934c 100644
--- a/fpdfsdk/fpdf_edittext.cpp
+++ b/fpdfsdk/fpdf_edittext.cpp
@@ -683,10 +683,14 @@
return nullptr;
}
- // Caller takes ownership.
+ // Caller takes ownership of result.
+ // SAFETY: caller ensures `cid_to_gid_map_data` points to at least
+ // `cid_to_gid_map_data_size` entries.
return FPDFFontFromCPDFFont(
- LoadCustomCompositeFont(doc, std::move(font), font_span, to_unicode_cmap,
- {cid_to_gid_map_data, cid_to_gid_map_data_size})
+ LoadCustomCompositeFont(
+ doc, std::move(font), font_span, to_unicode_cmap,
+ UNSAFE_BUFFERS(
+ pdfium::make_span(cid_to_gid_map_data, cid_to_gid_map_data_size)))
.Leak());
}
diff --git a/fpdfsdk/fpdf_sysfontinfo.cpp b/fpdfsdk/fpdf_sysfontinfo.cpp
index 57fdbc0..453d4be 100644
--- a/fpdfsdk/fpdf_sysfontinfo.cpp
+++ b/fpdfsdk/fpdf_sysfontinfo.cpp
@@ -10,8 +10,10 @@
#include <memory>
+#include "core/fxcrt/compiler_specific.h"
#include "core/fxcrt/fx_codepage.h"
#include "core/fxcrt/numerics/safe_conversions.h"
+#include "core/fxcrt/span.h"
#include "core/fxcrt/stl_util.h"
#include "core/fxcrt/unowned_ptr.h"
#include "core/fxge/cfx_font.h"
@@ -209,16 +211,19 @@
return pDefault->m_pFontInfo->GetFont(family);
}
+// TODO(tsepez): should be UNSAFE_BUFFER_USAGE.
static unsigned long DefaultGetFontData(struct _FPDF_SYSFONTINFO* pThis,
void* hFont,
unsigned int table,
unsigned char* buffer,
unsigned long buf_size) {
auto* pDefault = static_cast<FPDF_SYSFONTINFO_DEFAULT*>(pThis);
- return pdfium::checked_cast<unsigned long>(
- pDefault->m_pFontInfo->GetFontData(hFont, table, {buffer, buf_size}));
+ // SAFETY: required from caller.
+ return pdfium::checked_cast<unsigned long>(pDefault->m_pFontInfo->GetFontData(
+ hFont, table, UNSAFE_BUFFERS(pdfium::make_span(buffer, buf_size))));
}
+// TODO(tsepez): should be UNSAFE_BUFFER_USAGE.
static unsigned long DefaultGetFaceName(struct _FPDF_SYSFONTINFO* pThis,
void* hFont,
char* buffer,
diff --git a/fpdfsdk/fpdf_thumbnail.cpp b/fpdfsdk/fpdf_thumbnail.cpp
index 5b0d60b..2bd886d 100644
--- a/fpdfsdk/fpdf_thumbnail.cpp
+++ b/fpdfsdk/fpdf_thumbnail.cpp
@@ -11,6 +11,8 @@
#include "core/fpdfapi/parser/cpdf_dictionary.h"
#include "core/fpdfapi/parser/cpdf_stream.h"
#include "core/fpdfapi/parser/cpdf_stream_acc.h"
+#include "core/fxcrt/compiler_specific.h"
+#include "core/fxcrt/span.h"
#include "core/fxge/dib/cfx_dibitmap.h"
#include "fpdfsdk/cpdfsdk_helpers.h"
#include "public/fpdfview.h"
@@ -40,9 +42,11 @@
if (!thumb_stream)
return 0u;
+ // SAFETY: caller ensures `buffer` points to at least `buflen` bytes.
return DecodeStreamMaybeCopyAndReturnLength(
std::move(thumb_stream),
- {static_cast<uint8_t*>(buffer), static_cast<size_t>(buflen)});
+ UNSAFE_BUFFERS(pdfium::make_span(static_cast<uint8_t*>(buffer),
+ static_cast<size_t>(buflen))));
}
FPDF_EXPORT unsigned long FPDF_CALLCONV
@@ -54,9 +58,11 @@
if (!thumb_stream)
return 0u;
+ // SAFETY: caller ensures `buffer` points to at least `buflen` bytes.
return GetRawStreamMaybeCopyAndReturnLength(
std::move(thumb_stream),
- {static_cast<uint8_t*>(buffer), static_cast<size_t>(buflen)});
+ UNSAFE_BUFFERS(pdfium::make_span(static_cast<uint8_t*>(buffer),
+ static_cast<size_t>(buflen))));
}
FPDF_EXPORT FPDF_BITMAP FPDF_CALLCONV
diff --git a/fpdfsdk/fpdf_view.cpp b/fpdfsdk/fpdf_view.cpp
index 98017ae..195c368 100644
--- a/fpdfsdk/fpdf_view.cpp
+++ b/fpdfsdk/fpdf_view.cpp
@@ -1319,9 +1319,11 @@
if (static_cast<size_t>(index) >= xfa_packets.size())
return false;
+ // SAFETY: caller ensures `buffer` points to at least `buflen` bytes.
*out_buflen = DecodeStreamMaybeCopyAndReturnLength(
xfa_packets[index].data,
- {static_cast<uint8_t*>(buffer), static_cast<size_t>(buflen)});
+ UNSAFE_BUFFERS(pdfium::make_span(static_cast<uint8_t*>(buffer),
+ static_cast<size_t>(buflen))));
return true;
}
