Narrow scope of UNSAFE_BUFFERS() in fpdfsdk/fpdf*.cpp

Introduce SpanFromFPDFApiArgs() helper to centralize the dubious
API behavior that the length is ignored when a NULL buffer pointer
is passed. Then change two helper functions to accept spans, thus
removing some unsafe usage (in tests) where we make valid spans
to begin with.

-- Mark two other functions UNSAFE_BUFFER_USAGE while at it.
-- re-order some declarations just to avoid comments in middle of
   code blocks.

Change-Id: Id104aa89528e58a161e05ac9207812cd971c9015
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/118990
Reviewed-by: Thomas Sepez <tsepez@google.com>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>

15 files changed, 189 insertions, 182 deletions

diff --git a/fpdfsdk/cpdfsdk_helpers.cpp b/fpdfsdk/cpdfsdk_helpers.cpp
index a6b2337d0..383d7d1d9 100644
--- a/fpdfsdk/cpdfsdk_helpers.cpp
+++ b/fpdfsdk/cpdfsdk_helpers.cpp
@@ -223,17 +223,28 @@ CPDFSDK_InteractiveForm* FormHandleToInteractiveForm(FPDF_FORMHANDLE hHandle) {
 }
 
 ByteString ByteStringFromFPDFWideString(FPDF_WIDESTRING wide_string) {
-  return WideStringFromFPDFWideString(wide_string).ToUTF8();
+  // SAFETY: caller ensures `wide_string` is NUL-terminated and enforced
+  // by UNSAFE_BUFFER_USAGE in header file.
+  return UNSAFE_BUFFERS(WideStringFromFPDFWideString(wide_string).ToUTF8());
 }
 
-// TOOO(tsepez): should be UNSAFE_BUFFER_USAGE.
 WideString WideStringFromFPDFWideString(FPDF_WIDESTRING wide_string) {
-  // SAFETY: caller ensures `wide_string` is NUL-terminated.
+  // SAFETY: caller ensures `wide_string` is NUL-terminated and enforced
+  // by UNSAFE_BUFFER_USAGE in header file.
   return WideString::FromUTF16LE(UNSAFE_BUFFERS(
       pdfium::make_span(reinterpret_cast<const uint8_t*>(wide_string),
                         FPDFWideStringLength(wide_string) * 2)));
 }
 
+pdfium::span<char> SpanFromFPDFApiArgs(void* buffer, unsigned long buflen) {
+  if (!buffer) {
+    // API convention is to ignore `buflen` arg when `buffer` is NULL.
+    return pdfium::span<char>();
+  }
+  // SAFETY: required from caller, enforced by UNSAFE_BUFFER_USAGE in header.
+  return UNSAFE_BUFFERS(pdfium::make_span(static_cast<char*>(buffer), buflen));
+}
+
 #ifdef PDF_ENABLE_XFA
 RetainPtr<IFX_SeekableStream> MakeSeekableStream(
     FPDF_FILEHANDLER* pFilehandler) {
@@ -301,32 +312,20 @@ FS_MATRIX FSMatrixFromCFXMatrix(const CFX_Matrix& matrix) {
   return {matrix.a, matrix.b, matrix.c, matrix.d, matrix.e, matrix.f};
 }
 
-unsigned long NulTerminateMaybeCopyAndReturnLength(const ByteString& text,
-                                                   void* buffer,
-                                                   unsigned long buflen) {
+unsigned long NulTerminateMaybeCopyAndReturnLength(
+    const ByteString& text,
+    pdfium::span<char> result_span) {
   pdfium::span<const char> text_span = text.span_with_terminator();
-  if (buffer) {
-    // SAFETY: required from caller, enforced by UNSAFE_BUFFER_USAGE on
-    // declaration in header file.
-    pdfium::span<char> result_span =
-        UNSAFE_BUFFERS(pdfium::make_span(static_cast<char*>(buffer), buflen));
-    fxcrt::try_spancpy(result_span, text_span);
-  }
+  fxcrt::try_spancpy(result_span, text_span);
   return pdfium::checked_cast<unsigned long>(text_span.size());
 }
 
-unsigned long Utf16EncodeMaybeCopyAndReturnLength(const WideString& text,
-                                                  void* buffer,
-                                                  unsigned long buflen) {
+unsigned long Utf16EncodeMaybeCopyAndReturnLength(
+    const WideString& text,
+    pdfium::span<char> result_span) {
   ByteString encoded_text = text.ToUTF16LE();
   pdfium::span<const char> encoded_text_span = encoded_text.span();
-  if (buffer) {
-    // SAFETY: required from caller, enforced by UNSAFE_BUFFER_USAGE on
-    // declaration in header file.
-    pdfium::span<char> result_span =
-        UNSAFE_BUFFERS(pdfium::make_span(static_cast<char*>(buffer), buflen));
-    fxcrt::try_spancpy(result_span, encoded_text_span);
-  }
+  fxcrt::try_spancpy(result_span, encoded_text_span);
   return pdfium::checked_cast<unsigned long>(encoded_text_span.size());
 }
 
diff --git a/fpdfsdk/cpdfsdk_helpers.h b/fpdfsdk/cpdfsdk_helpers.h
index 93e6f79ae..117aeeb92 100644
--- a/fpdfsdk/cpdfsdk_helpers.h
+++ b/fpdfsdk/cpdfsdk_helpers.h
@@ -14,6 +14,7 @@
 #include "core/fpdfapi/parser/cpdf_parser.h"
 #include "core/fxcrt/compiler_specific.h"
 #include "core/fxcrt/retain_ptr.h"
+#include "core/fxcrt/span.h"
 #include "core/fxge/cfx_path.h"
 #include "public/fpdf_doc.h"
 #include "public/fpdf_ext.h"
@@ -239,8 +240,15 @@ inline XObjectContext* XObjectContextFromFPDFXObject(FPDF_XOBJECT xobject) {
 
 CPDFSDK_InteractiveForm* FormHandleToInteractiveForm(FPDF_FORMHANDLE hHandle);
 
-ByteString ByteStringFromFPDFWideString(FPDF_WIDESTRING wide_string);
-WideString WideStringFromFPDFWideString(FPDF_WIDESTRING wide_string);
+UNSAFE_BUFFER_USAGE ByteString
+ByteStringFromFPDFWideString(FPDF_WIDESTRING wide_string);
+
+UNSAFE_BUFFER_USAGE WideString
+WideStringFromFPDFWideString(FPDF_WIDESTRING wide_string);
+
+UNSAFE_BUFFER_USAGE pdfium::span<char> SpanFromFPDFApiArgs(
+    void* buffer,
+    unsigned long buflen);
 
 #ifdef PDF_ENABLE_XFA
 // Layering prevents fxcrt from knowing about FPDF_FILEHANDLER, so this can't
@@ -267,15 +275,13 @@ FS_RECTF FSRectFFromCFXFloatRect(const CFX_FloatRect& rect);
 CFX_Matrix CFXMatrixFromFSMatrix(const FS_MATRIX& matrix);
 FS_MATRIX FSMatrixFromCFXMatrix(const CFX_Matrix& matrix);
 
-UNSAFE_BUFFER_USAGE unsigned long NulTerminateMaybeCopyAndReturnLength(
+unsigned long NulTerminateMaybeCopyAndReturnLength(
     const ByteString& text,
-    void* buffer,
-    unsigned long buflen);
+    pdfium::span<char> result_span);
 
-UNSAFE_BUFFER_USAGE unsigned long Utf16EncodeMaybeCopyAndReturnLength(
+unsigned long Utf16EncodeMaybeCopyAndReturnLength(
     const WideString& text,
-    void* buffer,
-    unsigned long buflen);
+    pdfium::span<char> result_span);
 
 // Returns the length of the raw stream data from |stream|. The raw data is the
 // stream's data as stored in the PDF without applying any filters. If |buffer|
diff --git a/fpdfsdk/cpdfsdk_helpers_unittest.cpp b/fpdfsdk/cpdfsdk_helpers_unittest.cpp
index 492e3bc5e..db5c85af3 100644
--- a/fpdfsdk/cpdfsdk_helpers_unittest.cpp
+++ b/fpdfsdk/cpdfsdk_helpers_unittest.cpp
@@ -17,36 +17,32 @@ TEST(CPDFSDK_HelpersTest, NulTerminateMaybeCopyAndReturnLength) {
     const ByteString to_be_copied("toBeCopied");
     constexpr size_t kExpectedToBeCopiedLen = 10;
     ASSERT_EQ(kExpectedToBeCopiedLen, to_be_copied.GetLength());
-
-    // SAFETY: nullptr argument.
     EXPECT_EQ(kExpectedToBeCopiedLen + 1,
-              UNSAFE_BUFFERS(NulTerminateMaybeCopyAndReturnLength(to_be_copied,
-                                                                  nullptr, 0)));
+              NulTerminateMaybeCopyAndReturnLength(to_be_copied,
+                                                   pdfium::span<char>()));
 
     // Buffer should not change if declared length is too short.
     char buf[kExpectedToBeCopiedLen + 1];
-    // TODO(tsepez): convert to span.
-    UNSAFE_BUFFERS(FXSYS_memset(buf, 0x42, kExpectedToBeCopiedLen + 1));
+    fxcrt::spanset(pdfium::make_span(buf), 0x42);
     ASSERT_EQ(kExpectedToBeCopiedLen + 1,
-              UNSAFE_BUFFERS(NulTerminateMaybeCopyAndReturnLength(
-                  to_be_copied, buf, kExpectedToBeCopiedLen)));
+              NulTerminateMaybeCopyAndReturnLength(
+                  to_be_copied, UNSAFE_BUFFERS(pdfium::make_span(
+                                    buf, kExpectedToBeCopiedLen))));
     for (char c : buf)
       EXPECT_EQ(0x42, c);
 
     // Buffer should copy over if long enough.
-    // TODO(tsepez): convert to span.
     ASSERT_EQ(kExpectedToBeCopiedLen + 1,
-              UNSAFE_BUFFERS(NulTerminateMaybeCopyAndReturnLength(
-                  to_be_copied, buf, kExpectedToBeCopiedLen + 1)));
+              NulTerminateMaybeCopyAndReturnLength(to_be_copied,
+                                                   pdfium::make_span(buf)));
     EXPECT_EQ(to_be_copied, ByteString(buf));
   }
   {
     // Empty ByteString should still copy NUL terminator.
     const ByteString empty;
     char buf[1];
-    // TODO(tsepez): convert to span.
-    ASSERT_EQ(1u, UNSAFE_BUFFERS(
-                      NulTerminateMaybeCopyAndReturnLength(empty, buf, 1)));
+    ASSERT_EQ(1u, NulTerminateMaybeCopyAndReturnLength(empty,
+                                                       pdfium::make_span(buf)));
     EXPECT_EQ(empty, ByteString(buf));
   }
 }
diff --git a/fpdfsdk/fpdf_annot.cpp b/fpdfsdk/fpdf_annot.cpp
index c5ff6dff0..14e708d5a 100644
--- a/fpdfsdk/fpdf_annot.cpp
+++ b/fpdfsdk/fpdf_annot.cpp
@@ -1012,8 +1012,9 @@ FPDFAnnot_SetStringValue(FPDF_ANNOTATION annot,
   if (!pAnnotDict)
     return false;
 
+  // SAFETY: required from caller.
   pAnnotDict->SetNewFor<CPDF_String>(
-      key, WideStringFromFPDFWideString(value).AsStringView());
+      key, UNSAFE_BUFFERS(WideStringFromFPDFWideString(value).AsStringView()));
   return true;
 }
 
@@ -1027,8 +1028,9 @@ FPDFAnnot_GetStringValue(FPDF_ANNOTATION annot,
     return 0;
   }
   // SAFETY: required from caller.
-  return UNSAFE_BUFFERS(Utf16EncodeMaybeCopyAndReturnLength(
-      pAnnotDict->GetUnicodeTextFor(key), buffer, buflen));
+  return Utf16EncodeMaybeCopyAndReturnLength(
+      pAnnotDict->GetUnicodeTextFor(key),
+      UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
 }
 
 FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV
@@ -1114,10 +1116,10 @@ FPDFAnnot_SetAP(FPDF_ANNOTATION annot,
     stream_dict->SetFor("Resources", SetExtGStateInResourceDict(
                                          pDoc, pAnnotDict.Get(), "Normal"));
   }
-
+  // SAFETY: required from caller.
+  ByteString new_stream_data = PDF_EncodeText(
+      UNSAFE_BUFFERS(WideStringFromFPDFWideString(value).AsStringView()));
   auto new_stream = pDoc->NewIndirect<CPDF_Stream>(std::move(stream_dict));
-  ByteString new_stream_data =
-      PDF_EncodeText(WideStringFromFPDFWideString(value).AsStringView());
   new_stream->SetData(new_stream_data.unsigned_span());
 
   // Storing reference to indirect object in annotation's AP
@@ -1147,8 +1149,9 @@ FPDFAnnot_GetAP(FPDF_ANNOTATION annot,
 
   RetainPtr<CPDF_Stream> pStream = GetAnnotAPNoFallback(pAnnotDict.Get(), mode);
   // SAFETY: required from caller.
-  return UNSAFE_BUFFERS(Utf16EncodeMaybeCopyAndReturnLength(
-      pStream ? pStream->GetUnicodeText() : WideString(), buffer, buflen));
+  return Utf16EncodeMaybeCopyAndReturnLength(
+      pStream ? pStream->GetUnicodeText() : WideString(),
+      UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
 }
 
 FPDF_EXPORT FPDF_ANNOTATION FPDF_CALLCONV
@@ -1226,8 +1229,9 @@ FPDFAnnot_GetFormFieldName(FPDF_FORMHANDLE hHandle,
     return 0;
   }
   // SAFETY: required from caller.
-  return UNSAFE_BUFFERS(Utf16EncodeMaybeCopyAndReturnLength(
-      pFormField->GetFullName(), buffer, buflen));
+  return Utf16EncodeMaybeCopyAndReturnLength(
+      pFormField->GetFullName(),
+      UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
 }
 
 FPDF_EXPORT int FPDF_CALLCONV
@@ -1255,8 +1259,9 @@ FPDFAnnot_GetFormAdditionalActionJavaScript(FPDF_FORMHANDLE hHandle,
   CPDF_AAction additional_action = pFormField->GetAdditionalAction();
   CPDF_Action action = additional_action.GetAction(type);
   // SAFETY: required from caller.
-  return UNSAFE_BUFFERS(Utf16EncodeMaybeCopyAndReturnLength(
-      action.GetJavaScript(), buffer, buflen));
+  return Utf16EncodeMaybeCopyAndReturnLength(
+      action.GetJavaScript(),
+      UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
 }
 
 FPDF_EXPORT unsigned long FPDF_CALLCONV
@@ -1269,8 +1274,9 @@ FPDFAnnot_GetFormFieldAlternateName(FPDF_FORMHANDLE hHandle,
     return 0;
   }
   // SAFETY: required from caller.
-  return UNSAFE_BUFFERS(Utf16EncodeMaybeCopyAndReturnLength(
-      pFormField->GetAlternateName(), buffer, buflen));
+  return Utf16EncodeMaybeCopyAndReturnLength(
+      pFormField->GetAlternateName(),
+      UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
 }
 
 FPDF_EXPORT unsigned long FPDF_CALLCONV
@@ -1283,8 +1289,9 @@ FPDFAnnot_GetFormFieldValue(FPDF_FORMHANDLE hHandle,
     return 0;
   }
   // SAFETY: required from caller.
-  return UNSAFE_BUFFERS(Utf16EncodeMaybeCopyAndReturnLength(
-      pFormField->GetValue(), buffer, buflen));
+  return Utf16EncodeMaybeCopyAndReturnLength(
+      pFormField->GetValue(),
+      UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
 }
 
 FPDF_EXPORT int FPDF_CALLCONV FPDFAnnot_GetOptionCount(FPDF_FORMHANDLE hHandle,
@@ -1306,10 +1313,10 @@ FPDFAnnot_GetOptionLabel(FPDF_FORMHANDLE hHandle,
   if (!pFormField || index >= pFormField->CountOptions())
     return 0;
 
-  WideString ws = pFormField->GetOptionLabel(index);
   // SAFETY: required from caller.
-  return UNSAFE_BUFFERS(
-      Utf16EncodeMaybeCopyAndReturnLength(ws, buffer, buflen));
+  return Utf16EncodeMaybeCopyAndReturnLength(
+      pFormField->GetOptionLabel(index),
+      UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
 }
 
 FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV
@@ -1474,8 +1481,9 @@ FPDFAnnot_GetFormFieldExportValue(FPDF_FORMHANDLE hHandle,
     return 0;
   }
   // SAFETY: required from caller.
-  return UNSAFE_BUFFERS(Utf16EncodeMaybeCopyAndReturnLength(
-      pWidget->GetExportValue(), buffer, buflen));
+  return Utf16EncodeMaybeCopyAndReturnLength(
+      pWidget->GetExportValue(),
+      UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
 }
 
 FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FPDFAnnot_SetURI(FPDF_ANNOTATION annot,
@@ -1524,7 +1532,8 @@ FPDFAnnot_AddFileAttachment(FPDF_ANNOTATION annot, FPDF_WIDESTRING name) {
     return nullptr;
   }
 
-  WideString ws_name = WideStringFromFPDFWideString(name);
+  // SAFETY: required from caller.
+  WideString ws_name = UNSAFE_BUFFERS(WideStringFromFPDFWideString(name));
   if (ws_name.IsEmpty()) {
     return nullptr;
   }
diff --git a/fpdfsdk/fpdf_attachment.cpp b/fpdfsdk/fpdf_attachment.cpp
index dfd48a7dd..6fc9671f7 100644
--- a/fpdfsdk/fpdf_attachment.cpp
+++ b/fpdfsdk/fpdf_attachment.cpp
@@ -76,7 +76,8 @@ FPDFDoc_AddAttachment(FPDF_DOCUMENT document, FPDF_WIDESTRING name) {
   if (!pDoc)
     return nullptr;
 
-  WideString wsName = WideStringFromFPDFWideString(name);
+  // SAFETY: required from caller.
+  WideString wsName = UNSAFE_BUFFERS(WideStringFromFPDFWideString(name));
   if (wsName.IsEmpty())
     return nullptr;
 
@@ -139,8 +140,8 @@ FPDFAttachment_GetName(FPDF_ATTACHMENT attachment,
 
   CPDF_FileSpec spec(pdfium::WrapRetain(pFile));
   // SAFETY: required from caller.
-  return UNSAFE_BUFFERS(
-      Utf16EncodeMaybeCopyAndReturnLength(spec.GetFileName(), buffer, buflen));
+  return Utf16EncodeMaybeCopyAndReturnLength(
+      spec.GetFileName(), UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
 }
 
 FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV
@@ -178,12 +179,13 @@ FPDFAttachment_SetStringValue(FPDF_ATTACHMENT attachment,
   if (!pParamsDict)
     return false;
 
+  // SAFETY: required from caller.
+  ByteString bsValue = UNSAFE_BUFFERS(ByteStringFromFPDFWideString(value));
   ByteString bsKey = key;
-  ByteString bsValue = ByteStringFromFPDFWideString(value);
   bool bEncodedAsHex = bsKey == kChecksumKey;
-  if (bEncodedAsHex)
+  if (bEncodedAsHex) {
     bsValue = CFXByteStringHexDecode(bsValue);
-
+  }
   pParamsDict->SetNewFor<CPDF_String>(bsKey, bsValue, bEncodedAsHex);
   return true;
 }
@@ -215,8 +217,8 @@ FPDFAttachment_GetStringValue(FPDF_ATTACHMENT attachment,
     }
   }
   // SAFETY: required from caller.
-  return UNSAFE_BUFFERS(
-      Utf16EncodeMaybeCopyAndReturnLength(value, buffer, buflen));
+  return Utf16EncodeMaybeCopyAndReturnLength(
+      value, UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
 }
 
 FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV
diff --git a/fpdfsdk/fpdf_doc.cpp b/fpdfsdk/fpdf_doc.cpp
index 1dae6cb23..570fa31ae 100644
--- a/fpdfsdk/fpdf_doc.cpp
+++ b/fpdfsdk/fpdf_doc.cpp
@@ -116,8 +116,8 @@ FPDFBookmark_GetTitle(FPDF_BOOKMARK bookmark,
       pdfium::WrapRetain(CPDFDictionaryFromFPDFBookmark(bookmark)));
   WideString title = cBookmark.GetTitle();
   // SAFETY: required from caller.
-  return UNSAFE_BUFFERS(
-      Utf16EncodeMaybeCopyAndReturnLength(title, buffer, buflen));
+  return Utf16EncodeMaybeCopyAndReturnLength(
+      title, UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
 }
 
 FPDF_EXPORT int FPDF_CALLCONV FPDFBookmark_GetCount(FPDF_BOOKMARK bookmark) {
@@ -134,7 +134,8 @@ FPDFBookmark_Find(FPDF_DOCUMENT document, FPDF_WIDESTRING title) {
   if (!pDoc)
     return nullptr;
 
-  WideString encodedTitle = WideStringFromFPDFWideString(title);
+  // SAFETY: required from caller.
+  WideString encodedTitle = UNSAFE_BUFFERS(WideStringFromFPDFWideString(title));
   if (encodedTitle.IsEmpty())
     return nullptr;
 
@@ -222,8 +223,8 @@ FPDFAction_GetFilePath(FPDF_ACTION action, void* buffer, unsigned long buflen) {
   CPDF_Action cAction(pdfium::WrapRetain(CPDFDictionaryFromFPDFAction(action)));
   ByteString path = cAction.GetFilePath().ToUTF8();
   // SAFETY: required from caller.
-  return UNSAFE_BUFFERS(
-      NulTerminateMaybeCopyAndReturnLength(path, buffer, buflen));
+  return NulTerminateMaybeCopyAndReturnLength(
+      path, UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
 }
 
 FPDF_EXPORT unsigned long FPDF_CALLCONV
@@ -239,14 +240,11 @@ FPDFAction_GetURIPath(FPDF_DOCUMENT document,
   if (type != PDFACTION_URI) {
     return 0;
   }
+  // SAFETY: required from caller.
+  auto result_span = UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen));
   CPDF_Action cAction(pdfium::WrapRetain(CPDFDictionaryFromFPDFAction(action)));
   ByteString path = cAction.GetURI(pDoc);
-  if (buffer) {
-    // SAFETY: required from caller.
-    pdfium::span<char> result_span =
-        UNSAFE_BUFFERS(pdfium::make_span(static_cast<char*>(buffer), buflen));
-    fxcrt::try_spancpy(result_span, path.span_with_terminator());
-  }
+  fxcrt::try_spancpy(result_span, path.span_with_terminator());
   return static_cast<unsigned long>(path.span_with_terminator().size());
 }
 
@@ -490,8 +488,8 @@ FPDF_GetFileIdentifier(FPDF_DOCUMENT document,
     return 0;
 
   // SAFETY: required from caller.
-  return UNSAFE_BUFFERS(NulTerminateMaybeCopyAndReturnLength(
-      pValue->GetString(), buffer, buflen));
+  return NulTerminateMaybeCopyAndReturnLength(
+      pValue->GetString(), UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
 }
 
 FPDF_EXPORT unsigned long FPDF_CALLCONV FPDF_GetMetaText(FPDF_DOCUMENT document,
@@ -508,11 +506,10 @@ FPDF_EXPORT unsigned long FPDF_CALLCONV FPDF_GetMetaText(FPDF_DOCUMENT document,
   if (!pInfo)
     return 0;
 
-  WideString text = pInfo->GetUnicodeTextFor(tag);
-
   // SAFETY: required from caller.
-  return UNSAFE_BUFFERS(
-      Utf16EncodeMaybeCopyAndReturnLength(text, buffer, buflen));
+  return Utf16EncodeMaybeCopyAndReturnLength(
+      pInfo->GetUnicodeTextFor(tag),
+      UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
 }
 
 FPDF_EXPORT unsigned long FPDF_CALLCONV
@@ -530,6 +527,6 @@ FPDF_GetPageLabel(FPDF_DOCUMENT document,
     return 0;
   }
   // SAFETY: required from caller.
-  return UNSAFE_BUFFERS(
-      Utf16EncodeMaybeCopyAndReturnLength(str.value(), buffer, buflen));
+  return Utf16EncodeMaybeCopyAndReturnLength(
+      str.value(), UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
 }
diff --git a/fpdfsdk/fpdf_editimg.cpp b/fpdfsdk/fpdf_editimg.cpp
index 7ca3ac5bb..ab757576a 100644
--- a/fpdfsdk/fpdf_editimg.cpp
+++ b/fpdfsdk/fpdf_editimg.cpp
@@ -421,8 +421,8 @@ FPDFImageObj_GetImageFilter(FPDF_PAGEOBJECT image_object,
                             : pFilter->AsArray()->GetByteStringAt(index);
 
   // SAFETY: required from caller.
-  return UNSAFE_BUFFERS(
-      NulTerminateMaybeCopyAndReturnLength(bsFilter, buffer, buflen));
+  return NulTerminateMaybeCopyAndReturnLength(
+      bsFilter, UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
 }
 
 FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV
diff --git a/fpdfsdk/fpdf_editpage.cpp b/fpdfsdk/fpdf_editpage.cpp
index 72139131f..7fa4fe15b 100644
--- a/fpdfsdk/fpdf_editpage.cpp
+++ b/fpdfsdk/fpdf_editpage.cpp
@@ -368,9 +368,9 @@ FPDFPageObjMark_GetName(FPDF_PAGEOBJECTMARK mark,
     return false;
   }
   // SAFETY: required from caller.
-  *out_buflen = UNSAFE_BUFFERS(Utf16EncodeMaybeCopyAndReturnLength(
-      WideString::FromUTF8(pMarkItem->GetName().AsStringView()), buffer,
-      buflen));
+  *out_buflen = Utf16EncodeMaybeCopyAndReturnLength(
+      WideString::FromUTF8(pMarkItem->GetName().AsStringView()),
+      UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
   return true;
 }
 
@@ -402,8 +402,9 @@ FPDFPageObjMark_GetParamKey(FPDF_PAGEOBJECTMARK mark,
   for (auto& it : locker) {
     if (index == 0) {
       // SAFETY: required from caller.
-      *out_buflen = UNSAFE_BUFFERS(Utf16EncodeMaybeCopyAndReturnLength(
-          WideString::FromUTF8(it.first.AsStringView()), buffer, buflen));
+      *out_buflen = Utf16EncodeMaybeCopyAndReturnLength(
+          WideString::FromUTF8(it.first.AsStringView()),
+          UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
       return true;
     }
     --index;
@@ -460,8 +461,9 @@ FPDFPageObjMark_GetParamStringValue(FPDF_PAGEOBJECTMARK mark,
     return false;
 
   // SAFETY: required from caller.
-  *out_buflen = UNSAFE_BUFFERS(Utf16EncodeMaybeCopyAndReturnLength(
-      WideString::FromUTF8(pObj->GetString().AsStringView()), buffer, buflen));
+  *out_buflen = Utf16EncodeMaybeCopyAndReturnLength(
+      WideString::FromUTF8(pObj->GetString().AsStringView()),
+      UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
   return true;
 }
 
@@ -482,13 +484,10 @@ FPDFPageObjMark_GetParamBlobValue(FPDF_PAGEOBJECTMARK mark,
   if (!pObj || !pObj->IsString())
     return false;
 
+  // SAFETY: required from caller.
+  auto result_span = UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen));
   ByteString value = pObj->GetString();
-  if (buffer) {
-    // SAFETY: required from caller.
-    pdfium::span<char> result_span =
-        UNSAFE_BUFFERS(pdfium::make_span(static_cast<char*>(buffer), buflen));
-    fxcrt::try_spancpy(result_span, value.span());
-  }
+  fxcrt::try_spancpy(result_span, value.span());
   *out_buflen = pdfium::checked_cast<unsigned long>(value.span().size());
   return true;
 }
diff --git a/fpdfsdk/fpdf_edittext.cpp b/fpdfsdk/fpdf_edittext.cpp
index 04d035a27..f9d16ea36 100644
--- a/fpdfsdk/fpdf_edittext.cpp
+++ b/fpdfsdk/fpdf_edittext.cpp
@@ -590,10 +590,11 @@ FPDFPageObj_NewTextObj(FPDF_DOCUMENT document,
 FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV
 FPDFText_SetText(FPDF_PAGEOBJECT text_object, FPDF_WIDESTRING text) {
   CPDF_TextObject* pTextObj = CPDFTextObjectFromFPDFPageObject(text_object);
-  if (!pTextObj)
+  if (!pTextObj) {
     return false;
-
-  WideString encodedText = WideStringFromFPDFWideString(text);
+  }
+  // SAFETY: required from caller.
+  WideString encodedText = UNSAFE_BUFFERS(WideStringFromFPDFWideString(text));
   ByteString byteText;
   for (wchar_t wc : encodedText) {
     pTextObj->GetFont()->AppendChar(
@@ -723,10 +724,10 @@ FPDFTextObj_GetText(FPDF_PAGEOBJECT text_object,
   if (!pTextPage)
     return 0;
 
-  WideString text = pTextPage->GetTextByObject(pTextObj);
   // SAFETY: required from caller.
-  return UNSAFE_BUFFERS(
-      Utf16EncodeMaybeCopyAndReturnLength(text, buffer, length));
+  return Utf16EncodeMaybeCopyAndReturnLength(
+      pTextPage->GetTextByObject(pTextObj),
+      UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, length)));
 }
 
 FPDF_EXPORT FPDF_BITMAP FPDF_CALLCONV
@@ -855,14 +856,11 @@ FPDFFont_GetFontName(FPDF_FONT font, char* buffer, unsigned long length) {
   if (!pFont)
     return 0;
 
+  // SAFETY: required from caller.
+  auto result_span = UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, length));
   ByteString name = pFont->GetFont()->GetFamilyName();
   pdfium::span<const char> name_span = name.span_with_terminator();
-  if (buffer) {
-    // SAFETY: required from caller.
-    pdfium::span<char> result_span =
-        UNSAFE_BUFFERS(pdfium::make_span(buffer, length));
-    fxcrt::try_spancpy(result_span, name_span);
-  }
+  fxcrt::try_spancpy(result_span, name_span);
   return static_cast<unsigned long>(name_span.size());
 }
 
@@ -874,13 +872,11 @@ FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FPDFFont_GetFontData(FPDF_FONT font,
   if (!cfont || !out_buflen)
     return false;
 
+  // SAFETY: required from caller.
+  auto result_span = UNSAFE_BUFFERS(
+      SpanFromFPDFApiArgs(buffer, pdfium::checked_cast<unsigned long>(buflen)));
   pdfium::span<const uint8_t> data = cfont->GetFont()->GetFontSpan();
-  if (buffer) {
-    // SAFETY: required from caller.
-    pdfium::span<uint8_t> result_span =
-        UNSAFE_BUFFERS(pdfium::make_span(buffer, buflen));
-    fxcrt::try_spancpy(result_span, data);
-  }
+  fxcrt::try_spancpy(result_span, data);
   *out_buflen = data.size();
   return true;
 }
diff --git a/fpdfsdk/fpdf_formfill.cpp b/fpdfsdk/fpdf_formfill.cpp
index 9ae2c779d..83a589149 100644
--- a/fpdfsdk/fpdf_formfill.cpp
+++ b/fpdfsdk/fpdf_formfill.cpp
@@ -523,8 +523,9 @@ FORM_GetFocusedText(FPDF_FORMHANDLE hHandle,
     return 0;
   }
   // SAFETY: required from caller.
-  return UNSAFE_BUFFERS(Utf16EncodeMaybeCopyAndReturnLength(
-      pPageView->GetFocusedFormText(), buffer, buflen));
+  return Utf16EncodeMaybeCopyAndReturnLength(
+      pPageView->GetFocusedFormText(),
+      UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
 }
 
 FPDF_EXPORT unsigned long FPDF_CALLCONV
@@ -537,8 +538,9 @@ FORM_GetSelectedText(FPDF_FORMHANDLE hHandle,
     return 0;
   }
   // SAFETY: required from caller.
-  return UNSAFE_BUFFERS(Utf16EncodeMaybeCopyAndReturnLength(
-      pPageView->GetSelectedText(), buffer, buflen));
+  return Utf16EncodeMaybeCopyAndReturnLength(
+      pPageView->GetSelectedText(),
+      UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
 }
 
 FPDF_EXPORT void FPDF_CALLCONV
@@ -546,20 +548,24 @@ FORM_ReplaceAndKeepSelection(FPDF_FORMHANDLE hHandle,
                              FPDF_PAGE page,
                              FPDF_WIDESTRING wsText) {
   CPDFSDK_PageView* pPageView = FormHandleToPageView(hHandle, page);
-  if (!pPageView)
+  if (!pPageView) {
     return;
-
-  pPageView->ReplaceAndKeepSelection(WideStringFromFPDFWideString(wsText));
+  }
+  // SAFETY: required from caller.
+  pPageView->ReplaceAndKeepSelection(
+      UNSAFE_BUFFERS(WideStringFromFPDFWideString(wsText)));
 }
 
 FPDF_EXPORT void FPDF_CALLCONV FORM_ReplaceSelection(FPDF_FORMHANDLE hHandle,
                                                      FPDF_PAGE page,
                                                      FPDF_WIDESTRING wsText) {
   CPDFSDK_PageView* pPageView = FormHandleToPageView(hHandle, page);
-  if (!pPageView)
+  if (!pPageView) {
     return;
-
-  pPageView->ReplaceSelection(WideStringFromFPDFWideString(wsText));
+  }
+  // SAFETY: required from caller.
+  pPageView->ReplaceSelection(
+      UNSAFE_BUFFERS(WideStringFromFPDFWideString(wsText)));
 }
 
 FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FORM_SelectAllText(FPDF_FORMHANDLE hHandle,
diff --git a/fpdfsdk/fpdf_javascript.cpp b/fpdfsdk/fpdf_javascript.cpp
index 0377197e8..00a66321d 100644
--- a/fpdfsdk/fpdf_javascript.cpp
+++ b/fpdfsdk/fpdf_javascript.cpp
@@ -78,8 +78,8 @@ FPDFJavaScriptAction_GetName(FPDF_JAVASCRIPT_ACTION javascript,
     return 0;
   }
   // SAFETY: required from caller.
-  return UNSAFE_BUFFERS(
-      Utf16EncodeMaybeCopyAndReturnLength(js->name, buffer, buflen));
+  return Utf16EncodeMaybeCopyAndReturnLength(
+      js->name, UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
 }
 
 FPDF_EXPORT unsigned long FPDF_CALLCONV
@@ -92,6 +92,6 @@ FPDFJavaScriptAction_GetScript(FPDF_JAVASCRIPT_ACTION javascript,
     return 0;
   }
   // SAFETY: required from caller.
-  return UNSAFE_BUFFERS(
-      Utf16EncodeMaybeCopyAndReturnLength(js->script, buffer, buflen));
+  return Utf16EncodeMaybeCopyAndReturnLength(
+      js->script, UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
 }
diff --git a/fpdfsdk/fpdf_signature.cpp b/fpdfsdk/fpdf_signature.cpp
index 2bcfa8b93..5061f42b2 100644
--- a/fpdfsdk/fpdf_signature.cpp
+++ b/fpdfsdk/fpdf_signature.cpp
@@ -85,13 +85,10 @@ FPDFSignatureObj_GetContents(FPDF_SIGNATURE signature,
   if (!value_dict) {
     return 0;
   }
+  // SAFETY: required from caller.
+  auto result_span = UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, length));
   ByteString contents = value_dict->GetByteStringFor("Contents");
-  if (buffer) {
-    // SAFETY: required from caller.
-    pdfium::span<char> result_span =
-        UNSAFE_BUFFERS(pdfium::make_span(static_cast<char*>(buffer), length));
-    fxcrt::try_spancpy(result_span, contents.span());
-  }
+  fxcrt::try_spancpy(result_span, contents.span());
   return pdfium::checked_cast<unsigned long>(contents.span().size());
 }
 
@@ -141,8 +138,8 @@ FPDFSignatureObj_GetSubFilter(FPDF_SIGNATURE signature,
   ByteString sub_filter = value_dict->GetNameFor("SubFilter");
 
   // SAFETY: required from caller.
-  return UNSAFE_BUFFERS(
-      NulTerminateMaybeCopyAndReturnLength(sub_filter, buffer, length));
+  return NulTerminateMaybeCopyAndReturnLength(
+      sub_filter, UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, length)));
 }
 
 FPDF_EXPORT unsigned long FPDF_CALLCONV
@@ -164,8 +161,9 @@ FPDFSignatureObj_GetReason(FPDF_SIGNATURE signature,
     return 0;
 
   // SAFETY: required from caller.
-  return UNSAFE_BUFFERS(Utf16EncodeMaybeCopyAndReturnLength(
-      obj->GetUnicodeText(), buffer, length));
+  return Utf16EncodeMaybeCopyAndReturnLength(
+      obj->GetUnicodeText(),
+      UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, length)));
 }
 
 FPDF_EXPORT unsigned long FPDF_CALLCONV
@@ -187,8 +185,8 @@ FPDFSignatureObj_GetTime(FPDF_SIGNATURE signature,
     return 0;
 
   // SAFETY: required from caller.
-  return UNSAFE_BUFFERS(
-      NulTerminateMaybeCopyAndReturnLength(obj->GetString(), buffer, length));
+  return NulTerminateMaybeCopyAndReturnLength(
+      obj->GetString(), UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, length)));
 }
 
 FPDF_EXPORT unsigned int FPDF_CALLCONV
diff --git a/fpdfsdk/fpdf_structtree.cpp b/fpdfsdk/fpdf_structtree.cpp
index 231f4b010..797d328f0 100644
--- a/fpdfsdk/fpdf_structtree.cpp
+++ b/fpdfsdk/fpdf_structtree.cpp
@@ -27,8 +27,8 @@ UNSAFE_BUFFER_USAGE unsigned long WideStringToBuffer(const WideString& str,
     return 0;
   }
   // SAFETY: required from caller and enforced by UNSAFE_BUFFER_USAGE.
-  return UNSAFE_BUFFERS(
-      Utf16EncodeMaybeCopyAndReturnLength(str, buffer, buflen));
+  return Utf16EncodeMaybeCopyAndReturnLength(
+      str, UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
 }
 
 int GetMcidFromDict(const CPDF_Dictionary* dict) {
@@ -122,8 +122,8 @@ FPDF_StructElement_GetID(FPDF_STRUCTELEMENT struct_element,
     return 0;
   }
   // SAFETY: required from caller.
-  return UNSAFE_BUFFERS(
-      Utf16EncodeMaybeCopyAndReturnLength(id.value(), buffer, buflen));
+  return Utf16EncodeMaybeCopyAndReturnLength(
+      id.value(), UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
 }
 
 FPDF_EXPORT unsigned long FPDF_CALLCONV
@@ -140,8 +140,8 @@ FPDF_StructElement_GetLang(FPDF_STRUCTELEMENT struct_element,
     return 0;
   }
   // SAFETY: required from caller.
-  return UNSAFE_BUFFERS(
-      Utf16EncodeMaybeCopyAndReturnLength(lang.value(), buffer, buflen));
+  return Utf16EncodeMaybeCopyAndReturnLength(
+      lang.value(), UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
 }
 
 FPDF_EXPORT int FPDF_CALLCONV
@@ -218,8 +218,9 @@ FPDF_StructElement_GetStringAttribute(FPDF_STRUCTELEMENT struct_element,
       continue;
     }
     // SAFETY: required from caller.
-    return UNSAFE_BUFFERS(Utf16EncodeMaybeCopyAndReturnLength(
-        attr->GetUnicodeText(), buffer, buflen));
+    return Utf16EncodeMaybeCopyAndReturnLength(
+        attr->GetUnicodeText(),
+        UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
   }
   return 0;
 }
@@ -348,8 +349,8 @@ FPDF_StructElement_Attr_GetName(FPDF_STRUCTELEMENT_ATTR struct_attribute,
   for (auto& it : locker) {
     if (index == 0) {
       // SAFETY: required from caller.
-      *out_buflen = UNSAFE_BUFFERS(
-          NulTerminateMaybeCopyAndReturnLength(it.first, buffer, buflen));
+      *out_buflen = NulTerminateMaybeCopyAndReturnLength(
+          it.first, UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
       return true;
     }
     --index;
@@ -428,8 +429,9 @@ FPDF_StructElement_Attr_GetStringValue(FPDF_STRUCTELEMENT_ATTR struct_attribute,
     return false;
 
   // SAFETY: required from caller.
-  *out_buflen = UNSAFE_BUFFERS(Utf16EncodeMaybeCopyAndReturnLength(
-      WideString::FromUTF8(obj->GetString().AsStringView()), buffer, buflen));
+  *out_buflen = Utf16EncodeMaybeCopyAndReturnLength(
+      WideString::FromUTF8(obj->GetString().AsStringView()),
+      UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
   return true;
 }
 
@@ -451,13 +453,10 @@ FPDF_StructElement_Attr_GetBlobValue(FPDF_STRUCTELEMENT_ATTR struct_attribute,
   if (!obj || !obj->IsString())
     return false;
 
+  // SAFETY: required from caller.
+  auto result_span = UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen));
   ByteString blob_value = obj->GetString();
-  if (buffer) {
-    // SAFETY: required from caller.
-    pdfium::span<char> result_span =
-        UNSAFE_BUFFERS(pdfium::make_span(static_cast<char*>(buffer), buflen));
-    fxcrt::try_spancpy(result_span, blob_value.span());
-  }
+  fxcrt::try_spancpy(result_span, blob_value.span());
   *out_buflen = pdfium::checked_cast<unsigned long>(blob_value.span().size());
   return true;
 }
diff --git a/fpdfsdk/fpdf_text.cpp b/fpdfsdk/fpdf_text.cpp
index 524bdf844..87c47163b 100644
--- a/fpdfsdk/fpdf_text.cpp
+++ b/fpdfsdk/fpdf_text.cpp
@@ -132,14 +132,11 @@ FPDFText_GetFontInfo(FPDF_TEXTPAGE text_page,
   if (flags)
     *flags = font->GetFontFlags();
 
+  // SAFETY: required from caller.
+  auto result_span = UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen));
   ByteString basefont = font->GetBaseFontName();
   auto basefont_span = basefont.span_with_terminator();
-  if (buffer) {
-    // SAFETY: required from caller.
-    pdfium::span<char> result_span =
-        UNSAFE_BUFFERS(pdfium::make_span(static_cast<char*>(buffer), buflen));
-    fxcrt::try_spancpy(result_span, basefont_span);
-  }
+  fxcrt::try_spancpy(result_span, basefont_span);
   return pdfium::checked_cast<unsigned long>(basefont_span.size());
 }
 
@@ -424,8 +421,10 @@ FPDFText_FindStart(FPDF_TEXTPAGE text_page,
   options.bMatchCase = !!(flags & FPDF_MATCHCASE);
   options.bMatchWholeWord = !!(flags & FPDF_MATCHWHOLEWORD);
   options.bConsecutive = !!(flags & FPDF_CONSECUTIVE);
+
+  // SAFETY: required from caller.
   auto find = CPDF_TextPageFind::Create(
-      textpage, WideStringFromFPDFWideString(findwhat), options,
+      textpage, UNSAFE_BUFFERS(WideStringFromFPDFWideString(findwhat)), options,
       start_index >= 0 ? std::optional<size_t>(start_index) : std::nullopt);
 
   // Caller takes ownership.
diff --git a/fpdfsdk/fpdf_view.cpp b/fpdfsdk/fpdf_view.cpp
index 7955f2dfe..f6ea680d0 100644
--- a/fpdfsdk/fpdf_view.cpp
+++ b/fpdfsdk/fpdf_view.cpp
@@ -1107,8 +1107,8 @@ FPDF_VIEWERREF_GetName(FPDF_DOCUMENT document,
     return 0;
   }
   // SAFETY: required from caller.
-  return UNSAFE_BUFFERS(
-      NulTerminateMaybeCopyAndReturnLength(bsVal.value(), buffer, length));
+  return NulTerminateMaybeCopyAndReturnLength(
+      bsVal.value(), UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, length)));
 }
 
 FPDF_EXPORT FPDF_DWORD FPDF_CALLCONV
@@ -1314,8 +1314,9 @@ FPDF_GetXFAPacketName(FPDF_DOCUMENT document,
     return 0;
   }
   // TODO(crbug.com/pdfium/2155): resolve safety issues.
-  return UNSAFE_BUFFERS(NulTerminateMaybeCopyAndReturnLength(
-      xfa_packets[index].name, buffer, buflen));
+  return NulTerminateMaybeCopyAndReturnLength(
+      UNSAFE_BUFFERS(xfa_packets[index].name),
+      UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
 }
 
 FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV