From patchwork Tue Nov 22 13:28:12 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: 17701 Received: (qmail 13900 invoked by alias); 22 Nov 2016 13:28:29 -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 13888 invoked by uid 89); 22 Nov 2016 13:28: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=no X-HELO: mail-vk0-f41.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:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=uK0VDuvMqjOPwAkKdRnsWH1qMjELQaUulMYJElrXI3M=; b=KY6x2emqvYiFIPv0gfVZVeuYJW7kubkHWtUFwgXY6t6ZDbuTnznugHqvJF3j0fR3W6 2p/gFGelZyKyRigiSZatZ7jv4i4jSboOIrbuUq3A6O+IffbkEzB6zhJgNzXI75AzlbTP QcZ/p61Y3xHgVS6BD7oHdJvhOD1tI0mksH2ZgKyAIS1UlZ2yH8170wy8x7CpuZTUJP3k /ofCSWSYCQ/K++h4c6Is0PhJEbulIrdD+rH0g1YAS2nwU4GNwkQ/dX9mH072tzQuuNBu jltoIVcWNii4xCdh4wjAYWXpyURtwZT3E1fWKnrUWDcMl4j1n1RI3zUKA9BDWWNFifUw S73g== X-Gm-Message-State: AKaTC00qW8YqDIzJDk4x3mKrVI7B+A465Z8Qd+bSYeNXZHmqzSXp2DnkRMVqcH+wSUFI6+Vs X-Received: by 10.31.214.65 with SMTP id n62mr9539446vkg.127.1479821297113; Tue, 22 Nov 2016 05:28:17 -0800 (PST) Subject: Re: [PATCH v2] Fix writes past the allocated array bounds in execvpe (BZ#20847) To: libc-alpha@sourceware.org, Andreas Schwab References: <1479755377-12442-1-git-send-email-adhemerval.zanella@linaro.org> <87vavg7fx1.fsf@linux-m68k.org> <87r3647e8x.fsf@linux-m68k.org> <20161122101759.GA30936@linux.vnet.ibm.com> From: Adhemerval Zanella Message-ID: Date: Tue, 22 Nov 2016 11:28:12 -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: <20161122101759.GA30936@linux.vnet.ibm.com> On 22/11/2016 08:17, Dominik Vogt wrote: > On Mon, Nov 21, 2016 at 09:46:22PM +0100, Andreas Schwab wrote: >> On Nov 21 2016, Adhemerval Zanella wrote: >> >>> With this change are you ok to push this in? >> >> Yes, this is ok. > > No! The patch writes past the array bounds in the else branch. > > Ciao > > Dominik ^_^ ^_^ > Since I made a mistake to push this patch, which I apologize, I think your previous suggestions is indeed the correct one (patch below). Reading again, it indeed seems simpler to just account for arguments and not the final 'NULL' since the 'argv + 1' will indeed ignore the script name. diff --git a/posix/execvpe.c b/posix/execvpe.c index 7cdb06a..cf167d0 100644 --- a/posix/execvpe.c +++ b/posix/execvpe.c @@ -38,8 +38,8 @@ static void maybe_script_execute (const char *file, char *const argv[], char *const envp[]) { - ptrdiff_t argc = 0; - while (argv[argc++] != NULL) + ptrdiff_t argc; + for (argc = 0; argv[argc] != NULL; argc++) { if (argc == INT_MAX - 1) { @@ -50,11 +50,11 @@ maybe_script_execute (const char *file, char *const argv[], char *const envp[]) /* 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]; + char *new_argv[2 + argc]; new_argv[0] = (char *) _PATH_BSHELL; new_argv[1] = (char *) file; if (argc > 1) - memcpy (new_argv + 2, argv + 1, (argc - 1) * sizeof(char *)); + memcpy (new_argv + 2, argv + 1, argc * sizeof(char *)); else new_argv[2] = NULL;