tech-userlevel archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: buffer overflow in t_vis.c

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

Home | Main Index | Thread Index | Old Index