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?


"David A. Wheeler" <dwheeler@dwheeler.com> writes:

> 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;
> snprintf will return a NEGATIVE number on truncation,

I certainly hope not.  snprintf should decline to do any formatting at all
in that case and, yes, return a negative number, but because it failed
entirely.

The above code does not check for snprintf failure, and hence is not
really correct snprintf code.  Truncation and failure have to be checked
independently.  Yes, that means that you can't use snprintf to format
strings longer than INT_MAX.  I'm quite dubious that this is a serious
issue in all but very rare code, and that code can use other techniques.

>> ... concatenation without storing a "current position" between
>> operations is bad anyway (see: Schlemiel the painter).

> I completely disagree; this is simply another trade-off.

Right, I agree with this.  I think tracking that current position is, for
most code, representative of one of the bad habits of C programmers:
micro-optimizing code that no one is ever going to care about being fast.
Yes, the computer has to do more work to re-find the end of the string on
each operation, but usually no one is ever going to care, and the computer
will always get the correct answer.  If it makes the code any cleaner or
any more secure, one should absolutely take that tradeoff.

However, as Paul rightly points out, the same argument applies to malloc,
realloc, and free, and the asprintf or open_memstream code for
constructing strings is even cleaner and simpler.

> 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.

What generally you get from strlcpy/strlcat is turning buffer overflows
into truncations (which one can then check for, but which a lot of code
does not).  I generally agree with you that this is useful for legacy code
that uses a lot of static buffers.  I also generally agree with Paul that
static buffers are *themselves* a design flaw that should be avoided, and
it's not entirely clear to me that it's a good idea to modify code to add
strlcpy/strlcat and stop there, rather than modifying it to stop using
static buffers entirely.

And, honestly, if I were working on such legacy code, I would be very
tempted to use asprintf into a temporary variable to do whatever I want,
check the total length when finished to see if it will fit, and then
either return an error or memcpy the results into the static buffer.  That
doesn't change the calling contract, but gives you many of the safety
benefits of dynamic allocation.

-- 
Russ Allbery (eagle@eyrie.org)              <http://www.eyrie.org/~eagle/>


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