Possible overflow bug?

[Chris Rapier]

On Tue, Jun 6, 2023, 16:31 Sam James <sam at gentoo.org> wrote:

>
> Chris Rapier <rapier at psc.edu> writes:
>
> > On 6/6/23 2:59 PM, Peter Stuge wrote:
> >> Chris Rapier wrote:
> >>> openssh 9.3p1
> >> ..
> >>> In function 'explicit_bzero',
> >>>       inlined from 'kex_free_newkeys' at kex.c:743:2:
> >> kex.c in tag V_9_3_P1 doesn't call explicit_bzero() on line 743,
> >>> '__explicit_bzero_chk' writing 48 bytes into a region of size 8
> >> ..
> >>> kex.h: In function 'kex_free_newkeys':
> >>> kex.h:116:18: note: destination object 'name' of size 8
> >>>     116 |         char    *name;
> >> ... in fact kex_free_newkeys() in tag V_9_3_P1 doesn't ever call
> >> explicit_bzero() with an object called 'name'.
> >>
> >>> Not sure if this is a real problem or not but I thought I'd pass it
> >>> over just in case.
> >> Could you check if you have any patch applied on top of V_9_3_P1?
> >
> >
> > I'm using commit cb30fbdbee869f1ce11f06aa97e1cb8717a0b645 (HEAD, tag:
> > V_9_3_P1, openssh-master/V_9_3) and git diff isn't reporting anything
> > applied.
> >
> > And I just realized I grabbed that from the wrong window (which does
> > have patches applied). Same thing exists in the canonical code. Here
> > is the accurate one:
> >
> >
> > In file included from /usr/include/string.h:535,
> >                  from kex.c:34:
> > In function ‘explicit_bzero’,
> >     inlined from ‘kex_free_newkeys’ at kex.c:742:2:
> > /usr/include/bits/string_fortified.h:72:3: warning:
> > ‘__explicit_bzero_chk’ writing 48 bytes into a region of size 8
> > overflows the destination [-Wstringop-overflow=]
> >    72 |   __explicit_bzero_chk (__dest, __len, __glibc_objsize0
> (__dest));
> >       |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > In file included from kex.c:53:
> > kex.h: In function ‘kex_free_newkeys’:
> > kex.h:116:18: note: destination object ‘name’ of size 8
> >   116 |         char    *name;
> >       |                  ^~~~
> > /usr/include/bits/string_fortified.h:66:6: note: in a call to function
> > ‘__explicit_bzero_chk’ declared with attribute ‘access (write_only, 1,
> > 2)’
> >    66 | void __explicit_bzero_chk (void *__dest, size_t __len, size_t
> >    __destlen)
> >       |      ^~~~~~~~~~~~~~~~~~~~
> >
> > Sorry about the confusion before. I always have too many terminals open.
>
> Not a comment on this particular bug, but as an FYI, sanitizers are
> known to sometimes cause false-positive *compile*-time warnings (not
> runtime
> failures, which are pretty much always legitimate).
>
>
> This doesn't actually surprise me. I'd have expected to see actual
> problems during runtime if it really was zeroing out 40 extra bytes each
> time it called this function. On the other hand, I wanted to run it by
> people in case there really is a problem.
>
>