Double free due to erroneous exception re-raising code

Add-in Express™ Support Service
That's what is more important than anything else

Double free due to erroneous exception re-raising code
 
David Heffernan




Posts: 9
Joined: 2013-01-03
There are many instances in the Add-In Express code base which re-raise exceptions in an erroneous manner. For instance, in adxAddIn consider this method:


constructor TadxEventSink.Create(AddInModule: TadxCOMAddInModule; AControl: TadxCommandBarControl);
begin
  inherited Create;
  FAddInModule := AddInModule;
  FIControl := AControl;
  FIBuiltInControl := nil;
  try
    if AControl.DefaultInterface.Type_ = msoControlButton then
      InterfaceConnect(AControl.DefaultInterface, DIID__CommandBarButtonEvents, Self, FCookie)
    else
      InterfaceConnect(AControl.DefaultInterface, DIID__CommandBarComboBoxEvents, Self, FCookie);
  except
    on E:SysUtils.Exception do begin
      {$IFDEF UNICODE}
      OutputDebugString(PWideChar('TadxEventSink.Create1 error: ' + E.Message));
      {$ELSE}
      OutputDebugString(PAnsiChar('TadxEventSink.Create1 error: ' + E.Message));
      {$ENDIF}
      raise E;
    end;
  end;
end;


In this code, the exception is deemed to be handled by the except block, and when execution leaves the except block, the exception is destroyed. However, the use of raise E will also raise that same object. Which means that when it is caught higher up, it will be destroyed again. The classic double free.

Marc Durdin discusses this issue in more detail here: https://marc.durdin.net/2012/10/how-not-to-re-raise-an-exception-in-delphi/

You need to use the special syntax of raise that is provided for re-raising exceptions: http://docwiki.embarcadero.com/RADStudio/en/Exceptions#Re-raising_Exceptions

So, change


raise E;


to


raise;


As I said, this mistake is made many times in the source code.
Posted 12 Oct, 2015 05:34:36 Top
Andrei Smolin


Add-in Express team


Posts: 14094
Joined: 2006-05-11
Huge thanks, David!

We will change the code.

Regards from Belarus (GMT+3),

Andrei Smolin
Add-in Express Team Leader
Posted 12 Oct, 2015 07:41:43 Top
David Heffernan




Posts: 9
Joined: 2013-01-03
Thanks Andrei
Posted 12 Oct, 2015 08:13:39 Top
Andrei Smolin


Add-in Express team


Posts: 14094
Joined: 2006-05-11
You are welcome!

Regards from Belarus (GMT+3),

Andrei Smolin
Add-in Express Team Leader
Posted 12 Oct, 2015 08:15:48 Top