On Thu, Apr 13, 2017 at 07:56:39AM +0100, Iain Hibbert wrote: > On Thu, 13 Apr 2017, Brooks Davis wrote: > > > I've found a one byte buffer overflow in t_vis.c. It's caused by a > > quite reasonable confusion about an undocumented behavior of always add > > a '\0' terminating the dst string in strnunvisx(). This patch fixes the > > test, but I think the behavior is confusing and should be documented in > > addition to the requirement that the buffers by the same length. > > I don't think the comment is very clear, can you say where the additional > \0 comes from? Is it in fact strunvisx() which adds it, or is it because > the original byte string is not NUL terminated, but the strsvisx() call > returns a NUL terminated string, and then when you strunvisx() on that, it > considers that the string terminator is part of the string? I guess it's rightly that strunvisx() considers the string terminator added by strsvisx() to be part of the string. It seems a bit weird that we don't have a strunvis() set that is actually the opposite of strvis(). > would it be better for the test, to use strnunvisx(), or will that fail > and return ENOSPC ? (reading the manpage, I'm not sure if it will just set > errno, rather than fail) I believe it will fail with ENOSPC, there's a CHECKSPACE() right before the offending store. -- Brooks
Attachment:
signature.asc
Description: PGP signature