Not logged in
Log in now
Create an account
Subscribe to LWN
LWN.net Weekly Edition for December 5, 2013
Deadline scheduling: coming soon?
LWN.net Weekly Edition for November 27, 2013
ACPI for ARM?
LWN.net Weekly Edition for November 21, 2013
But if this happened, and it had never occurred to you that it -could- happen, better that the program crashes than to do something unpredictable.
On the other had, if it did occur to you, you'd have put a check in.
The ups and downs of strlcpy()
Posted Jul 21, 2012 0:27 UTC (Sat) by dlang (✭ supporter ✭, #313)
not everything is so critical that being protected against every possible exploit is the most important thing.
If you are running a game server, doing something "unexpected" for someone who puts in a 200 character name may be preferred to shutting down the game for everyone else.
It also depends on what the worst 'unexpected' think that it could do is. If it's "gain a shell prompt on the server" it's a lot more significant than put the wrong thing in a high score list"
You are exibiting the biggest failing of security people, mistaking security as an end in and of itself as opposed to being a tool to support everything else. As a security person myself, it's a tendency that I trip over regularly in myself. Everything has a cost and sometimes the cost of something is higher than the thing it's preventing.
Posted Jul 23, 2012 20:12 UTC (Mon) by mmeehan (subscriber, #72524)
I like how glib handles this (G_DISABLE_CHECKS and G_DISABLE_ASSERT). Normally function calls are made safe with g_assert and g_return_if_fail macros, but if you'd like to be unsafe (and slightly faster), you can disable them with compile-time options. By default you're safer security-wise, but you can remove the brakes if you desire.
Posted Jul 25, 2012 19:13 UTC (Wed) by bronson (subscriber, #4806)
It would be nice if safety was always the default. Alas, libc (as standardized) only started thinking this way relatively recently.
Posted Jul 25, 2012 20:58 UTC (Wed) by smurf (subscriber, #17840)
The fundamental point is that you cannot know beforehand whether looking at the string twice is a [performance] problem, whether truncating (with or without fixing incomplete UTF-8 codes) is better than not starting to fill the buffer in the first place, whether calling abort() is a good idea (I'd say that if you are a library, it almost never is), whether to return something negative or the new length or the source length or …, and a host of related questions, all of which do not lend themselves to consensus answers. As this discussion shows quite clearly, IMHO.
My point is that, with the sole exception of leaving the destination buffer undisturbed when the source won't fit, any of the aforementioned behaviors can be implemented with a reasonably-trivial O(1) wrapper around strlcpy(). Therefore, keeping strlcpy() out of libc is … kindof stupid. Again, IMHO.
Instead, people are told to use strncpy(). Which they'll do incorrectly. Let's face it, running off the end of a string into la-la land is always worse than truncating it.
Posted Jul 26, 2012 1:28 UTC (Thu) by nybble41 (subscriber, #55106)
typedef size_t (*strxcpy_handler_t)(char *dst, const char *src, size_t size, void *data);
size_t strxcpy(char *dst, const char *src, size_t size, strxcpy_handler_t overflow_fn, void *overflow_data)
const char *q;
for (p = dst, q = src; *q; ++p, ++q)
if ((p - dst) >= size)
return overflow_fn(dst, src, size, overflow_data);
*p = *q;
/* get here only if strlen(src) < size */
*p++ = '\0';
return (p - dst);
size_t strxcpy_truncate(char *dst, const char *src, size_t size, void *data)
if (size <= 0) abort();
dst[size - 1] = '\0';
return size + strlen(src + size);
size_t strxcpy_abort(char *dst, const char *src, size_t size, void *data)
if (strxcpy(dst, src, dst_size, strxcpy_truncate, NULL) >= dst_size) ...;
(void)strxcpy(dst, src, dst_size, strxcpy_abort, NULL);
(void)strxcpy(dst, src, dst_size, strxcpy_subst, "(input too long)");
/* ... */
Posted Jul 26, 2012 8:53 UTC (Thu) by renox (subscriber, #23785)
That said, one size doesn't fit all so having different function is reasonable, the biggest issue is that there is no sane default behaviour..
Posted Jul 26, 2012 16:27 UTC (Thu) by nybble41 (subscriber, #55106)
The strxcpy function isn't just a wrapper; it does all of the real work. The strxcpy_abort, strxcpy_truncate functions only run when an overflow condition is detected. This allows you to substitute your own preferred method of error-handling.
This is actually rather similar to the way exceptions are handled in Common Lisp or Scheme programs, except that the Lisp version would use dynamic variables rather than explicit arguments for the handler code, which results in less cluttered code.
(define (default-error-handler error-value) (abort))
(define current-error-handler (make-parameter default-error-handler))
(... (if (ok? var) var ((current-error-handler) var)) ... ))
; aborts on error
; evaluates to #t on success, or #f on error
(parameterize ([current-error-handler (lambda _ (return #f))])
; uses "value" in place of var on error
(parameterize ([current-error-handler (lambda _ value)])
Scheme-style parameters are attached to the current continuation, meaning that they're not only thread-safe, but that the bindings only affect the inner dynamic scope of the (parameterize) form, even in exceptional cases such as non-local return (like the middle example above) and even re-entry into a dynamic scope which was previously exited.
Posted Jul 25, 2012 12:30 UTC (Wed) by NAR (subscriber, #1313)
Posted Jul 26, 2012 11:51 UTC (Thu) by hppnq (guest, #14462)
Copyright © 2013, Eklektix, Inc.
Comments and public postings are copyrighted by their creators.
Linux is a registered trademark of Linus Torvalds