Hmm. That's weird code:
int d_idx = dp->maxdepth.mm / (STATS_DEPTH_BUCKET * 1000);
d_idx = std::clamp(d_idx, 0, STATS_MAX_DEPTH / STATS_DEPTH_BUCKET);
process_dive(*dp, out.stats_by_depth[d_idx + 1]);
So we make sure that d_idx is an allowable index value, and then add 1 to it...
I could see how that might access beyond the end of the array.
/D
> On Jan 10, 2025, at 09:46, Christof Arnosti via subsurface
> <[email protected]> wrote:
>
> Check the passed stats_t reference, this is the structure that is read there.
> I guess the pointer passed there is outside of the out.stat_by_depth array.
> The field "total_average_depth_time" is quite at the beginning of this
> structure, thus the 8 byte offset makes sense.
>
> Best
> Christof
>
> On 10.01.25 18:33, Dirk Hohndel via subsurface wrote:
>> I ended up using the ASAN build we already have configured in cmake:
>>
>> =================================================================
>> ==960805==ERROR: AddressSanitizer: heap-buffer-overflow on address
>> 0x520000010e18 at pc 0x56f170bc8a05 bp 0x7ffcd0552140 sp 0x7ffcd0552130
>> READ of size 4 at 0x520000010e18 thread T0
>> #0 0x56f170bc8a04 in process_dive
>> /home/hohndel/src/subsurface/core/statistics.cpp:50
>> #1 0x56f170bca05e in calculate_stats_summary(bool)
>> /home/hohndel/src/subsurface/core/statistics.cpp:162
>> #2 0x56f170a2dbef in exportHTMLstatistics
>> /home/hohndel/src/subsurface/core/divelogexportlogic.cpp:81
>> #3 0x56f170a307fd in exportHtmlInitLogic(QString const&,
>> htmlExportSetting&)
>> /home/hohndel/src/subsurface/core/divelogexportlogic.cpp:142
>> #4 0x56f1709f89a3 in main /home/hohndel/src/subsurface/export-html.cpp:67
>> #5 0x7522d4c2a1c9 in __libc_start_call_main
>> ../sysdeps/nptl/libc_start_call_main.h:58
>> #6 0x7522d4c2a28a in __libc_start_main_impl ../csu/libc-start.c:360
>> #7 0x56f1709f7aa4 in _start
>> (/srv/hohndel/src/subsurface/build/export-html+0x236aa4) (BuildId:
>> 3185437f967b115b8c1c9b6c8b5b8fe3d46e49bb)
>>
>> 0x520000010e18 is located 8 bytes after 3472-byte region
>> [0x520000010080,0x520000010e10)
>> allocated by thread T0 here:
>> #0 0x7522d88fe548 in operator new(unsigned long)
>> ../../../../src/libsanitizer/asan/asan_new_delete.cpp:95
>> #1 0x56f170bd0eef in std::__new_allocator<stats_t>::allocate(unsigned
>> long, void const*) /usr/include/c++/13/bits/new_allocator.h:151
>> #2 0x56f170bd051e in std::allocator_traits<std::allocator<stats_t>
>> >::allocate(std::allocator<stats_t>&, unsigned long)
>> /usr/include/c++/13/bits/alloc_traits.h:482
>> #3 0x56f170bd051e in std::_Vector_base<stats_t, std::allocator<stats_t>
>> >::_M_allocate(unsigned long) /usr/include/c++/13/bits/stl_vector.h:381
>> #4 0x56f170bcf11d in std::vector<stats_t, std::allocator<stats_t>
>> >::_M_default_append(unsigned long) /usr/include/c++/13/bits/vector.tcc:663
>> #5 0x56f170bcd6ba in std::vector<stats_t, std::allocator<stats_t>
>> >::resize(unsigned long) /usr/include/c++/13/bits/stl_vector.h:1016
>> #6 0x56f170bc96bb in calculate_stats_summary(bool)
>> /home/hohndel/src/subsurface/core/statistics.cpp:119
>> #7 0x56f170a2dbef in exportHTMLstatistics
>> /home/hohndel/src/subsurface/core/divelogexportlogic.cpp:81
>> #8 0x56f170a307fd in exportHtmlInitLogic(QString const&,
>> htmlExportSetting&)
>> /home/hohndel/src/subsurface/core/divelogexportlogic.cpp:142
>> #9 0x56f1709f89a3 in main /home/hohndel/src/subsurface/export-html.cpp:67
>> #10 0x7522d4c2a1c9 in __libc_start_call_main
>> ../sysdeps/nptl/libc_start_call_main.h:58
>> #11 0x7522d4c2a28a in __libc_start_main_impl ../csu/libc-start.c:360
>> #12 0x56f1709f7aa4 in _start
>> (/srv/hohndel/src/subsurface/build/export-html+0x236aa4) (BuildId:
>> 3185437f967b115b8c1c9b6c8b5b8fe3d46e49bb)
>>
>> SUMMARY: AddressSanitizer: heap-buffer-overflow
>> /home/hohndel/src/subsurface/core/statistics.cpp:50 in process_dive
>>
>> So clearly we are addressing data outside the structure. I haven't figured
>> out how that could happen on line 50, though...
>>
>> /D
>>
>>> On Jan 10, 2025, at 08:42, Berthold Stoeger <[email protected]>
>>> <mailto:[email protected]> wrote:
>>>
>>> Hi Dirk,
>>>
>>> please run a debug version under valgrind and send me the result. A minimal
>>> reproducing example would also be nice (repeatedly delete one half of the
>>> dives and check if the issue is still there).
>>>
>>> N.B.: I'm in Indonesia until end of February and therefore will not be very
>>> responsive. I'll try to check my e-mails once per day.
>>>
>>> Berthold
>>>
>>> On Freitag, 10. Jänner 2025 23:29:14 WIB Dirk Hohndel via subsurface wrote:
>>>> I can't easily give you data to replicate the issue (as that's a user's
>>>> dive
>>>> log - they gave me permission to use it to debug the issue, but not to
>>>> share it with others), but I can see that the code crashes in
>>>> statistics.cpp:201 on the emplace_back() call. malloc(): invalid size
>>>> (unsorted)
>>>> my guess is that this isn't where the actual but happens, this feels more
>>>> like a memory corruption to me, but what do I know.
>>>>
>>>> Oh, this happens with export_html, not the main app. I'll try to debug this
>>>> some more, but on the off chance that this rings a bell for someone, please
>>>> do let me know.
>>>>
>>>> /D
>>>> _______________________________________________
>>>> subsurface mailing list -- [email protected]
>>>> <mailto:[email protected]>
>>>> To unsubscribe send an email to [email protected]
>>>> <mailto:[email protected]>
>>>
>> _______________________________________________
>> subsurface mailing list -- [email protected]
>> <mailto:[email protected]>
>> To unsubscribe send an email to [email protected]
>> <mailto:[email protected]>
> _______________________________________________
> subsurface mailing list -- [email protected]
> To unsubscribe send an email to [email protected]
_______________________________________________
subsurface mailing list -- [email protected]
To unsubscribe send an email to [email protected]
