strxcpyx: don't overflow dest on strpcpyf truncate
authorVito Caputo <vcaputo@pengaru.com>
Sun, 2 Jul 2017 09:01:59 +0000 (02:01 -0700)
committerVito Caputo <vcaputo@pengaru.com>
Mon, 3 Jul 2017 13:45:53 +0000 (06:45 -0700)
When vsnprintf() truncated output, dest was advanced by the entire
size of dest leaving it just past the end.  Then the fall-through \0
termination scribbled one past the end.  The explicit null termination
is not necessary since vsnprintf() always includes the terminator even
when truncated.

Additionally these functions encourage calling with zero-length sizes,
while assuming non-zero sizes with potential buffer overflows.

Simply short-circuit the relevant functions when size == 0.

Fixes https://github.com/systemd/systemd/issues/6252

src/basic/strxcpyx.c

index aaf11d2..5e20984 100644 (file)
 
 /*
  * Concatenates/copies strings. In any case, terminates in all cases
- * with '\0' * and moves the @dest pointer forward to the added '\0'.
- * Returns the * remaining size, and 0 if the string was truncated.
+ * with '\0' and moves the @dest pointer forward to the added '\0'.
+ * Returns the remaining size, and 0 if the string was truncated.
+ *
+ * Due to the intended usage, these helpers silently noop invocations
+ * having zero size.  This is technically an exception to the above
+ * statement "terminates in all cases".  It's unexpected for such calls to
+ * occur outside of a loop where this is the preferred behavior.
  */
 
 #include <stdarg.h>
@@ -32,6 +37,9 @@
 size_t strpcpy(char **dest, size_t size, const char *src) {
         size_t len;
 
+        if (size == 0)
+                return 0;
+
         len = strlen(src);
         if (len >= size) {
                 if (size > 1)
@@ -51,17 +59,18 @@ size_t strpcpyf(char **dest, size_t size, const char *src, ...) {
         va_list va;
         int i;
 
+        if (size == 0)
+                return 0;
+
         va_start(va, src);
         i = vsnprintf(*dest, size, src, va);
         if (i < (int)size) {
                 *dest += i;
                 size -= i;
         } else {
-                *dest += size;
                 size = 0;
         }
         va_end(va);
-        *dest[0] = '\0';
         return size;
 }