diff mbox

[COMMITTED,2.19] Avoid overlapping addresses to stpcpy calls in nscd (BZ #16760)

Message ID 1450634134-13084-3-git-send-email-aurelien@aurel32.net
State Committed
Headers

Commit Message

Aurelien Jarno Dec. 20, 2015, 5:55 p.m. UTC 
  From: Siddhesh Poyarekar <siddhesh@redhat.com>

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)
---
 ChangeLog            |  4 ++++
 NEWS                 |  7 ++++---
 nscd/netgroupcache.c | 16 ++++++++++------
 3 files changed, 18 insertions(+), 9 deletions(-)
diff mbox

Patch

diff --git a/ChangeLog b/ChangeLog
index 896b564..e82ba7d 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 6f295a2..2972c4a 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 8c619ea..c61d10b 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;
 			  }