Avoid overlapping addresses to stpcpy calls in nscd (BZ #16760)

Calls to stpcpy from nscd netgroups code will have overlapping source
and destination when all three values in the returned triplet are
non-NULL and in the expected (host,user,domain) order.  This is seen
in valgrind as:

==3181== Source and destination overlap in stpcpy(0x19973b48, 0x19973b48)
==3181==    at 0x4C2F30A: stpcpy (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==3181==    by 0x12567A: addgetnetgrentX (string3.h:111)
==3181==    by 0x12722D: addgetnetgrent (netgroupcache.c:665)
==3181==    by 0x11114C: nscd_run_worker (connections.c:1338)
==3181==    by 0x4E3C102: start_thread (pthread_create.c:309)
==3181==    by 0x59B81AC: clone (clone.S:111)
==3181==

Fix this by using memmove instead of stpcpy.

diff --git a/ChangeLog b/ChangeLog
index f6d309fbeedfbe21b82ec9d621db59d88185185a..7cf7bd1e43f868db4e2c44b42351a4cc8fdcd6ca 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2014-03-27  Siddhesh Poyarekar  <siddhesh@redhat.com>
+
+       [BZ #16760]
+       * nscd/netgroupcache.c (addgetnetgrentX): Use memmove instead
+       of stpcpy.
+
 2014-03-27  Andi Kleen  <ak@linux.intel.com>
 
        * nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h (lll_lock,

diff --git a/NEWS b/NEWS
index 895c640c5ea1406fa57303f45c2d33fc51536abe..6286681b47a19b28e65121d55d53b16509b4127f 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -12,7 +12,7 @@ Version 2.20
   15347, 15804, 15894, 16002, 16198, 16284, 16357, 16447, 16532, 16545,
   16574, 16599, 16600, 16609, 16610, 16611, 16613, 16623, 16632, 16634,
   16639, 16642, 16649, 16670, 16674, 16677, 16680, 16683, 16689, 16695,
-  16701, 16706, 16707, 16712, 16713, 16714, 16731, 16743, 16758.
+  16701, 16706, 16707, 16712, 16713, 16714, 16731, 16743, 16758, 16760.
 
 * Running the testsuite no longer terminates as soon as a test fails.
   Instead, a file tests.sum (xtests.sum from "make xcheck") is generated,

diff --git a/nscd/netgroupcache.c b/nscd/netgroupcache.c
index 5d15aa49f416bdc11137b6ac47ffdd8a21ac746d..820d8234b40bf219666c6f4dd5477d68c9e7b625 100644 (file)
--- a/nscd/netgroupcache.c
+++ b/nscd/netgroupcache.c
@@ -216,6 +216,10 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req,
                            const char *nuser = data.val.triple.user;
                            const char *ndomain = data.val.triple.domain;
 
+                           size_t hostlen = strlen (nhost ?: "") + 1;
+                           size_t userlen = strlen (nuser ?: "") + 1;
+                           size_t domainlen = strlen (ndomain ?: "") + 1;
+
                            if (nhost == NULL || nuser == NULL || ndomain == NULL
                                || nhost > nuser || nuser > ndomain)
                              {
@@ -233,9 +237,6 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req,
                                     : last + strlen (last) + 1 - buffer);
 
                                /* We have to make temporary copies.  */
-                               size_t hostlen = strlen (nhost ?: "") + 1;
-                               size_t userlen = strlen (nuser ?: "") + 1;
-                               size_t domainlen = strlen (ndomain ?: "") + 1;
                                size_t needed = hostlen + userlen + domainlen;
 
                                if (buflen - req->key_len - bufused < needed)
@@ -269,9 +270,12 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req,
                              }
 
                            char *wp = buffer + buffilled;
-                           wp = stpcpy (wp, nhost) + 1;
-                           wp = stpcpy (wp, nuser) + 1;
-                           wp = stpcpy (wp, ndomain) + 1;
+                           wp = memmove (wp, nhost ?: "", hostlen);
+                           wp += hostlen;
+                           wp = memmove (wp, nuser ?: "", userlen);
+                           wp += userlen;
+                           wp = memmove (wp, ndomain ?: "", domainlen);
+                           wp += domainlen;
                            buffilled = wp - buffer;
                            ++nentries;
                          }