about summary refs log tree commit diff
diff options
context:
space:
mode:
authorSiddhesh Poyarekar <siddhesh@redhat.com>2014-03-27 19:48:15 +0530
committerAurelien Jarno <aurelien@aurel32.net>2015-12-20 17:43:27 +0100
commitb963026c07a304bcfcf56ad5ee9b4f0797c7d3df (patch)
tree29d7fd373ac338212d0cc724831a0efb04764e61
parent56b2cf5633f90c722b8f4ed257311b23ebed7399 (diff)
downloadglibc-b963026c07a304bcfcf56ad5ee9b4f0797c7d3df.tar.gz
glibc-b963026c07a304bcfcf56ad5ee9b4f0797c7d3df.tar.xz
glibc-b963026c07a304bcfcf56ad5ee9b4f0797c7d3df.zip
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)
-rw-r--r--ChangeLog4
-rw-r--r--NEWS7
-rw-r--r--nscd/netgroupcache.c16
3 files changed, 18 insertions, 9 deletions
diff --git a/ChangeLog b/ChangeLog
index 896b564707..e82ba7d174 100644
--- 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 6f295a201a..2972c4a5ea 100644
--- 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 8c619eab31..c61d10b170 100644
--- 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;
 			  }