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.

(cherry picked from commit ea7d8b95e2fcb81f68b04ed7787a3dbda023991a)

diff --git a/ChangeLog b/ChangeLog
index 896b564707fcc275c001bea97c2604124380e001..e82ba7d174711691733dd893a9f7a8eb53cf2454 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -8,6 +8,10 @@
        * inet/getnetgrent_r.c (get_nonempty_val): New function.
        (nscd_getnetgrent): Use it.
 
+       [BZ #16760]
+       * nscd/netgroupcache.c (addgetnetgrentX): Use memmove instead
+       of stpcpy.
+
 2015-11-24  Andreas Schwab  <schwab@suse.de>
 
        [BZ #17062]

diff --git a/NEWS b/NEWS
index 6f295a201a12d7ee3839d0a2de553723b6b6c4ea..2972c4a5ea75ef344ed6127fb783d589d4aeca6d 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -9,9 +9,10 @@ Version 2.19.1
 
 * The following bugs are resolved with this release:
 
-  15946, 16545, 16574, 16623, 16657, 16695, 16743, 16758, 16759, 16878,
-  16882, 16885, 16916, 16932, 16943, 16958, 17048, 17062, 17069, 17079,
-  17137, 17153, 17213, 17263, 17269, 17325, 17555, 18007, 18032, 18287.
+  15946, 16545, 16574, 16623, 16657, 16695, 16743, 16758, 16759, 16760,
+  16878, 16882, 16885, 16916, 16932, 16943, 16958, 17048, 17062, 17069,
+  17079, 17137, 17153, 17213, 17263, 17269, 17325, 17555, 18007, 18032,
+  18287.
 
 * A buffer overflow in gethostbyname_r and related functions performing DNS
   requests has been fixed.  If the NSS functions were called with a

diff --git a/nscd/netgroupcache.c b/nscd/netgroupcache.c
index 8c619eab3158d13f160639db11a5bdda69143a15..c61d10b1701725304fb54a5b7427c0561ce16b30 100644 (file)
--- a/nscd/netgroupcache.c
+++ b/nscd/netgroupcache.c
@@ -211,6 +211,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)
                              {
@@ -228,9 +232,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)
@@ -264,9 +265,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;
                          }