diff --git a/fpdfsdk/fpdfxfa/BUILD.gn b/fpdfsdk/fpdfxfa/BUILD.gn index 4374f59050..1118a6b6a6 100644 --- a/fpdfsdk/fpdfxfa/BUILD.gn +++ b/fpdfsdk/fpdfxfa/BUILD.gn @@ -45,12 +45,15 @@ pdfium_embeddertest_source_set("embeddertests") { sources = [ "cpdfxfa_context_embeddertest.cpp", "cpdfxfa_docenvironment_embeddertest.cpp", + "cpdfxfa_page_embeddertest.cpp", ] configs = [ "//v8:external_startup_data" ] deps = [ ":fpdfxfa", "../:fpdfsdk", "../../fxjs", + "../../xfa/fxfa", + "../../xfa/fxfa/parser", ] pdfium_root_dir = "../../" } diff --git a/fpdfsdk/fpdfxfa/cpdfxfa_page_embeddertest.cpp b/fpdfsdk/fpdfxfa/cpdfxfa_page_embeddertest.cpp new file mode 100644 index 0000000000..903d9ca405 --- /dev/null +++ b/fpdfsdk/fpdfxfa/cpdfxfa_page_embeddertest.cpp @@ -0,0 +1,104 @@ +// Copyright 2024 The PDFium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "fpdfsdk/fpdfxfa/cpdfxfa_page.h" + +#include "core/fxcrt/observed_ptr.h" +#include "fpdfsdk/cpdfsdk_annot.h" +#include "fpdfsdk/cpdfsdk_formfillenvironment.h" +#include "fpdfsdk/cpdfsdk_helpers.h" +#include "fpdfsdk/cpdfsdk_pageview.h" +#include "fpdfsdk/fpdfxfa/cpdfxfa_context.h" +#include "fpdfsdk/fpdfxfa/cpdfxfa_widget.h" +#include "fxjs/xfa/cjx_object.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "testing/xfa_js_embedder_test.h" +#include "xfa/fxfa/cxfa_ffdocview.h" +#include "xfa/fxfa/cxfa_fffield.h" +#include "xfa/fxfa/cxfa_ffwidget.h" +#include "xfa/fxfa/parser/cxfa_node.h" + +namespace { + +CPDFXFA_Widget* FindXFAWidgetByName(CPDFSDK_PageView* page_view, + const WideString& name) { + for (CPDFSDK_Annot* annot : page_view->GetAnnotList()) { + CPDFXFA_Widget* xfa = ToXFAWidget(annot); + if (!xfa) { + continue; + } + CXFA_Node* node = xfa->GetXFAFFWidget()->GetNode(); + if (node && node->JSObject()->GetCData(XFA_Attribute::Name) == name) { + return xfa; + } + } + return nullptr; +} + +} // namespace + +class CPDFXFAPageEmbedderTest : public XFAJSEmbedderTest {}; + +// CPDFXFA_Page::DrawFocusAnnot() holds a raw CPDFSDK_Annot* across a +// CXFA_FFPageWidgetIterator loop. The iterator's MoveToNext() may lazily call +// CXFA_FFTextEdit::LoadWidget(), whose initial SetText() dispatches an XFA +// `change` event. That handler can run arbitrary script, including +// removeInstance() + xfa.host.openList(), which synchronously triggers a +// relayout that calls DeleteAnnotForFFWidget() on the focused annot's +// CPDFXFA_Widget — freeing it while DrawFocusAnnot() still holds the raw +// pointer. The trailing ToXFAWidget(pAnnot) is then a virtual call on freed +// memory. +// +// The lazy-load precondition (a widget that has XFA_WidgetStatus::kVisible set +// but for which IsLoaded() returns false at draw time) is reachable but +// requires a specific sequence of layout transitions; this test simulates it +// directly via ClearNormalWidgetForTesting() so the dangerous re-entry path is +// exercised deterministically. The unguarded raw pointer in DrawFocusAnnot() +// is the bug under test (compare the ObservedPtr<> guard already applied in +// the sibling iterator helper in the same file). +TEST_F(CPDFXFAPageEmbedderTest, DrawFocusAnnotUseAfterFree) { + ASSERT_TRUE(OpenDocument("xfa_drawfocus_uaf.pdf")); + ScopedPage page = LoadScopedPage(0); + ASSERT_TRUE(page); + + CPDFSDK_FormFillEnvironment* env = + CPDFSDKFormFillEnvironmentFromFPDFFormHandle(form_handle()); + ASSERT_TRUE(env); + CPDFSDK_PageView* page_view = + env->GetPageView(IPDFPageFromFPDFPage(page.get())); + ASSERT_TRUE(page_view); + + // Focus the victim widget V — DrawFocusAnnot() will be passed its + // CPDFXFA_Widget* as a raw pointer. + CPDFXFA_Widget* v_annot = FindXFAWidgetByName(page_view, L"V"); + ASSERT_TRUE(v_annot); + CXFA_FFWidget* v_widget = v_annot->GetXFAFFWidget(); + { + ObservedPtr obs(v_annot); + ASSERT_TRUE(env->SetFocusAnnot(obs)); + } + ASSERT_EQ(v_annot, page_view->GetFocusAnnot()); + + // Force the trigger widget T back to "not loaded" so the iterator inside + // DrawFocusAnnot() re-runs its LoadWidget() and fires T's `change` script. + CPDFXFA_Widget* t_annot = FindXFAWidgetByName(page_view, L"T"); + ASSERT_TRUE(t_annot); + CXFA_FFField* t_field = ToField(t_annot->GetXFAFFWidget()); + ASSERT_TRUE(t_field); + t_field->ClearNormalWidgetForTesting(); + ASSERT_FALSE(t_field->IsLoaded()); + + // Arm the script (it is gated on this so it does not fire during initial + // page load / LoadFXAnnots()). + EXPECT_TRUE(Execute("xfa.form.form1.flag = \"armed\"")); + + // Reaches CPDFXFA_Page::DrawFocusAnnot() via FPDF_FFLDraw(). On the + // unpatched tree this is a heap-use-after-free in ToXFAWidget(pAnnot). + ScopedFPDFBitmap bitmap = RenderLoadedPage(page.get()); + + // Confirm the script actually ran and the victim's annot is gone, i.e. the + // free side of the UAF was reached during the draw above. + EXPECT_TRUE(Execute("xfa.form.form1.flag")); + EXPECT_FALSE(page_view->GetAnnotForFFWidget(v_widget)); +} diff --git a/testing/resources/xfa_drawfocus_uaf.in b/testing/resources/xfa_drawfocus_uaf.in new file mode 100644 index 0000000000..78b07ebae1 --- /dev/null +++ b/testing/resources/xfa_drawfocus_uaf.in @@ -0,0 +1,55 @@ +{{header}} +{{include xfa_catalog_1_0.fragment}} +{{include xfa_object_2_0.fragment}} +{{include xfa_preamble_3_0.fragment}} +{{include xfa_config_4_0.fragment}} +{{object 5 0}} << + {{streamlen}} +>> +stream + +endstream +endobj +{{include xfa_locale_6_0.fragment}} +{{include xfa_postamble_7_0.fragment}} +{{include xfa_pages_8_0.fragment}} +{{xref}} +{{trailer}} +{{startxref}} +%%EOF diff --git a/testing/resources/xfa_drawfocus_uaf.pdf b/testing/resources/xfa_drawfocus_uaf.pdf new file mode 100644 index 0000000000..1a267e53cb --- /dev/null +++ b/testing/resources/xfa_drawfocus_uaf.pdf @@ -0,0 +1,264 @@ +%PDF-1.7 +% ò¤ô +1 0 obj << + /AcroForm 2 0 R + /Extensions << + /ADBE << + /BaseVersion /1.7 + /ExtensionLevel 8 + >> + >> + /NeedsRendering true + /Pages 8 0 R + /Type /Catalog +>> +endobj +2 0 obj << + /XFA [ + (preamble) + 3 0 R + (config) + 4 0 R + (template) + 5 0 R + (localeSet) + 6 0 R + (postamble) + 7 0 R + ] +>> +endobj +3 0 obj << + /Length 123 +>> +stream + +endstream +endobj +4 0 obj << + /Length 641 +>> +stream + + + pdf + + + + + + + 1.7 + 8 + client + XFA + 1 + + + * + + pdf + + + + + required + + preSubmit + + +endstream +endobj +5 0 obj << + /Length 1315 +>> +stream + +endstream +endobj +6 0 obj << + /Length 3454 +>> +stream + + + + + January + February + March + April + May + June + July + August + September + October + November + December + + + Jan + Feb + Mar + Apr + May + Jun + Jul + Aug + Sep + Oct + Nov + Dec + + + Sunday + Monday + Tuesday + Wednesday + Thursday + Friday + Saturday + + + Sun + Mon + Tue + Wed + Thu + Fri + Sat + + + AM + PM + + + BC + AD + + + + EEEE, MMMM D, YYYY + MMMM D, YYYY + MMM D, YYYY + M/D/YY + + + h:MM:SS A Z + h:MM:SS A Z + h:MM:SS A + h:MM A + + GyMdkHmsSEDFwWahKzZ + + z,zz9.zzz + $z,zz9.99|($z,zz9.99) + z,zz9% + + + . + , + % + - + 0 + + + $ + USD + . + + + + + + + + + + + + + + + + + +endstream +endobj +7 0 obj << + /Length 10 +>> +stream + +endstream +endobj +8 0 obj << + /Type /Pages + /Count 1 + /Kids [9 0 R] +>> +endobj +9 0 obj << + /Type /Page + /Parent 8 0 R + /MediaBox [0 0 612 792] +>> +endobj +xref +0 10 +0000000000 65535 f +0000000015 00000 n +0000000199 00000 n +0000000358 00000 n +0000000534 00000 n +0000001228 00000 n +0000002597 00000 n +0000006105 00000 n +0000006167 00000 n +0000006230 00000 n +trailer << + /Root 1 0 R + /Size 10 +>> +startxref +6307 +%%EOF diff --git a/xfa/fxfa/cxfa_fffield.h b/xfa/fxfa/cxfa_fffield.h index e26b49ea22..1d2b45f51f 100644 --- a/xfa/fxfa/cxfa_fffield.h +++ b/xfa/fxfa/cxfa_fffield.h @@ -73,6 +73,12 @@ class CXFA_FFField : public CXFA_FFWidget, public IFWL_WidgetDelegate { void UpdateFWL(); uint32_t UpdateUIProperty(); + // Test-only hook to simulate the "visible-but-not-yet-loaded" state that + // is otherwise hard to construct: forces IsLoaded() to return false so the + // page-widget iterator re-runs LoadWidget() (and thus the widget's initial + // SetText(), which dispatches a `change` event). + void ClearNormalWidgetForTesting() { normal_widget_ = nullptr; } + protected: explicit CXFA_FFField(CXFA_Node* pNode);