From ea7d8b95e2fcb81f68b04ed7787a3dbda023991a Mon Sep 17 00:00:00 2001 From: Siddhesh Poyarekar Date: Thu, 27 Mar 2014 19:48:15 +0530 Subject: 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. --- ChangeLog | 6 ++++++ NEWS | 2 +- nscd/netgroupcache.c | 16 ++++++++++------ 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/ChangeLog b/ChangeLog index f6d309f..7cf7bd1 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2014-03-27 Siddhesh Poyarekar + + [BZ #16760] + * nscd/netgroupcache.c (addgetnetgrentX): Use memmove instead + of stpcpy. + 2014-03-27 Andi Kleen * nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h (lll_lock, diff --git a/NEWS b/NEWS index 895c640..6286681 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 5d15aa4..820d823 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; } -- cgit v1.1