This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Implement C11 annex K?


On Fri, Aug 15, 2014 at 06:00:59PM -0400, David A. Wheeler wrote:
> David A. Wheeler (me) said:
> > > snprintf is overcomplicated for simple string copying (a common operation)
> 
> On Fri, 15 Aug 2014 12:39:12 -0400, Rich Felker <dalias@libc.org> wrote:
> > One extra argument of "%s" is not really over-complicated. I'll grant
> > one thing: having the return value with type int rather than size_t is
> > mildly problematic, in that snprintf can't be fully equivalent to
> > strlcpy or strncpy_s, but in cases where the input string is >2GB
> > you'd probably prefer to have an error rather than processing an
> > unbounded amount of input, as it's almost surely malicious. If your
> > data is really potentially that large, C strings are not the proper
> > format for it.
> 
> I agree snprintf's return type (int) is problematic compared to strlcpy/strlcat (size_t).
> 
> Your comments here are a little oblique; let me explain what *I* think the return type problem is
> (I'm *guessing* we mean the same thing). snprintf returns the number of chars it *WOULD* have written,
> not what it ACTUALLY wrote, so the "obvious" way to use it for a string copy looks like this:
>  char buf[100];
>  if (snprintf(buf, sizeof(buf), "%s", src) >= sizeof(buf)) {  // FAIL
>    // handle truncation
>  }
> But wait, "int" is signed!!  Thus, if you give it a source with a length a little longer than INT_MAX,
> then the int will (probably) wrap;

No, it's not permitted to wrap, at least not by POSIX. It fails with
EOVERFLOW and returns a negative value. (ISO C is less explicit here,
but it says "Thus, the null-terminated output has been completely
written if and only if the returned value is nonnegative and less than
n." which makes it clear that you can check with such a test.)

> snprintf will return a NEGATIVE number on
> truncation, which will always less less than a sizeof, and thus the "handle truncation" is *skipped*

This can happen only if size_t has integer conversion rank lower than
int, which doesn't happen in practice and would only happen on a
pathological implementation. In reality, size_t has greater integer
conversion rank, and thus any negative int is implicitly converted to
a value larger than INT_MAX.

> even when truncation has occurred. INT_MAX is only guaranteed to be +32767, so this
> can happen easily on small systems (think "internet of things"), but even on some
> 32-bit systems this can happen.
> 
> This can be handled, but you can't just use a simple cast; errors also return negatives from snprintf.

A simple cast to uintmax_t will ensure the desired behavior of
catching both truncation and errors simply by >= comparison against
the buffer size.

> Why do I need this complexity for trivial copy and concatenation again?
> It might be better for it to just not copy so much (that's the point of rsize_t in annex K),
> but clearly returning a value that is unlikely to be interpreted correctly is a problem.

I consider this a purely theoretical problem, since there are no
systems anyone is interested in where size_t has rank lower than int.

> > ... concatenation without storing a "current position" between
> > operations is bad anyway (see: Schlemiel the painter).
> 
> I completely disagree; this is simply another trade-off.
> 
> In many cases you *should* hire Schlemiel.  Tracking current position

OK you just got your email a tweet. :)

> > > Many people who are trying to write secure software in C (such as the OpenBSD and Microsoft folks)
> > > are increasingly trying to *stop* the use of traditional functions like strcpy and strncpy,
> > 
> > I reject the claim that MS's interest in stopping the use of these
> > traditional functions has anything to do with security.
> 
> I disagree, but that's not my point.  My point is that buffer overflows are
> a serious problem; we need widely-available functions for common cases that are
> easy to use and make them much less likely.  Also, strlcpy/strlcat were created by OpenBSD
> to counter buffer overflows, so clearly it's not just Microsoft that see a need for
> standard easy-to-use functions to counter buffer overflows in fixed-length buffers.

Oh, I believe the OpenBSD ones are actually about security. They did
the right thing, made the "secure" functions have simple signatures
that are easy to use and hard to get wrong, and only "replaced" the
functions that really needed replacement. I just don't believe it for
the MS ones.

Rich


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]