snprintf(3) behaviour regarding large "n"

[Steffen Nurpmeso]

Hello NetBSD,

i recently had a hard time because NetBSD was the only operating
system that fails to pass the testsuite (S/MIME function
verification, and only this test!) of the S-nail mailer
i maintain.

Short: the problem is that i use snprintf(3) with a "size_t n"
argument of UI32_MAX (is EQ to UINT32_MAX).
Well, looking at POSIX this is even somewhat correct (but then it
should be EOVERFLOW for conformance), but it is (a) neither
documented in the manual nor (b) would a halfway sane person do it
like that -- :) --, though i'd agree that i should have used
SIZE_MAX to indicate what i had in mind ("buffer _is_ large
enough"), not at last because there is an error defined for when
_the return value_ would exceed INT_MAX (also EOVERFLOW).

The patch has not been compile tested (regarding availability of
INT_MAX), sorry (very small resources here).

--steffen

[Steffen Nurpmeso]

P.S.: it would possibly be even better to not do something like
casting a size_t to int and testing for negativity, but simply
testing '> INT_MAX' instead.

--steffen

[Steffen Nurpmeso]

And, finally:

|Log Message:
|Return EOVERFLOW like FreeBSD does if the buffer size exceeds INT_MAX
|(well FreeBSD documents INT_MAX + 1, but in the code it is INT_MAX).

hm, ok, yes S-nail without the fix doesn't print the Message-Id:
even on FreeBSD, but at least the buffer is terminated at [0] and
thus no binary garbage is printed when the buffer is printed.
Ciao,

--steffen

[Christos Zoulas]

Post by Steffen Nurpmeso
|Return EOVERFLOW like FreeBSD does if the buffer size exceeds INT_MAX
|(well FreeBSD documents INT_MAX + 1, but in the code it is INT_MAX).
even on FreeBSD, but at least the buffer is terminated at [0] and
thus no binary garbage is printed when the buffer is printed.
Ciao,

I am not sure if it is a good idea to touch the buffer when returning
an error.

christos

[Steffen Nurpmeso]

***@astron.com (Christos Zoulas) wrote:
|In article <20140929201300.a8ENWnx3%***@yandex.com>,
|Steffen Nurpmeso <***@yandex.com> wrote:
|>And, finally:
|>
|>|Log Message:
|>|Return EOVERFLOW like FreeBSD does if the buffer size exceeds INT_MAX
|>|(well FreeBSD documents INT_MAX + 1, but in the code it is INT_MAX).
|>
|>hm, ok, yes S-nail without the fix doesn't print the Message-Id:
|>even on FreeBSD, but at least the buffer is terminated at [0] and
|>thus no binary garbage is printed when the buffer is printed.
|>Ciao,
|
|I am not sure if it is a good idea to touch the buffer when returning
|an error.

What if EOVERFLOW occurs because buffer space runs out after
processing say 1000 bytes. And here there is buffer space
available: the buffer is too large. What a shitty error
condition, imho.

I just looked at OpenBSD now -- they clamp to INT_MAX if >INT_MAX,
which is a much more sane approach that i would also have
implemented; of course i personally would NEVER implement grazy
shit like STD I/O, i have implemented nice context-carrier classes
for this problem, which is the only sane approach; and now that
i know Plan9 i'm no longer alone with this good stuff :-). Hmm.
(Haha).

But the problem is that the return values are int whereas the
buffer size is size_t, the prototypes should at least go for
a ssize_t return but that of course is not ISO C.
But then again: ISO C is plain grazy!
How often do i see patches flying by where memory of an object is
temporarily copied to aligned data on the stack only to be able to
not run into undefined behaviour. Plain overengineered shit.
Of course you can terminate a buffer that is UI32_MAX bytes.

--steffen

[David Holland]

Post by Steffen Nurpmeso
I just looked at OpenBSD now -- they clamp to INT_MAX if >INT_MAX,
which is a much more sane approach that i would also have
implemented; of course i personally would NEVER implement grazy
shit like STD I/O, i have implemented nice context-carrier classes
for this problem, which is the only sane approach; and now that
i know Plan9 i'm no longer alone with this good stuff :-). Hmm.
(Haha).

Problem with that is that there's a lot of code that checks if the
return value is less than the max size to see if the string (didn't)
get truncated. So if you clamp the max size, this logic breaks.

anyway, this seems like a theoretical problem - are you really
snprintf'ing into a 2GB buffer?

[Steffen Nurpmeso]

David Holland <dholland-***@netbsd.org> wrote:
|On Mon, Sep 29, 2014 at 09:49:18PM +0200, Steffen Nurpmeso wrote:
|> I just looked at OpenBSD now -- they clamp to INT_MAX if >INT_MAX,
|> which is a much more sane approach that i would also have

|Problem with that is that there's a lot of code that checks if the
|return value is less than the max size to see if the string (didn't)
|get truncated. So if you clamp the max size, this logic breaks.

Does the function stop processing and return EOVERFLOW if the
result would exceed that "int" that is used for the return value,
i.e. the actual buffer size?
If so, i think you are creating an artificial problem.

|anyway, this seems like a theoretical problem - are you really
|snprintf'ing into a 2GB buffer?

I think you're right with that.
(A different I/O would be fine, but i cannot imagine it will get
around the unsigned argument / signed return discrepancy.)

P.S.: this snippet flew by when i (git) grep'ed in usr.bin for
snprintf(3) usage (gzip/gzip.c) and isn't multibyte safe.
Shall i open a bug report?

/* Add (usually) .gz to filename */
if ((size_t)snprintf(outfile, outsize, "%s%s",
file, suffixes[0].zipped) >= outsize)
memcpy(outfile + outsize - suffixes[0].ziplen - 1,
suffixes[0].zipped, suffixes[0].ziplen + 1);

--steffen

[David Holland]

Post by Steffen Nurpmeso
|> I just looked at OpenBSD now -- they clamp to INT_MAX if >INT_MAX,
|> which is a much more sane approach that i would also have
|Problem with that is that there's a lot of code that checks if the
|return value is less than the max size to see if the string (didn't)
|get truncated. So if you clamp the max size, this logic breaks.
Does the function stop processing and return EOVERFLOW if the
result would exceed that "int" that is used for the return value,
i.e. the actual buffer size?
If so, i think you are creating an artificial problem.

It does not:

snprintf(), vsnprintf(), and vsnprintf_ss() will write at most size-1 of
the characters printed into the output string (the size'th character then
gets the terminating `\0'); if the return value is greater than or equal
to the size argument, the string was too short and some of the printed
characters were discarded.

Post by Steffen Nurpmeso
P.S.: this snippet flew by when i (git) grep'ed in usr.bin for
snprintf(3) usage (gzip/gzip.c) and isn't multibyte safe.
Shall i open a bug report?
/* Add (usually) .gz to filename */
if ((size_t)snprintf(outfile, outsize, "%s%s",
file, suffixes[0].zipped) >= outsize)
memcpy(outfile + outsize - suffixes[0].ziplen - 1,
suffixes[0].zipped, suffixes[0].ziplen + 1);

Other than it should check for error in case of EILSEQ, that's more or
less the normal idiom...