From 9e5bd85a5fc5d80cc8706d2b8bac520f2c6a8149 Mon Sep 17 00:00:00 2001
From: Vito Caputo <vcaputo@pengaru.com>
Date: Sun, 2 Jul 2017 02:01:59 -0700
Subject: [PATCH 1/3] strxcpyx: don't overflow dest on strpcpyf truncate

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 | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/src/basic/strxcpyx.c b/src/basic/strxcpyx.c
index aaf11d21f6b..5e2098467b8 100644
--- a/src/basic/strxcpyx.c
+++ b/src/basic/strxcpyx.c
@@ -19,8 +19,13 @@
 
 /*
  * 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;
 }
 

From f91049d5d7d7dc25b574c57236932280a83f79ba Mon Sep 17 00:00:00 2001
From: Vito Caputo <vcaputo@pengaru.com>
Date: Sun, 2 Jul 2017 02:09:06 -0700
Subject: [PATCH 2/3] strxcpyx: assert throughout on non-NULL src/dest

---
 src/basic/strxcpyx.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/src/basic/strxcpyx.c b/src/basic/strxcpyx.c
index 5e2098467b8..c6fbe79647c 100644
--- a/src/basic/strxcpyx.c
+++ b/src/basic/strxcpyx.c
@@ -37,6 +37,9 @@
 size_t strpcpy(char **dest, size_t size, const char *src) {
         size_t len;
 
+        assert(dest);
+        assert(src);
+
         if (size == 0)
                 return 0;
 
@@ -59,6 +62,9 @@ size_t strpcpyf(char **dest, size_t size, const char *src, ...) {
         va_list va;
         int i;
 
+        assert(dest);
+        assert(src);
+
         if (size == 0)
                 return 0;
 
@@ -77,6 +83,9 @@ size_t strpcpyf(char **dest, size_t size, const char *src, ...) {
 size_t strpcpyl(char **dest, size_t size, const char *src, ...) {
         va_list va;
 
+        assert(dest);
+        assert(src);
+
         va_start(va, src);
         do {
                 size = strpcpy(dest, size, src);
@@ -89,6 +98,9 @@ size_t strpcpyl(char **dest, size_t size, const char *src, ...) {
 size_t strscpy(char *dest, size_t size, const char *src) {
         char *s;
 
+        assert(dest);
+        assert(src);
+
         s = dest;
         return strpcpy(&s, size, src);
 }
@@ -97,6 +109,9 @@ size_t strscpyl(char *dest, size_t size, const char *src, ...) {
         va_list va;
         char *s;
 
+        assert(dest);
+        assert(src);
+
         va_start(va, src);
         s = dest;
         do {

From 54d46a789e3add120708621283f3f1302df7fb1d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Sun, 2 Jul 2017 12:37:42 -0400
Subject: [PATCH 3/3] test-strxcpyx: add test for strpcpyf overflow

This fails before 'strxcpyx: don't overflow dest on strpcpyf truncate'.
---
 src/test/test-strxcpyx.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/test/test-strxcpyx.c b/src/test/test-strxcpyx.c
index 9bea7701314..d95945f6b01 100644
--- a/src/test/test-strxcpyx.c
+++ b/src/test/test-strxcpyx.c
@@ -51,6 +51,13 @@ static void test_strpcpyf(void) {
 
         assert_se(streq(target, "space left: 25. foobar"));
         assert_se(space_left == 3);
+
+        /* test overflow */
+        s = target;
+        space_left = strpcpyf(&s, 12, "00 left: %i. ", 999);
+        assert_se(streq(target, "00 left: 99"));
+        assert_se(space_left == 0);
+        assert_se(target[12] == '2');
 }
 
 static void test_strpcpyl(void) {