commit 6967ba99bcf4671083206ab1e8881014a844f3fb
author Tom Sepez <tsepez@chromium.org> Fri May 10 00:40:04 2024 +0000
committer Pdfium LUCI CQ <pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri May 10 00:40:04 2024 +0000
tree 6823a39d35f0e0e2c6991519ce9fb91403d48e5c
parent a529e930cccdd986419d1ca3284615a635da1883

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>

fpdfsdk/fpdf_attachment.cpp

diff --git a/fpdfsdk/fpdf_attachment.cpp b/fpdfsdk/fpdf_attachment.cpp
index dfd48a7..6fc9671 100644
--- a/fpdfsdk/fpdf_attachment.cpp
+++ b/fpdfsdk/fpdf_attachment.cpp
@@ -76,7 +76,8 @@
   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 @@
 
   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 @@
   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 @@
     }
   }
   // 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