Blob and copy/move constructor

werto87 commented 2 years ago

Blob has no explicit copy/move constructor but a destructor which calls delete. So making a copy/move can result in double free or use after free. example:

  soci::session sql (soci::sqlite3, databaseName);
  auto originalBlob = soci::blob{ sql };
  auto copyBlob = originalBlob;

AddressSanitizer:DEADLYSIGNAL
=================================================================
==46681==ERROR: AddressSanitizer: SEGV on unknown address 0x00000700000a (pc 0x55f275330f88 bp 0x7fff806d8ef0 sp 0x7fff806d8ee0 T0)
==46681==The signal is caused by a READ memory access.
    #0 0x55f275330f88 in soci::blob::~blob() /home/walde/.conan/data/soci/4.0.2/werto87/stable/build/7ef31e0413043c7eb48a64bb0d0adb2c77e9639e/source_subfolder/src/core/blob.cpp:23:5
    #1 0x55f2752e611f in main /home/walde/projects/sandbox/cxx_template/main.cxx:129:1
    #2 0x14f42d6f030f in __libc_start_call_main (/usr/lib/libc.so.6+0x2d30f)
    #3 0x14f42d6f03c0 in __libc_start_main_impl (/usr/lib/libc.so.6+0x2d3c0)
    #4 0x55f2751fdd14 in _start (/home/walde/projects/sandbox/cxx_template/build/bin/project+0x32d14)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /home/walde/.conan/data/soci/4.0.2/werto87/stable/build/7ef31e0413043c7eb48a64bb0d0adb2c77e9639e/source_subfolder/src/core/blob.cpp:23:5 in soci::blob::~blob()
==46681==ABORTING

Possible solution: remove destructor of blob and replace "details::blobbackend * backEnd;" with std::unique_ptr < details::blobbackend > backEnd;

Problem: This is api breaking

Krzmbrzl commented 2 years ago

This is api breaking

You mean, because it makes blobs non-copyable? To me that appears more like a bug fix than a realy API break (given that right now everyone who performed a copy will have a double-free and thus a bug in their code)

vadz commented 2 years ago

Yes, I agree that in this case we should break compatibility because not compiling is better than compiling and crashing during run-time.

Krzmbrzl commented 2 years ago

Using smart-pointers is not possible though, as we currently still support pre-CXX11 standards. But it can still be done.

vadz commented 2 years ago

4.1.0 is supposed to require C++14, see #907. I just never got around actually removing C++98 support from the code yet.

Krzmbrzl commented 2 years ago

Ah well - I already implemented in a compatible way anyway :shrug:

SOCI / soci

Blob and copy/move constructor #957