From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id CC16E385840F for ; Fri, 24 Feb 2023 10:58:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org CC16E385840F Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1677236290; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=3jm0d9bSx+fw6HTy5Y1UJeTVXArorC2+ntZI7OR2vl0=; b=JOAm+iN5bBbSOLKhpKudQQPfYa9iOYnhZc/ZJwL1L3lKh8Y3lNKYo6eldSvyFFzrHIblUq SQWHN8msY75OWjHsSo9iIw8Re5L4/WqrXKvKs6DW36aCC28jc2WDYG7fldg5/6oQVnbcvh cWNm8C1zptF4IE8uusddLXartMzj9Mo= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-438-Y4vQXixDOemQ7Cu6j6omkQ-1; Fri, 24 Feb 2023 05:58:07 -0500 X-MC-Unique: Y4vQXixDOemQ7Cu6j6omkQ-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id C849595D606; Fri, 24 Feb 2023 10:58:06 +0000 (UTC) Received: from oldenburg.str.redhat.com (unknown [10.2.16.16]) by smtp.corp.redhat.com (Postfix) with ESMTPS id D65CB40C1423; Fri, 24 Feb 2023 10:58:05 +0000 (UTC) From: Florian Weimer To: Szabolcs Nagy Cc: Adhemerval Zanella Netto , "H.J. Lu" , Noah Goldstein , libc-alpha@sourceware.org Subject: Re: [PATCH v2] string: Fix OOB read on generic strncmp References: <20230222163159.3446687-1-adhemerval.zanella@linaro.org> Date: Fri, 24 Feb 2023 11:58:04 +0100 In-Reply-To: (Szabolcs Nagy's message of "Fri, 24 Feb 2023 10:19:15 +0000") Message-ID: <87ilfrmitf.fsf@oldenburg.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.2 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-4.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: * Szabolcs Nagy: > The 02/23/2023 15:15, Adhemerval Zanella Netto wrote: >> Noah has brought to my attention that he tried to add similar tests, >> but they were rejected by strncmp string must be null-terminated [1]. >> >> The working drafts for C standard I have access (n1256.pdf for C99 and >> n3047.pdf for c2x) do not say possibly null-terminated array (as some >> stackoverflow answer state [2]) they refer only as array. So I tend >> to follow Florian understanding that strncmp inputs should be NULL >> terminated. > > c11 draft is n1570.pdf > https://open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf > > i dont understand what extension Florian is talking about. > (i think that was about strcmp not strncmp) > > c11 and c23 are clear that strncmp args may *not* be null-terminated > so i think we should be careful not to overread. It's common to use strncmp as a starts-with predicate, like this: strncmp (s, "prefix", 5) This requires that reading stops at the first null byte. C11 wording makes this kind of usage inadvisable because it treats the inputs as arrays, and this means that mean that implementations could read past the null byte. But that doesn't match current programming practice. The strnlen function has the same problem if you want to use it to limit readhead, e.g. in sscanf to fix bug 17577. (POSIX also speaks of an array argument.) In stdio-common/Xprintf_buffer_puts_1.c, we already use it to avoid a similar performance glitch. It's not the first such uses, there is already a similar call (with similar rationale) in string/strcasestr.c, and for wcsnlen in stdio-common/vfprintf-process-arg.c. I think we should support all these as extensions. The alternative would be to add new functions that stop reading after the first null byte (particularly for the strnlen optimization). Unfortunately, the last time I brought this up, the manual was updated to double down on the array interpretation. I think this was the result: commit 2cc4b9cce51643ec299e97450ccde4deeecfb083 Author: Paul Eggert Date: Fri Dec 4 08:27:14 2015 -0800 Consistency about byte vs character in string.texi I think that's just not the right direction for glibc. Thanks, Florian