about summary refs log tree commit diff
diff options
context:
space:
mode:
authorSiddhesh Poyarekar <siddhesh@redhat.com>2014-03-27 19:48:15 +0530
committerSiddhesh Poyarekar <siddhesh@redhat.com>2014-03-27 19:48:15 +0530
commitea7d8b95e2fcb81f68b04ed7787a3dbda023991a (patch)
tree20e3018b10da00729cd8a8feec1e2265b173c321
parentdf5b85da90915ce6208ad737807e3d8f2a8fce87 (diff)
downloadglibc-ea7d8b95e2fcb81f68b04ed7787a3dbda023991a.tar.gz
glibc-ea7d8b95e2fcb81f68b04ed7787a3dbda023991a.tar.xz
glibc-ea7d8b95e2fcb81f68b04ed7787a3dbda023991a.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.
-rw-r--r--ChangeLog6
-rw-r--r--NEWS2
-rw-r--r--nscd/netgroupcache.c16
3 files changed, 17 insertions, 7 deletions
diff --git a/ChangeLog b/ChangeLog
index f6d309fbee..7cf7bd1e43 100644
--- 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 895c640c5e..6286681b47 100644
--- 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 5d15aa49f4..820d8234b4 100644
--- 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;
 			  }