CHTMLToolTip design issue.

Message Topic Search Topic Options Post Reply Create New Topic Printable Version Translate Topic

   I have long had to work around issues when using HTML tooltips due to how the tooltip context destroys and deletes the actual tooltip. The issue is with HTML tooltips which must pump messages while waiting on navigation. The problem is while navigation is occurring, the "safe" delete code can be called.

Our HTML tooltips are more dynamic and can take a bit longer for navigation to complete and I have had to resort to all sorts of gymnastics to avoid touching deleted memory when returning from the message pumping loop. 

 Yes, Codejock's own CHTMLToolTip has exactly the same vulnerability — and it has NO protection at all.

  The scenario:

  1. FilterToolTipMessageHelper or SetStyle/ShowImage/ModifyToolTipStyle calls SAFE_DELETEWINDOW(m_pToolTip) — which is
  just ptr->DestroyWindow(); delete ptr; ptr = NULL;
  2. CHTMLToolTip::GetToolSize (line 1785) has a PumpMessage loop at lines 1847-1855 waiting for the IE WebBrowser to
  reach READYSTATE_COMPLETE. While pumping, a dispatched WM_MOUSEMOVE can flow through PreTranslateMessage →
  FilterToolTipMessage → FilterToolTipMessageHelper, which can call SAFE_DELETEWINDOW(m_pToolTip) at line 2437.
  3. When that happens: DestroyWindow() runs (the real CWnd::DestroyWindow — no override), delete runs the destructor
  (~CHTMLToolTip releases m_pBrowserApp, then ~CXTPToolTipContextToolTip and ~CWnd run), and then memory is freed.
  Control returns to the PumpMessage loop inside GetToolSize, which is now executing on a freed this pointer.
  4. The next line — m_pBrowserApp->get_ReadyState(&rs) at line 1854 — dereferences a freed IWebBrowser2* pointer
  through a freed this.

  Codejock has zero protection against this. No deferred DestroyWindow, no deferred delete, no m_bPumping flag, no
  refcounting. The CHTMLToolTip class is shipped with a latent use-after-free.

  The reason it rarely crashes in practice: Codejock's CHTMLToolTip is rarely used (most apps use xtpToolTipStandard or
  xtpToolTipOffice), and when it is used, the IE READYSTATE_COMPLETE pump is usually very fast since the content is a
  simple about: URL with inline HTML. The window of vulnerability is tiny — but it's there.

  Your JHTMLToolTip greatly expanded the pump duration (navigating to file-based HTML, waiting for complex rendering,
  doing multi-pass sizing), which is why you needed the three-layer defense. You weren't fixing a problem you introduced
  — you were protecting against a bug Codejock shipped.

This begs the question - why isn't the CJ source using addref/release mechanism as all the tips are CWnd derived objects? I have code to call InternalAddRef and InternalRelease before and after the pumping code needed to use HTML tips but this "safe" delete, which isn't so safe for me makes no use of ref counting.

My workaround is complex and involves overriding new/delete operators, deferring the actual delete to later. Testing constantly to see if CJ called delete ... all sorts of stuff. But it still suffers from the fact the c++ destructor is called when CJ calls "delete ptr;" My hacks solved most of the crashes when I first used HTML tips but it isn't foolproof due to the destructor running.

Author	Message Topic Search Topic Options Post Reply Create New Topic Printable Version Translate Topic
rdhd Members Profile Send Private Message Find Members Posts Add to Buddy List Senior Member Joined: 13 August 2007 Location: United States Status: Offline Points: 982	Post Options Post Reply Quote rdhd Report Post Thanks(0) Quote Reply Topic: CHTMLToolTip design issue. Posted: 6 hours 15 minutes ago at 10:19am
	I have long had to work around issues when using HTML tooltips due to how the tooltip context destroys and deletes the actual tooltip. The issue is with HTML tooltips which must pump messages while waiting on navigation. The problem is while navigation is occurring, the "safe" delete code can be called. Our HTML tooltips are more dynamic and can take a bit longer for navigation to complete and I have had to resort to all sorts of gymnastics to avoid touching deleted memory when returning from the message pumping loop. Yes, Codejock's own CHTMLToolTip has exactly the same vulnerability — and it has NO protection at all. The scenario: 1. FilterToolTipMessageHelper or SetStyle/ShowImage/ModifyToolTipStyle calls SAFE_DELETEWINDOW(m_pToolTip) — which is just ptr->DestroyWindow(); delete ptr; ptr = NULL; 2. CHTMLToolTip::GetToolSize (line 1785) has a PumpMessage loop at lines 1847-1855 waiting for the IE WebBrowser to reach READYSTATE_COMPLETE. While pumping, a dispatched WM_MOUSEMOVE can flow through PreTranslateMessage → FilterToolTipMessage → FilterToolTipMessageHelper, which can call SAFE_DELETEWINDOW(m_pToolTip) at line 2437. 3. When that happens: DestroyWindow() runs (the real CWnd::DestroyWindow — no override), delete runs the destructor (~CHTMLToolTip releases m_pBrowserApp, then ~CXTPToolTipContextToolTip and ~CWnd run), and then memory is freed. Control returns to the PumpMessage loop inside GetToolSize, which is now executing on a freed this pointer. 4. The next line — m_pBrowserApp->get_ReadyState(&rs) at line 1854 — dereferences a freed IWebBrowser2* pointer through a freed this. Codejock has zero protection against this. No deferred DestroyWindow, no deferred delete, no m_bPumping flag, no refcounting. The CHTMLToolTip class is shipped with a latent use-after-free. The reason it rarely crashes in practice: Codejock's CHTMLToolTip is rarely used (most apps use xtpToolTipStandard or xtpToolTipOffice), and when it is used, the IE READYSTATE_COMPLETE pump is usually very fast since the content is a simple about: URL with inline HTML. The window of vulnerability is tiny — but it's there. Your JHTMLToolTip greatly expanded the pump duration (navigating to file-based HTML, waiting for complex rendering, doing multi-pass sizing), which is why you needed the three-layer defense. You weren't fixing a problem you introduced — you were protecting against a bug Codejock shipped. This begs the question - why isn't the CJ source using addref/release mechanism as all the tips are CWnd derived objects? I have code to call InternalAddRef and InternalRelease before and after the pumping code needed to use HTML tips but this "safe" delete, which isn't so safe for me makes no use of ref counting. My workaround is complex and involves overriding new/delete operators, deferring the actual delete to later. Testing constantly to see if CJ called delete ... all sorts of stuff. But it still suffers from the fact the c++ destructor is called when CJ calls "delete ptr;" My hacks solved most of the crashes when I first used HTML tips but it isn't foolproof due to the destructor running.