From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oa1-x2d.google.com (mail-oa1-x2d.google.com [IPv6:2001:4860:4864:20::2d]) by sourceware.org (Postfix) with ESMTPS id 0AE3F3858284 for ; Fri, 24 Feb 2023 12:35:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 0AE3F3858284 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org Received: by mail-oa1-x2d.google.com with SMTP id 586e51a60fabf-1720600a5f0so19443912fac.11 for ; Fri, 24 Feb 2023 04:35:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1677242102; h=content-transfer-encoding:in-reply-to:organization:from:references :cc:to:content-language:subject:user-agent:mime-version:date :message-id:from:to:cc:subject:date:message-id:reply-to; bh=dEV5yfGQ1HYjWJFNMicv8nq4J1110rBysYaTe5jH9cM=; b=O/+8mdoXb+QP7jfvOEbI4MbGtsc2hKbcC/lH0DJka1ZYpy41Jtj2bNVlq5T8dmp+k8 hDJNGMTHMqpunhODhYwbj1KBKzeJ9/fpm+PQKygv/Uphopk1uu//lGqvAtvTjtyCyNCS 8YylzO0CNDCpvdEIfoqscvRgP8skaaUtSfjrkgRVlY2MAVTNK0sv/eLAw/qMa9QCt0O3 RIBJpHeXyiHm9DEFGV4X1ry7jJCv/tZpRD8otkz1F0EDSzDTVZGhGiCxkH8M9LBrcDJP XEs8casVUWcmgAzOZorYQLxGThXYYJrCX3YpDKSRN4EjZ40FvrPOdlQpCSkbgD16w0NV aBWw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1677242102; h=content-transfer-encoding:in-reply-to:organization:from:references :cc:to:content-language:subject:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=dEV5yfGQ1HYjWJFNMicv8nq4J1110rBysYaTe5jH9cM=; b=U4Z+C4XHzH1JrPO6G3ykcQXtSPAEYGwpDwMgbN//pmCoQhbQw+SVhNOYbsJRCSyXU3 1+DmQp5SCKxg6feGMX1/BNKa/tkVwAwqiCE6D2MI1DEotb5kXbjFeK18c6hwb8x3r2Fc DUMdAoYDHiBV6RvKyP2yi7fjiP1VlEHRdQXRpPalYpxUMv3rq1MThIlL1SVssl4BZ3mo tBNMObE2vbX0BmG7Umq7IkcX6vedvFCQFQt5jhFii9VE41CLFsad98YlBWOhf0nHLzYm dIMVmElZfBeEG8RkaZwETBhFy/zKfiTbkxvc48FPgSMkaIOYbBp9o0MRF3YIq9PqzfE1 shAw== X-Gm-Message-State: AO0yUKWEsfl5w30aikeW18CSMGTk0p1oK7wBJw/FQz4l9Ifs9dpXDXNb DiFKYsYm14An203sN2mETKOo+A== X-Google-Smtp-Source: AK7set+XLzxLGNhM1B0MenC7lW5eHx+mb4W3KlgKt9OhzqeR/BezKK8aiQRsMwxXkugxaUVB6m82IA== X-Received: by 2002:a05:6870:2393:b0:172:5de5:785 with SMTP id e19-20020a056870239300b001725de50785mr4921366oap.6.1677242101689; Fri, 24 Feb 2023 04:35:01 -0800 (PST) Received: from ?IPV6:2804:18:18f2:41f9:b823:cfb9:44bf:c519? ([2804:18:18f2:41f9:b823:cfb9:44bf:c519]) by smtp.gmail.com with ESMTPSA id m16-20020a056820035000b00517fc5fdf5bsm3588569ooe.17.2023.02.24.04.34.59 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 24 Feb 2023 04:35:01 -0800 (PST) Message-ID: <76498f0c-6520-4932-25e6-b3691bf81db5@linaro.org> Date: Fri, 24 Feb 2023 09:34:57 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.7.2 Subject: Re: [PATCH v2] string: Fix OOB read on generic strncmp Content-Language: en-US To: Wilco Dijkstra , Florian Weimer , Szabolcs Nagy , "H.J. Lu" , Noah Goldstein Cc: 'GNU C Library' References: From: Adhemerval Zanella Netto Organization: Linaro In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-5.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,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: On 24/02/23 09:24, Wilco Dijkstra wrote: > Hi, > >> 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. > > C11 says you can't read past the end of the array, but it doesn't say that > you *must* read past a NUL-byte. My interpretation is that this is a valid > strncmp implementation: > > int > simple_strncmp (const char *s1, const char *s2, size_t size) > { > size_t len1 = strnlen (s1, size); > size_t len2 = strnlen (s2, size); > if (len1 < len2) > len1++; > else if (len1 > len2) > len1 = len2 + 1; > return memcmp (s1, s2, len1); > } > > Ie. both strings must be valid up until the given size or NUL-terminated if > smaller. This works on the example above even if the first string is only 1 byte. That was my understanding as well when I wrote the generic strncmp, and multiple strncmp implementation also follows this same idea. So another option is to keep the generic strncmp as is and just remove the crypt/badsalttest.c, since it passing invalid inputs on crypt (non null terminated strings). > >> 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). > > Existing functions already do stop after the first NUL byte. Even if the C > standard doesn't explicitly disallow it, it doesn't seem valid to read beyond > it (a lot of code could fail if we did). > > Cheers, > Wilco