Opened on 08/04/2014 at 04:06:28 PM

Closed on 11/21/2016 at 10:59:50 AM

#1173 closed change (fixed)

Ensure that C++ exceptions do not propagate out of entry points

Reported by: Assignee:
Priority: P4 Milestone: Adblock-Plus-for-Internet-Explorer-1.6
Module: Adblock-Plus-for-Internet-Explorer Keywords:
Cc: Blocked By: #2015
Blocking: Platform: Internet Explorer
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

Description (last modified by


The plugin code is not friendly to exceptions. In most places in the code, an exception will propagate upward all the way up the stack to the entry point. At the time of creating this ticket, none of the entry points have exception handlers (try-catch block), and so such exceptions will lead to undefined behavior.

Since the C++ idiom is to use exceptions as an ordinary way to signal certain rare circumstances (memory allocation, invalid constructor calls, etc.), the absence of such handlers is a fundamental quality problem.

What to change

  • Ensure that all entry points have an exception handler that will catch all thrown exceptions, that is, a catch(...) statement on the outermost block.
  • Ideally, use entry point calling conventions only for entry points. This will simplify auditing the code to ensure that this change as finished.

Attachments (0)

Change History (14)

comment:1 Changed on 08/04/2014 at 04:35:54 PM by

  • Description modified (diff)

Entry points (as a rule) don't use C++ calling conventions, and thus tend to be marked by platform-specific annotations, such as WINAPI and STDMETHODIMP, both of whose definitions include the annotation __stdcall.

  • The entry points associated with DLL packaging. They are in Plugin.cpp: DllMain, DllCanUnloadNow, DllRegisterServer, DllUnregisterServer
  • BHO entry points from implementing IObjectWithSite, namely CPluginClass::SetSite.
  • The browser event handler CPluginClass::Invoke.
  • The COM entry points from implementing the user settings class CPluginUserSettings.
  • The event handlers in WBPassthruSink.
  • The COM entry points from implementing IInternetProtocolImpl in the PassthroughAPP library.

I believe this enumerates most of the entry points, though I'm not making any claim that it's all of them.

comment:2 Changed on 12/11/2014 at 10:03:27 AM by oleksandr

  • Priority changed from Unknown to P4
  • Ready set

comment:3 Changed on 12/31/2014 at 08:24:58 PM by

  • Review URL(s) modified (diff)

comment:4 Changed on 01/06/2015 at 06:01:25 PM by

Changeset 740 ea510d07ff97f1b50ef5389c0918276d843df391 fixes this for CPluginClass::Invoke

comment:5 follow-up: Changed on 01/29/2015 at 01:35:35 PM by sergz

  • Review URL(s) modified (diff)

Wrap more entry points in PluginClass.cpp and add the proposal for the wrapper function. Having these functions we don't have to repeat try-catch and can improve catching for all entry points at once. As well as we don't need to comment each method.

comment:6 Changed on 02/19/2015 at 07:27:01 AM by oleksandr

  • Milestone set to Adblock-Plus-for-Internet-Explorer-1.4

comment:7 Changed on 02/19/2015 at 07:32:46 AM by oleksandr

  • Blocked By 2015 added

comment:8 Changed on 02/19/2015 at 07:34:38 AM by oleksandr

  • Milestone Adblock-Plus-for-Internet-Explorer-1.4 deleted

comment:9 Changed on 02/23/2015 at 02:50:35 PM by

  • Review URL(s) modified (diff)

comment:10 in reply to: ↑ 5 Changed on 02/24/2015 at 09:02:31 AM by sergz

  • Review URL(s) modified (diff)

Oleksandr said:

So currently I agree with Eric. Let's just stick to try-catch blocks for now.

comment:11 Changed on 12/22/2015 at 01:34:30 PM by

  • Tester set to Unknown

More entry points.

  • In CPluginClass interface `IOleCommandTarget'
    • QueryStatus()
    • Exec()
  • In CPluginClass, callbacks for UI elements
    • NewStatusProc(), status bar windows
    • PaneWindowProc(), window class of status bar
Last edited on 12/22/2015 at 01:49:03 PM by

comment:12 Changed on 01/07/2016 at 02:22:05 PM by

  • Review URL(s) modified (diff)

comment:13 Changed on 10/18/2016 at 10:22:38 AM by oleksandr

  • Milestone set to Adblock-Plus-for-Internet-Explorer-Next

comment:14 Changed on 11/21/2016 at 10:59:50 AM by oleksandr

  • Resolution set to fixed
  • Status changed from new to closed

Add Comment

Modify Ticket

Change Properties
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from (none).
Note: See TracTickets for help on using tickets.