From patchwork Thu Mar 27 04:04:06 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Siddhesh Poyarekar X-Patchwork-Id: 307 Return-Path: X-Original-To: siddhesh@wilcox.dreamhost.com Delivered-To: siddhesh@wilcox.dreamhost.com Received: from homiemail-mx20.g.dreamhost.com (caibbdcaaahc.dreamhost.com [208.113.200.72]) by wilcox.dreamhost.com (Postfix) with ESMTP id C98DE36033A for ; Wed, 26 Mar 2014 21:03:34 -0700 (PDT) Received: by homiemail-mx20.g.dreamhost.com (Postfix, from userid 14307373) id 7E3D840BB3972; Wed, 26 Mar 2014 21:03:34 -0700 (PDT) X-Original-To: glibc@patchwork.siddhesh.in Delivered-To: x14307373@homiemail-mx20.g.dreamhost.com Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by homiemail-mx20.g.dreamhost.com (Postfix) with ESMTPS id 59E7B40BB3953 for ; Wed, 26 Mar 2014 21:03:34 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:from:to:subject:message-id:mime-version :content-type; q=dns; s=default; b=x1dyFDJW2lPVm3CTMrKPAl5nuznr6 SHA8uwhNrDYx4DxUt9LWdnar+dV70gsDAhsYZglHPwa75O7QV+rqMaWLxnW7VuL2 NHK2AaLwY1ADyTTrkW97K2JedwTNHNHrzbhvDeqv/w3RCepM0DlemhLjTbvMo9hJ tkyXrGU/fKy0Dg= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:from:to:subject:message-id:mime-version :content-type; s=default; bh=aWW+Su5YgefjlMXi83y4IquvjcM=; b=brd UabGsUbzYx9U+A6PxEux4X/uQWWsr69OFhzEkkmRW6+mt727TGc/lW46XB0I2CkR zhFX1e7j7bxaYIndeKtM2xSGC8KuCqED//yolbb5s9l+hTwnnz1J5IXpTHPkpda5 Og422avxb7x2J72ALNm5pScBPLklP7AmP+b1Qbjw= Received: (qmail 7526 invoked by alias); 27 Mar 2014 04:03:31 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 7510 invoked by uid 89); 27 Mar 2014 04:03:31 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.9 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Date: Thu, 27 Mar 2014 09:34:06 +0530 From: Siddhesh Poyarekar To: libc-alpha@sourceware.org Subject: [PATCH] Avoid overlapping addresses to stpcpy calls in nscd (BZ #16760) Message-ID: <20140327040406.GA26264@spoyarek.pnq.redhat.com> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.22.1-rc1 (2013-10-16) X-DH-Original-To: glibc@patchwork.siddhesh.in 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. Tested x86_64 using various combinations of triplets (including NULL and non-NULL ones) to verify that this works correctly and there are no regressions. Siddhesh [BZ #16760] * nscd/netgroupcache.c (addgetnetgrentX): Use memmove instead of stpcpy. --- nscd/netgroupcache.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/nscd/netgroupcache.c b/nscd/netgroupcache.c index 5d15aa4..9999a1e 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) @@ -259,7 +260,6 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req, : NULL); buffer = newbuf; } - nhost = memcpy (buffer + bufused, nhost ?: "", hostlen); nuser = memcpy ((char *) nhost + hostlen, @@ -269,9 +269,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; }