timbunce / devel-nytprof

Devel::NYTProf is a powerful feature-rich source code profiler for Perl. (Mostly in maintenance mode, so PRs are much more likely to be acted upon than Issues.)
http://blog.timbunce.org/tag/nytprof/
67 stars 51 forks source link

NYTProf.xs: prevent memory corruption in incr_sub_inclusive_time #115

Closed lucrocha closed 6 years ago

lucrocha commented 6 years ago

In incr_sub_inclusive_time, the write to subr_call_key could in some circumstances write beyond the size of the buffer: buffer overflow detected : /usr/sbin/uwsgi terminated ======= Backtrace: ========= /lib64/libc.so.6(fortify_fail+0x37)[0x7fb2c7589d87] /lib64/libc.so.6(+0x10df40)[0x7fb2c7587f40] /lib64/libc.so.6(+0x10d449)[0x7fb2c7587449] /lib64/libc.so.6(_IO_default_xsputn+0xbc)[0x7fb2c74f264c] /lib64/libc.so.6(_IO_vfprintf+0x151d)[0x7fb2c74c269d] /lib64/libc.so.6(vsprintf_chk+0x88)[0x7fb2c75874d8] /lib64/libc.so.6(__sprintf_chk+0x7d)[0x7fb2c758742d] /usr/local/git_tree/main/lib/site/lib/auto/Devel/NYTProf/NYTProf.so(+0xe483)[0x7fb2ad9f0483] /usr/local/booking-perl/5.24.3/lib/CORE/libperl.so(Perl_leave_scope+0x116)[0x7fb2c54fc3b6]

With gdb attached I could find the function:

10 0x00007faa38ff1363 in incr_sub_inclusive_time () from /usr/lib/pakket/5.24.3/libraries/active/lib/perl5/x86_64-linux/auto/Devel/NYTProf/NYTProf.so

Notably, the crash didn't happen with optimizations disabled, with the -g to Makefile.PL.

There's already a check for not exceeding the size of the buffer, but that comes after the memory corruption happens.

Changing from sprintf to snprinf fixes the memory corruption, and will return the number of bytes that would have been written if enough space was available, so the check for size still happens.