From patchwork Mon Nov 21 20:37:13 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella X-Patchwork-Id: 83297 Delivered-To: patch@linaro.org Received: by 10.140.97.165 with SMTP id m34csp1754936qge; Mon, 21 Nov 2016 12:37:40 -0800 (PST) X-Received: by 10.13.248.193 with SMTP id i184mr16327049ywf.265.1479760660187; Mon, 21 Nov 2016 12:37:40 -0800 (PST) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id n7si5168438ywf.282.2016.11.21.12.37.39 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 21 Nov 2016 12:37:40 -0800 (PST) Received-SPF: pass (google.com: domain of libc-alpha-return-74998-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@sourceware.org; spf=pass (google.com: domain of libc-alpha-return-74998-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=libc-alpha-return-74998-patch=linaro.org@sourceware.org; dmarc=fail (p=NONE dis=NONE) header.from=linaro.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:subject:to:references:cc:from:message-id:date :mime-version:in-reply-to:content-type :content-transfer-encoding; q=dns; s=default; b=malFmh0In961XN5r Du7xFoZkOXpxr82VrjWVtlpfPtOFjz4uugxV8HHPKz+M7DeHnl8QDHnthulP2lbj 8AkGwk/k7aw2m4UKoYUYo95EcItMmFaFT5QoUErXAuUcgvJSVXy1+gZ/X0+gF2j+ c4YGD/paoqi8Ie8XcdKxFJxhyvw= 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:subject:to:references:cc:from:message-id:date :mime-version:in-reply-to:content-type :content-transfer-encoding; s=default; bh=guSUK7iTyruL7yHiXLIPI6 z6Q6I=; b=nrUCEkWXUi6wnsNsRdec8rQxyehcCSg0FCFcI8oEckKLz1VFKEpCvm EN5R3U9YCAHQJSmKZuB13z4H0MXkymvsq3MOYsfU8Mlgv03TZ/XrEcKjvQah3GYt 7J/Ja8BTGFLEIeZhGPHTCSDtKFUzmYAOkonHAxw/rQIwgtAC2VFkw= Received: (qmail 78360 invoked by alias); 21 Nov 2016 20:37:30 -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 78344 invoked by uid 89); 21 Nov 2016 20:37:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=41, 20, 41, 19, 4119, 4120 X-HELO: mail-ua0-f175.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=FJ9aqJ1y9TTYNTe5DYrDAUU85FpbRthGleC0WNd5VK4=; b=GzRxGQDSAfzCm5NjdIfyqxGStAvfdMfIx6IeOPYUdMgzR9o7f85TTxfvM2WwWQn41p v1Hf2uRmAJuhysiEMC4KqXZKRYX49zuxQyxADtk/izfal+3pSylHHtsuB+nrra/CFwns IhBVeYbV2nwkqfed6tdISD7VKy696hLKmnZzrD2vlnQntqUClItWBvT14Ik6KFAQ97Xc u8Zwr7RlHFwEJ7FxRTiBrphV6ey54JpDAe7w94xbhUQtThaIkV7DZ36f9iBuKoznePza gDimR/r6YQiglSqbiogoroRqRhEyTqkjQaRJPkDMGzvR2S4qBEJaDasObZcLW35pTEMx jedg== X-Gm-Message-State: AKaTC02MYPLcApTFru8sRouTh9/ZyyhFaGKpGLtJXB5Se9J8VjFhqe/6P0t0NPpXiIt/2x30 X-Received: by 10.159.34.41 with SMTP id 38mr8758214uad.160.1479760638062; Mon, 21 Nov 2016 12:37:18 -0800 (PST) Subject: Re: [PATCH v2] Fix writes past the allocated array bounds in execvpe (BZ#20847) To: Andreas Schwab References: <1479755377-12442-1-git-send-email-adhemerval.zanella@linaro.org> <87vavg7fx1.fsf@linux-m68k.org> Cc: libc-alpha@sourceware.org From: Adhemerval Zanella Message-ID: Date: Mon, 21 Nov 2016 18:37:13 -0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <87vavg7fx1.fsf@linux-m68k.org> On 21/11/2016 18:10, Andreas Schwab wrote: > On Nov 21 2016, Adhemerval Zanella wrote: > >> diff --git a/posix/execvpe.c b/posix/execvpe.c >> index d933f9c..96a12bf5 100644 >> --- a/posix/execvpe.c >> +++ b/posix/execvpe.c >> @@ -41,19 +41,20 @@ maybe_script_execute (const char *file, char *const argv[], char *const envp[]) >> ptrdiff_t argc = 0; >> while (argv[argc++] != NULL) >> { >> - if (argc == INT_MAX - 1) >> + if (argc == INT_MAX - 2) >> { >> errno = E2BIG; >> return; >> } >> } >> >> - /* Construct an argument list for the shell. */ >> - char *new_argv[argc + 1]; >> + /* Construct an argument list for the shell. It will contain at minimum 3 >> + arguments (current shell, script, and an ending NULL. */ >> + char *new_argv[argc + 2]; > > The array is now always one element too big, unless execvpe was called > with argv[0] == NULL. You are right, I keep forgetting 'argc' in this context also contains the script name itself. The memcpy adjustment is indeed the only fix required for this part: With this change are you ok to push this in? diff --git a/posix/execvpe.c b/posix/execvpe.c index d933f9c..7cdb06a 100644 --- a/posix/execvpe.c +++ b/posix/execvpe.c @@ -48,12 +48,13 @@ maybe_script_execute (const char *file, char *const argv[], char *const envp[]) } } - /* Construct an argument list for the shell. */ + /* Construct an argument list for the shell. It will contain at minimum 3 + arguments (current shell, script, and an ending NULL. */ char *new_argv[argc + 1]; new_argv[0] = (char *) _PATH_BSHELL; new_argv[1] = (char *) file; if (argc > 1) - memcpy (new_argv + 2, argv + 1, argc * sizeof(char *)); + memcpy (new_argv + 2, argv + 1, (argc - 1) * sizeof(char *)); else new_argv[2] = NULL;