From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-3.1 required=3.0 tests=AWL,BAYES_00,BODY_8BITS, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_PASS, SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by dcvr.yhbt.net (Postfix) with ESMTP id E43031F9FC for ; Fri, 26 Mar 2021 05:30:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229871AbhCZF25 (ORCPT ); Fri, 26 Mar 2021 01:28:57 -0400 Received: from cloud.peff.net ([104.130.231.41]:49522 "EHLO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229463AbhCZF2d (ORCPT ); Fri, 26 Mar 2021 01:28:33 -0400 Received: (qmail 28593 invoked by uid 109); 26 Mar 2021 05:28:33 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Fri, 26 Mar 2021 05:28:33 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 7939 invoked by uid 111); 26 Mar 2021 05:28:34 -0000 Received: from coredump.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Fri, 26 Mar 2021 01:28:34 -0400 Authentication-Results: peff.net; auth=none Date: Fri, 26 Mar 2021 01:28:31 -0400 From: Jeff King To: =?utf-8?B?zqPPhM6xz43Pgc6/z4Igzp3PhM6tzr3PhM6/z4I=?= Cc: git , =?utf-8?B?zqPPhM6xz43Pgc6/z4Igzp3PhM6tzr3PhM6/z4I=?= , Junio C Hamano , Bagas Sanjaya , Stavros Ntentos <133706+stdedos@users.noreply.github.com> Subject: Re: [RFC PATCH v1 1/2] pathspec: warn: long and short forms are incompatible Message-ID: References: <20210326024005.26962-1-stdedos+git@gmail.com> <20210326024005.26962-2-stdedos+git@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20210326024005.26962-2-stdedos+git@gmail.com> Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org On Fri, Mar 26, 2021 at 04:40:04AM +0200, Σταύρος Ντέντος wrote: > +void check_mishandled_exclude(const char *entry) { > + size_t entry_len = strlen(entry); > + char flags[entry_len]; > + char path[entry_len]; We avoid using variable-length arrays in our codebase. For one thing, they were not historically supported by all platforms (we are slowly using more C99 features, but we are introducing them slowly and intentionally). But two, they are limited in size and the failure mode is not graceful. If "entry" is larger than the available stack, then we'll get a segfault with no option to handle it better. > + if (sscanf(entry, ":!(%4096[^)])%4096s", &flags, &path) != 2) { > + return; We also generally avoid using scanf, because it's error-prone. The "4096" is scary here, but I don't _think_ it's a buffer overflow, because "path" is already the same size as "entry" (not including the NUL terminator, but that is negated by the fact that we'll have skipped at least ":!"). Is this "%4096[^)]" actually valid? I don't think scanf understands regular expressions. We'd want to avoid making an extra copy of the string anyway. So you'd probably want to just parse left-to-right in the original string, like: const char *p = entry; /* skip past stuff we know must be there */ if (!skip_prefix(p, ":!(", &p)) return; /* this checks count_slashes() > 0 in the flags section, though I'm * not sure I understand what that is looking for... */ for (; *p && *p != ')'; p++) { if (*p == '/') return; } if (*p++ != ')') return; /* now p is pointing at "path", though we don't seem to do anything * with it... */ -Peff