Avoid crashes in FPDFAnnot_GetOptionCount() / FPDFAnnot_GetOptionLabel()
Both of these APIs can fail a CHECK() if the input annotation is the
wrong type. Add an embedder test to demonstrate this issue. Then fix the
crash by adding CPDF_FormField::HasOptField() and checking its return
value before calling CPDF_FormField::CountOptions().
Bug: 366102255
Change-Id: I11b6bee3b1defdbabeb4d0301161783436153a34
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/124530
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Tom Sepez <tsepez@google.com>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfdoc/cpdf_formfield.cpp b/core/fpdfdoc/cpdf_formfield.cpp
index efed7e4..1e7886d 100644
--- a/core/fpdfdoc/cpdf_formfield.cpp
+++ b/core/fpdfdoc/cpdf_formfield.cpp
@@ -60,18 +60,6 @@
}
}
-bool HasOptField(CPDF_FormField::Type type) {
- switch (type) {
- case CPDF_FormField::kCheckBox:
- case CPDF_FormField::kRadioButton:
- case CPDF_FormField::kComboBox:
- case CPDF_FormField::kListBox:
- return true;
- default:
- return false;
- }
-}
-
} // namespace
// static
@@ -592,14 +580,26 @@
return -1;
}
+bool CPDF_FormField::HasOptField() const {
+ switch (GetType()) {
+ case CPDF_FormField::kCheckBox:
+ case CPDF_FormField::kRadioButton:
+ case CPDF_FormField::kComboBox:
+ case CPDF_FormField::kListBox:
+ return true;
+ default:
+ return false;
+ }
+}
+
int CPDF_FormField::CountOptions() const {
- CHECK(HasOptField(GetType()));
+ CHECK(HasOptField());
RetainPtr<const CPDF_Array> pArray = ToArray(GetFieldAttrInternal("Opt"));
return pArray ? fxcrt::CollectionSize<int>(*pArray) : 0;
}
WideString CPDF_FormField::GetOptionText(int index, int sub_index) const {
- CHECK(HasOptField(GetType()));
+ CHECK(HasOptField());
RetainPtr<const CPDF_Array> pArray = ToArray(GetFieldAttrInternal("Opt"));
if (!pArray)
return WideString();
diff --git a/core/fpdfdoc/cpdf_formfield.h b/core/fpdfdoc/cpdf_formfield.h
index fa7bf33..c43f89e 100644
--- a/core/fpdfdoc/cpdf_formfield.h
+++ b/core/fpdfdoc/cpdf_formfield.h
@@ -117,6 +117,10 @@
bool SetItemSelection(int index, NotificationOption notify);
int GetDefaultSelectedItem() const;
+
+ bool HasOptField() const;
+
+ // These can only be called if HasOptField() returns true.
int CountOptions() const;
WideString GetOptionLabel(int index) const;
WideString GetOptionValue(int index) const;
diff --git a/fpdfsdk/fpdf_annot.cpp b/fpdfsdk/fpdf_annot.cpp
index 5cfbbfa..e42e6db 100644
--- a/fpdfsdk/fpdf_annot.cpp
+++ b/fpdfsdk/fpdf_annot.cpp
@@ -1317,8 +1317,11 @@
FPDF_EXPORT int FPDF_CALLCONV FPDFAnnot_GetOptionCount(FPDF_FORMHANDLE hHandle,
FPDF_ANNOTATION annot) {
- const CPDF_FormField* pFormField = GetFormField(hHandle, annot);
- return pFormField ? pFormField->CountOptions() : -1;
+ const CPDF_FormField* form_field = GetFormField(hHandle, annot);
+ if (!form_field || !form_field->HasOptField()) {
+ return -1;
+ }
+ return form_field->CountOptions();
}
FPDF_EXPORT unsigned long FPDF_CALLCONV
@@ -1327,16 +1330,19 @@
int index,
FPDF_WCHAR* buffer,
unsigned long buflen) {
- if (index < 0)
+ if (index < 0) {
return 0;
+ }
- const CPDF_FormField* pFormField = GetFormField(hHandle, annot);
- if (!pFormField || index >= pFormField->CountOptions())
+ const CPDF_FormField* form_field = GetFormField(hHandle, annot);
+ if (!form_field || !form_field->HasOptField() ||
+ index >= form_field->CountOptions()) {
return 0;
+ }
// SAFETY: required from caller.
return Utf16EncodeMaybeCopyAndReturnLength(
- pFormField->GetOptionLabel(index),
+ form_field->GetOptionLabel(index),
UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
}
diff --git a/fpdfsdk/fpdf_annot_embeddertest.cpp b/fpdfsdk/fpdf_annot_embeddertest.cpp
index ead0815..7a36b97 100644
--- a/fpdfsdk/fpdf_annot_embeddertest.cpp
+++ b/fpdfsdk/fpdf_annot_embeddertest.cpp
@@ -2357,6 +2357,28 @@
}
}
+TEST_F(FPDFAnnotEmbedderTest, GetOptionCountWrongAnnotationType) {
+ // Open a file with annotations and load its first page.
+ ASSERT_TRUE(OpenDocument("multiple_form_types.pdf"));
+ ScopedEmbedderTestPage page = LoadScopedPage(0);
+ ASSERT_TRUE(page);
+
+ // This annotation is a form field, but does not support Options.
+ ScopedFPDFAnnotation annot(FPDFPage_GetAnnot(page.get(), 3));
+ ASSERT_TRUE(annot);
+
+ // Check types.
+ EXPECT_EQ(FPDF_ANNOT_WIDGET, FPDFAnnot_GetSubtype(annot.get()));
+ EXPECT_EQ(FPDF_FORMFIELD_TEXTFIELD,
+ FPDFAnnot_GetFormFieldType(form_handle(), annot.get()));
+
+ // Option APIs do not support this annot type. They gracefully return an error
+ // and do not crash.
+ EXPECT_EQ(-1, FPDFAnnot_GetOptionCount(form_handle(), annot.get()));
+ EXPECT_EQ(
+ 0u, FPDFAnnot_GetOptionLabel(form_handle(), annot.get(), 0, nullptr, 0));
+}
+
TEST_F(FPDFAnnotEmbedderTest, GetOptionLabelCombobox) {
// Open a file with combobox widget annotations and load its first page.
ASSERT_TRUE(OpenDocument("combobox_form.pdf"));
