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 Netto X-Patchwork-Id: 17685 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;