From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_MSPIKE_H2 autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 18003 invoked from network); 10 Mar 2023 16:59:24 -0000 Received: from second.openwall.net (193.110.157.125) by inbox.vuxu.org with ESMTPUTF8; 10 Mar 2023 16:59:24 -0000 Received: (qmail 11487 invoked by uid 550); 10 Mar 2023 16:59:21 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 11450 invoked from network); 10 Mar 2023 16:59:20 -0000 DKIM-Filter: OpenDKIM Filter v2.11.0 mail.ispras.ru 2A5E340D403D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ispras.ru; s=default; t=1678467549; bh=jusjLQxe87WwaCLQB9SmGBoKf8cl843NeGikR31tbrw=; h=Date:From:To:Subject:Reply-To:In-Reply-To:References:From; b=rDiINh/7AW7/OuNo2+oM03JIwj3sna91jtccifJNNOVozAIgvz2TOWziLwqLzejgf aE/VQpf/4339+Ri5xxnyj5pCOZ7H4u59X8Pf8Pkld0wmZzFjzdSa+8jIAufqA9ozL2 5GhqRt9UC5ZAc99c/15WLQ39qWHMAvmzfyrdgRaA= MIME-Version: 1.0 Date: Fri, 10 Mar 2023 19:59:09 +0300 From: Alexey Izbyshev To: musl@lists.openwall.com Mail-Followup-To: musl@lists.openwall.com In-Reply-To: References: <20230310161022.108593-1-izbyshev@ispras.ru> User-Agent: Roundcube Webmail/1.4.4 Message-ID: <18659a22996ae335c466e9f92d0dd84b@ispras.ru> X-Sender: izbyshev@ispras.ru Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [musl] [PATCH] getopt: fix null pointer arithmetic ub On 2023-03-10 19:28, Alexander Monakov wrote: > Hi, > > On Fri, 10 Mar 2023, Alexey Izbyshev wrote: > >> When an option that requires an argument is the last character of >> argv[argc-1], getopt computes argv[argc] + optpos. While optpos >> is always zero in this case, adding it to null pointer is still >> undefined. >> --- >> src/misc/getopt.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/src/misc/getopt.c b/src/misc/getopt.c >> index c3f66995..af12973a 100644 >> --- a/src/misc/getopt.c >> +++ b/src/misc/getopt.c >> @@ -87,7 +87,8 @@ int getopt(int argc, char * const argv[], const char >> *optstring) >> if (optstring[i] == ':') { >> optarg = 0; >> if (optstring[i+1] != ':' || optpos) { >> - optarg = argv[optind++] + optpos; >> + optarg = argv[optind++]; >> + if (optarg) optarg += optpos; > > Can this be written as 'if (optpos) optarg += optpos;' instead? That > will be > folded back into plain addition by the compiler. > Yes, "if (optpos) ..." is actually what I initially wrote before changing it to the submitted variant. I'm fine with changing it back; thanks for the codegen check. > (also (unlike the quoted variant) would allow undefined behavior > instrumentation to catch attempted NULL pointer arithmetic) > Yes, a good point too. Alexey