From fb66d7ca9641724670c96e999ad5b0fd6eb78d46 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Tue, 7 Mar 2023 16:04:24 +0000 Subject: [Backport] CVE-2023-1530: Use after free in PDF (1/2) Manual backport of patch originally reviewed on https://pdfium-review.googlesource.com/c/pdfium/+/104397: Observe CPWL_* object destruction across CPDFSDK_Widget methods This is a simple fix to stop the symptoms while we investigate how to avoid mutations at these points in the first place. -- fix some nearby braces and annoying blank lines while at it. Bug: chromium:1419831 Change-Id: I20c38806b91c7c0c9016bb1b567a04ce319243d8 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/104397 Commit-Queue: Tom Sepez Reviewed-by: Lei Zhang Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/469851 Reviewed-by: Michal Klocek --- .../pdfium/fpdfsdk/formfiller/cffl_checkbox.cpp | 12 ++++++++-- .../pdfium/fpdfsdk/formfiller/cffl_listbox.cpp | 16 ++++++++----- .../pdfium/fpdfsdk/formfiller/cffl_textfield.cpp | 26 ++++++++++++---------- 3 files changed, 34 insertions(+), 20 deletions(-) diff --git a/chromium/third_party/pdfium/fpdfsdk/formfiller/cffl_checkbox.cpp b/chromium/third_party/pdfium/fpdfsdk/formfiller/cffl_checkbox.cpp index c8f114896a1..3647bd3dbc4 100644 --- a/chromium/third_party/pdfium/fpdfsdk/formfiller/cffl_checkbox.cpp +++ b/chromium/third_party/pdfium/fpdfsdk/formfiller/cffl_checkbox.cpp @@ -64,8 +64,12 @@ bool CFFL_CheckBox::OnChar(CPDFSDK_Annot* pAnnot, CPWL_CheckBox* pWnd = GetCheckBox(pPageView, true); if (pWnd && !pWnd->IsReadOnly()) { + ObservedPtr pObservedBox(pWnd); CPDFSDK_Widget* pWidget = ToCPDFSDKWidget(pAnnot); - pWnd->SetCheck(!pWidget->IsChecked()); + const bool is_checked = pWidget->IsChecked(); + if (pObservedBox) { + pObservedBox->SetCheck(!is_checked); + } } return CommitData(pPageView, nFlags); @@ -86,8 +90,12 @@ bool CFFL_CheckBox::OnLButtonUp(CPDFSDK_PageView* pPageView, CPWL_CheckBox* pWnd = GetCheckBox(pPageView, true); if (pWnd) { + ObservedPtr pObservedBox(pWnd); CPDFSDK_Widget* pWidget = ToCPDFSDKWidget(pAnnot); - pWnd->SetCheck(!pWidget->IsChecked()); + const bool is_checked = pWidget->IsChecked(); + if (pObservedBox) { + pObservedBox->SetCheck(!is_checked); + } } return CommitData(pPageView, nFlags); diff --git a/chromium/third_party/pdfium/fpdfsdk/formfiller/cffl_listbox.cpp b/chromium/third_party/pdfium/fpdfsdk/formfiller/cffl_listbox.cpp index 0cbc20a213c..c35f1485d51 100644 --- a/chromium/third_party/pdfium/fpdfsdk/formfiller/cffl_listbox.cpp +++ b/chromium/third_party/pdfium/fpdfsdk/formfiller/cffl_listbox.cpp @@ -110,7 +110,11 @@ void CFFL_ListBox::SaveData(CPDFSDK_PageView* pPageView) { return; int32_t nNewTopIndex = pListBox->GetTopVisibleIndex(); + ObservedPtr observed_box(pListBox); m_pWidget->ClearSelection(NotificationOption::kDoNotNotify); + if (!observed_box) { + return; + } if (m_pWidget->GetFieldFlags() & pdfium::form_flags::kChoiceMultiSelect) { for (int32_t i = 0, sz = pListBox->GetCount(); i < sz; i++) { if (pListBox->IsItemSelected(i)) { @@ -125,17 +129,17 @@ void CFFL_ListBox::SaveData(CPDFSDK_PageView* pPageView) { ObservedPtr observed_widget(m_pWidget.Get()); ObservedPtr observed_this(this); m_pWidget->SetTopVisibleIndex(nNewTopIndex); - if (!observed_widget) + if (!observed_widget) { return; - + } m_pWidget->ResetFieldAppearance(); - if (!observed_widget) + if (!observed_widget) { return; - + } m_pWidget->UpdateField(); - if (!observed_widget || !observed_this) + if (!observed_widget || !observed_this) { return; - + } SetChangeMark(); } diff --git a/chromium/third_party/pdfium/fpdfsdk/formfiller/cffl_textfield.cpp b/chromium/third_party/pdfium/fpdfsdk/formfiller/cffl_textfield.cpp index 93ec9fe1e72..85402d45d23 100644 --- a/chromium/third_party/pdfium/fpdfsdk/formfiller/cffl_textfield.cpp +++ b/chromium/third_party/pdfium/fpdfsdk/formfiller/cffl_textfield.cpp @@ -145,26 +145,30 @@ bool CFFL_TextField::IsDataChanged(CPDFSDK_PageView* pPageView) { } void CFFL_TextField::SaveData(CPDFSDK_PageView* pPageView) { - CPWL_Edit* pWnd = GetEdit(pPageView, false); - if (!pWnd) + ObservedPtr observed_edit(GetEdit(pPageView, false)); + if (!observed_edit) { return; - + } WideString sOldValue = m_pWidget->GetValue(); - WideString sNewValue = pWnd->GetText(); + if (!observed_edit) { + return; + } + + WideString sNewValue = observed_edit->GetText(); ObservedPtr observed_widget(m_pWidget.Get()); ObservedPtr observed_this(this); m_pWidget->SetValue(sNewValue, NotificationOption::kDoNotNotify); - if (!observed_widget) + if (!observed_widget) { return; - + } m_pWidget->ResetFieldAppearance(); - if (!observed_widget) + if (!observed_widget) { return; - + } m_pWidget->UpdateField(); - if (!observed_widget || !observed_this) + if (!observed_widget || !observed_this) { return; - + } SetChangeMark(); } @@ -175,9 +179,7 @@ void CFFL_TextField::GetActionData(CPDFSDK_PageView* pPageView, case CPDF_AAction::kKeyStroke: if (CPWL_Edit* pWnd = GetEdit(pPageView, false)) { fa.bFieldFull = pWnd->IsTextFull(); - fa.sValue = pWnd->GetText(); - if (fa.bFieldFull) { fa.sChange.clear(); fa.sChangeEx.clear(); -- cgit v1.2.1