[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Groff] Re: groff 1.17: pre-grohtml's unsafe temporary file handling
Re: [Groff] Re: groff 1.17: pre-grohtml's unsafe temporary file handling & other improvements
Wed, 13 Jun 2001 04:52:34 +0400
On Tue, Jun 12, 2001 at 06:17:26PM +0100, Gaius Mulley wrote:
> Many thanks to all who have suggested improvements to pre-grohtml.
> I've reworked several buffer issues within pre-grohtml in light of
> the overflow problem highlighted by several people. Here is the
> latest patch:
It really looks better, thanks!
> + n = vsnprintf (p, size, fmt, ap);
I'm afraid, groff is meant to be portable to systems which either
don't have vsnprintf() or have versions written prior to when the
return value semantics were discussed, let alone standardized.
I think other parts of groff would benefit from the ability to use
*snprintf(), so it may be a good idea to include an implementation
with groff itself for use on systems which lack these functions.
> + /* If that worked, return the string. */
> + if (n > -1 && n < size)
> + return p;
> + /* Else try again with more space. */
> + if (n > -1) /* glibc 2.1 */
> + size = n+1; /* precisely what is needed */
This is how *snprintf() should work.
> + else /* glibc 2.0 */
There's a third possibility: implementations which only return the
number of characters actually written. SUSv2 is not very explicit
about the return value:
Upon successful completion, these functions return the number of
bytes transmitted excluding the terminating null in the case of
sprintf() or snprintf() or a negative value if an output error was
If the value of n is zero on a call to snprintf(), an unspecified
value less than 1 is returned.
It is non-obvious whether a too small value of n qualifies as an
Clearly, this specification was written prior to the public discussions
where Theo de Raadt and others were advocating the semantics now seen
with glibc 2.1+.
A simple way to cover this third possibility is to require that:
n > -1 && n < size - 1
and increase the allocation size if all of the buffer was used.
> + size *= 2; /* twice the old size */
> + if ((p = (char *)realloc (p, size)) == NULL)
> + return NULL;
This is a non-issue for pre-grohtml, but this use of realloc() would
leak memory on failure. Probably worth adding a comment, or use a
temporary variable to avoid the leak.
For the size *= 2 case, you could add another realloc() to decrease
the allocation size to what was actually needed.