Bug: python-sip segmentation fault

[Phil Thompson]

I think the call became redundant when I replaced the call to 
sip_api_common_dtor() with the call to sip_api_instance_destroyed().

Phil

On 13/01/2022 11:09, artem rus wrote:
> Yes, and I believe this fixes the segmentation fault, but is it ok ?
> Maybe it creates new bugs, e.g. memory leakage ?
> 
> чт, 13 янв. 2022 г. в 13:42, Phil Thompson 
> <phil at riverbankcomputing.com>:
>> 
>> On 12/01/2022 20:42, artem rus wrote:
>> > Hello.
>> >
>> > It seems I found a use-after-free problem in sip4 and after code
>> > inspection I suspect the problem exists in sip5 and sip6.
>> >
>> > The bug is reported to debian bug tracker:
>> > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=998897
>> >
>> > The problem is two lines in objmap.c:
>> > sip_api_instance_destroyed(sw);
>> > sipSetNotInMap(sw);
>> >
>> > Originally lines was in different order, but by changeset
>> > 1526:b493c6f3e015 4.18-maint
>> > from repo https://www.riverbankcomputing.com/hg/sip
>> > lines exchanged:
>> >
>> > --- a/siplib/objmap.c    Tue Jun 21 14:14:37 2016 +0100
>> > +++ b/siplib/objmap.c    Sun Jul 03 23:32:48 2016 +0100
>> > @@ -270,9 +270,21 @@
>> >                  }
>> >                  else
>> >                  {
>> > -                    /* We are removing it from the map here. */
>> > +                    /*
>> > +                     * We are removing it from the map here.  However,
>> > note
>> > +                     * that we first have to call the destructor
>> > before marking
>> > +                     * it as not being in the map, as the destructor
>> > itself
>> > +                     * might end up trying to remove the wrapper and
>> > its
>> > +                     * aliases from the map.  In that case, if the
>> > wrapper is
>> > +                     * already marked as not in the map, the removal
>> > will just
>> > +                     * return early, leaving any potential aliases as
>> > stale
>> > +                     * entries in the map.  If we later try to wrap a
>> > different
>> > +                     * object at the same address, we end up
>> > retrieving the
>> > +                     * stale alias entry from the object map,
>> > triggering a
>> > +                     * use-after-free when accessing its C++ object.
>> > +                     */
>> > +                    sip_api_common_dtor(sw);
>> >                      sipSetNotInMap(sw);
>> > -                    sip_api_common_dtor(sw);
>> >                  }
>> >
>> >                  sw = next;
>> >
>> > Now line sip_api_instance_destroyed(sw) decreases refcounter and frees
>> > memory (for my system refcounter after this line is always 0). Next
>> > line tries to use freed memory and sometimes makes a segmentation
>> > fault.
>> >
>> > Can you check the problem in the latest sip and if it exists, fix it ?
>> >  Can you give me a patch for sip4 ?
>> >
>> > Best regards
>> > Artem Rusanov
>> 
>> Can you try just removing the call to sipSetNotInMap()?
>> 
>> Phil