PS: Same bug seems to be in
mobile-widgets/statsmanager.cpp
stats/chartlistmodel.cpp
stats/statsstate.cpp
On Samstag, 11. Jänner 2025 05:52:11 WIB Berthold Stoeger via subsurface wrote:
> The +1 is OK, since the resize() calls also have a +1:
>
> out.stats_by_depth.resize((STATS_MAX_DEPTH / STATS_DEPTH_BUCKET) + 1);
> out.stats_by_temp.resize((STATS_MAX_TEMP / STATS_TEMP_BUCKET) + 1);
>
> but the std::clamp() are not OK, because the interval is inclusive. They
> should read
>
> d_idx = std::clamp(d_idx, 0, STATS_MAX_DEPTH / STATS_DEPTH_BUCKET - 1);
> t_idx = std::clamp(t_idx, 0, STATS_MAX_TEMP / STATS_TEMP_BUCKET - 1);
>
> Berthold
>
> On Samstag, 11. Jänner 2025 00:53:06 WIB Dirk Hohndel via subsurface wrote:
> > 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
> > > <subsurface@subsurface-divelog.org> 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
_______________________________________________
subsurface mailing list -- subsurface@subsurface-divelog.org
To unsubscribe send an email to subsurface-le...@subsurface-divelog.org
