Fix UAF in SetVisible().

SetVisible() may be called during Destroy() which may be called
during SetVisible().

This fixes the latest in a family of bugs that happen after an
instance is freed by code triggered by JS code while it's executing
a method.

The CL has a lot of protection for many of these points where JS
may be executed and potentially destroy objects. The return types
of many methods that may execute JS have been changed to bool,
indicating whether the instance is still alive after the call.

Bug: chromium:770148
Change-Id: If5a9db4d8d6aac10f4dd6b645922bb96c116684d
Reviewed-on: https://pdfium-review.googlesource.com/15190
Reviewed-by: dsinclair <dsinclair@chromium.org>
Commit-Queue: Henrique Nakashima <hnakashima@chromium.org>

1 files changed, 11 insertions, 14 deletions

diff --git a/fpdfsdk/pwl/cpwl_caret.cpp b/fpdfsdk/pwl/cpwl_caret.cpp
index c58f019c4c..77a768afe0 100644
--- a/fpdfsdk/pwl/cpwl_caret.cpp
+++ b/fpdfsdk/pwl/cpwl_caret.cpp
@@ -83,7 +83,7 @@ void CPWL_Caret::SetCaret(bool bVisible,
     EndTimer();
     CPWL_Wnd::SetVisible(false);
     // Note, |this| may no longer be viable at this point. If more work needs
-    // to be done, add an observer.
+    // to be done, check the return value of SetVisible().
     return;
   }
 
@@ -93,15 +93,13 @@ void CPWL_Caret::SetCaret(bool bVisible,
     EndTimer();
     BeginTimer(PWL_CARET_FLASHINTERVAL);
 
-    ObservedPtr observer(this);
-    CPWL_Wnd::SetVisible(true);
-    if (!observer)
+    if (!CPWL_Wnd::SetVisible(true))
       return;
 
     m_bFlash = true;
     Move(m_rcInvalid, false, true);
     // Note, |this| may no longer be viable at this point. If more work needs
-    // to be done, add an observer.
+    // to be done, check the return value of Move().
     return;
   }
 
@@ -113,15 +111,12 @@ void CPWL_Caret::SetCaret(bool bVisible,
   m_bFlash = true;
   Move(m_rcInvalid, false, true);
   // Note, |this| may no longer be viable at this point. If more work
-  // needs to be done, add an observer.
+  // needs to be done, check the return value of Move().
 }
 
-void CPWL_Caret::InvalidateRect(CFX_FloatRect* pRect) {
+bool CPWL_Caret::InvalidateRect(CFX_FloatRect* pRect) {
   if (!pRect) {
-    CPWL_Wnd::InvalidateRect(nullptr);
-    // Note, |this| may no longer be viable at this point. If more work needs
-    // to be done, add an observer.
-    return;
+    return CPWL_Wnd::InvalidateRect(nullptr);
   }
 
   CFX_FloatRect rcRefresh = *pRect;
@@ -131,7 +126,9 @@ void CPWL_Caret::InvalidateRect(CFX_FloatRect* pRect) {
   }
   rcRefresh.top += 1;
   rcRefresh.bottom -= 1;
-  CPWL_Wnd::InvalidateRect(&rcRefresh);
-  // Note, |this| may no longer be viable at this point. If more work needs
-  // to be done, add an observer.
+  return CPWL_Wnd::InvalidateRect(&rcRefresh);
+}
+
+bool CPWL_Caret::SetVisible(bool bVisible) {
+  return true;
 }