From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oa1-x2f.google.com (mail-oa1-x2f.google.com [IPv6:2001:4860:4864:20::2f]) by sourceware.org (Postfix) with ESMTPS id 2C5673858284 for ; Fri, 24 Feb 2023 12:57:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2C5673858284 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-x2f.google.com with SMTP id 586e51a60fabf-172b0ba97b0so829124fac.4 for ; Fri, 24 Feb 2023 04:57:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1677243475; 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=LIK+fyJm9wO7j63fKW1qbdSQ8gfexmM/xOpgREPdaDY=; b=MvCvjXrOZTj9RH1zxDWuXOf2cddYyxfY3avw2VjzZ449427gptatNDCCT6ZF0orJJX YUJUH1G4L9YgCpPlgkWko5/wKE5vcQG4Hw0CbHWbstrg7HOGfdER7lVcSt747ma9wMWb 1sNWrThcuJ7n1I8bWBw1apT2jKmlYCsaO2hHMTS5xGj1x5pUaohp0kMk09wiDKOYK8tQ 5f2PZK/EtGRUxrNbbTfD9nNglpaZM8X8S5JgQg4HJNlGtO2PPAtm5XhlNVnHfMnVCzL3 +3pHrq9YACuD/kyr8EMbmfBeHbFT7Wup4EotBimvrAkQK9FWapaFtKWff4ClG5WHolRr sL1A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1677243475; 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=LIK+fyJm9wO7j63fKW1qbdSQ8gfexmM/xOpgREPdaDY=; b=FsWbAXjwYURz1GHxuJsHmpKDc5Qd6USD/2Gs70ZhFU1Xy79aDlp81mZdv1NFngb57K dq32bTeaPtU8731tcwoWB6xtPir0TgThgnnlrDGJ7gYJnkLZACfkSxw7vr0jUlIhNcjy eQ0L/xgAX2P6y2mfPxV2zILRSOxa7FBiYcI8QtarlArqXEmqAlIxmXQcLba9xc8jlliP 4nVbCNPxnxpBmpO0NfqtbVJ2I7mDoss/ojKcprw+1gbLCVMYJrTjTLtaoBRdFPOL0BvH /p34WYJCRVVfCroemPGyKUE3S8USu0QMSgBaY0pAc9uLtPrsDDRIH5k+vn6HU3zgal8z 54BA== X-Gm-Message-State: AO0yUKXFR2HJZIvsgRs65pf8IQvr5Z8M7nTtF+WSeZegb3phjXSf+TxB UUPSLnq2UZge5CQk3HkcbVQJprWwQBSOT0AkB5k= X-Google-Smtp-Source: AK7set+6CA6G69Hju1v+mUx7kCw8uaICiZIW99gPrge+CAxdweVqUt+0OZR7dPNH204tMHH11d6kCQ== X-Received: by 2002:a05:6870:b491:b0:16f:3616:948c with SMTP id y17-20020a056870b49100b0016f3616948cmr13548282oap.27.1677243475174; Fri, 24 Feb 2023 04:57:55 -0800 (PST) Received: from ?IPV6:2804:18:18f2:41f9:1957:9792:e638:66ba? ([2804:18:18f2:41f9:1957:9792:e638:66ba]) by smtp.gmail.com with ESMTPSA id zj16-20020a0568716c9000b001724742cfcesm2720037oab.38.2023.02.24.04.57.53 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 24 Feb 2023 04:57:54 -0800 (PST) Message-ID: <0f2bca41-b383-bafd-fb19-0639fbe15a31@linaro.org> Date: Fri, 24 Feb 2023 09:57:51 -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: Szabolcs Nagy , Wilco Dijkstra Cc: Florian Weimer , 'GNU C Library' References: From: Adhemerval Zanella Netto Organization: Linaro In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit 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:43, Szabolcs Nagy wrote: > The 02/24/2023 12:24, Wilco Dijkstra wrote: >>> 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. > > note that the crypt/badsalttest test case that started > this discussion relies on strncmp stopping at the first > mismatch byte, not just at null byte: > > https://sourceware.org/git/?p=glibc.git;a=blob;f=crypt/badsalttest.c;h=bc1e5c1442c731b597551919bec9174ecd2cce61;hb=HEAD#l60 > https://sourceware.org/git/?p=glibc.git;a=blob;f=crypt/crypt-entry.c;h=c24c0110a66a0b6a8e885b5a3d0e942da46b1325;hb=HEAD#l84 > > i think it's reasonable to support stopping the read at > null byte but allow reading past the first mismatch. > (memcmp allows reading past mismatches too). > > i.e. the strncmp patch and testcase patch is not needed. > > then the question is if non-null-termnated input is valid > as crypt salt argument (if not the test should be fixed, > if yes, it should not use strncmp). I think the problem is using a non-NULL terminated string as crypt input, thus the test is not really valid. In fact, is not even valid for glibc for all strncmp implementations (some do fail and read past first mismatch depending of input alignment).