summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom Sepez <tsepez@chromium.org>2023-03-16 11:25:27 +0000
committerMichael BrĂ¼ning <michael.bruning@qt.io>2023-03-24 10:08:17 +0000
commitfbe4414812b6c75fb4a3b05511248a4fb2abdc6b (patch)
tree8a9702a522f2599b5b3a6c9bc56b2197bbebe41b
parentab921d32398be925fcaf8edd3796d363d5716597 (diff)
downloadqtwebengine-chromium-fbe4414812b6c75fb4a3b05511248a4fb2abdc6b.tar.gz
[Backport] CVE-2023-1530: Use after free in PDF (1/2)
Cherry-pick of patch originally reviewed on https://pdfium-review.googlesource.com/c/pdfium/+/104833: 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 <tsepez@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> (cherry picked from commit 63e3719f1ec20ee6db804b2b2d4b00680db18d9c) Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/104833 Auto-Submit: Tom Sepez <tsepez@chromium.org> (cherry picked from commit a0d16d18d072ce77e639a09ed211340a2ad9034e) Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/468612 Reviewed-by: Michal Klocek <michal.klocek@qt.io>
-rw-r--r--chromium/third_party/pdfium/fpdfsdk/formfiller/cffl_checkbox.cpp25
-rw-r--r--chromium/third_party/pdfium/fpdfsdk/formfiller/cffl_listbox.cpp20
-rw-r--r--chromium/third_party/pdfium/fpdfsdk/formfiller/cffl_textfield.cpp25
3 files changed, 41 insertions, 29 deletions
diff --git a/chromium/third_party/pdfium/fpdfsdk/formfiller/cffl_checkbox.cpp b/chromium/third_party/pdfium/fpdfsdk/formfiller/cffl_checkbox.cpp
index 87253783371..3d2cfacfda8 100644
--- a/chromium/third_party/pdfium/fpdfsdk/formfiller/cffl_checkbox.cpp
+++ b/chromium/third_party/pdfium/fpdfsdk/formfiller/cffl_checkbox.cpp
@@ -65,9 +65,13 @@ bool CFFL_CheckBox::OnChar(CPDFSDK_Widget* pWidget,
CFFL_FormField::OnChar(pWidget, nChar, nFlags);
CPWL_CheckBox* pWnd = CreateOrUpdatePWLCheckBox(pPageView);
- if (pWnd && !pWnd->IsReadOnly())
- pWnd->SetCheck(!pWidget->IsChecked());
-
+ if (pWnd && !pWnd->IsReadOnly()) {
+ ObservedPtr<CPWL_CheckBox> pObservedBox(pWnd);
+ const bool is_checked = pWidget->IsChecked();
+ if (pObservedBox) {
+ pObservedBox->SetCheck(!is_checked);
+ }
+ }
return CommitData(pPageView, nFlags);
}
default:
@@ -80,14 +84,17 @@ bool CFFL_CheckBox::OnLButtonUp(CPDFSDK_PageView* pPageView,
Mask<FWL_EVENTFLAG> nFlags,
const CFX_PointF& point) {
CFFL_Button::OnLButtonUp(pPageView, pWidget, nFlags, point);
-
- if (!IsValid())
+ if (!IsValid()) {
return true;
-
+ }
CPWL_CheckBox* pWnd = CreateOrUpdatePWLCheckBox(pPageView);
- if (pWnd)
- pWnd->SetCheck(!pWidget->IsChecked());
-
+ if (pWnd) {
+ ObservedPtr<CPWL_CheckBox> pObservedBox(pWnd);
+ 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 42314af7395..a2de5a80de1 100644
--- a/chromium/third_party/pdfium/fpdfsdk/formfiller/cffl_listbox.cpp
+++ b/chromium/third_party/pdfium/fpdfsdk/formfiller/cffl_listbox.cpp
@@ -106,11 +106,15 @@ bool CFFL_ListBox::IsDataChanged(const CPDFSDK_PageView* pPageView) {
void CFFL_ListBox::SaveData(const CPDFSDK_PageView* pPageView) {
CPWL_ListBox* pListBox = GetPWLListBox(pPageView);
- if (!pListBox)
+ if (!pListBox) {
return;
-
+ }
int32_t nNewTopIndex = pListBox->GetTopVisibleIndex();
+ ObservedPtr<CPWL_ListBox> observed_box(pListBox);
m_pWidget->ClearSelection();
+ 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))
@@ -122,17 +126,17 @@ void CFFL_ListBox::SaveData(const CPDFSDK_PageView* pPageView) {
ObservedPtr<CPDFSDK_Widget> observed_widget(m_pWidget.Get());
ObservedPtr<CFFL_ListBox> 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 e1ff0319c64..6170d7530d8 100644
--- a/chromium/third_party/pdfium/fpdfsdk/formfiller/cffl_textfield.cpp
+++ b/chromium/third_party/pdfium/fpdfsdk/formfiller/cffl_textfield.cpp
@@ -147,26 +147,29 @@ bool CFFL_TextField::IsDataChanged(const CPDFSDK_PageView* pPageView) {
}
void CFFL_TextField::SaveData(const CPDFSDK_PageView* pPageView) {
- CPWL_Edit* pWnd = GetPWLEdit(pPageView);
- if (!pWnd)
+ ObservedPtr<CPWL_Edit> observed_edit(GetPWLEdit(pPageView));
+ if (!observed_edit) {
return;
-
+ }
WideString sOldValue = m_pWidget->GetValue();
- WideString sNewValue = pWnd->GetText();
+ if (!observed_edit) {
+ return;
+ }
+ WideString sNewValue = observed_edit->GetText();
ObservedPtr<CPDFSDK_Widget> observed_widget(m_pWidget.Get());
ObservedPtr<CFFL_TextField> observed_this(this);
m_pWidget->SetValue(sNewValue);
- 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();
}
@@ -177,9 +180,7 @@ void CFFL_TextField::GetActionData(const CPDFSDK_PageView* pPageView,
case CPDF_AAction::kKeyStroke:
if (CPWL_Edit* pWnd = GetPWLEdit(pPageView)) {
fa.bFieldFull = pWnd->IsTextFull();
-
fa.sValue = pWnd->GetText();
-
if (fa.bFieldFull) {
fa.sChange.clear();
fa.sChangeEx.clear();