From patchwork Mon May 1 15:53:48 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 13227600 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id CECA0C77B73 for ; Mon, 1 May 2023 15:53:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232868AbjEAPxx (ORCPT ); Mon, 1 May 2023 11:53:53 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38724 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232064AbjEAPxv (ORCPT ); Mon, 1 May 2023 11:53:51 -0400 Received: from mail-yb1-xb29.google.com (mail-yb1-xb29.google.com [IPv6:2607:f8b0:4864:20::b29]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AB7A7E8 for ; Mon, 1 May 2023 08:53:50 -0700 (PDT) Received: by mail-yb1-xb29.google.com with SMTP id 3f1490d57ef6-b9da6374fa2so3176761276.0 for ; Mon, 01 May 2023 08:53:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20221208.gappssmtp.com; s=20221208; t=1682956429; x=1685548429; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=r5MQh5k9nsKF29e4Xq+z7dgsJl0I4C2yAieDUuExjzs=; b=K51qSRoUTm8/fs9dSSEqMSEiFagpzPzmOK26krKAeUleoGafsPQyLGTntiqV/qsa7n xmuU5HdAM8NktFnLRn8XTacN1G/BMbpC47t/GnPUk2xKoo0un+xlxCZ7cGXdDzpmE/lN XSuE5ZDOWJmVSTChCHq2kXZxhWWtHuAwjXNWBz2Q5SuUZRUgezRadHs/Vu8k96CSkz3M knL48GReJhNSpxON+8FYqAd9vmJK1QiVBf4MbxYSKuSGvo9NhnrZ0w3VHEmffx8umdfz 2WkZrfXMMOqY15E3//2XRrcFQ5KFF+ILmGHNWx3jnueUzeI369D/f6J38fja04NUqh9+ 14Fw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682956430; x=1685548430; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=r5MQh5k9nsKF29e4Xq+z7dgsJl0I4C2yAieDUuExjzs=; b=KlWrrWWO1gIRJkC5Pu7HHmcdsR1nQ1K2VfU3BKYUcPxvKVVwYsg3kwvuiuFkAPL2Io UFAp+TDDsH0V4DpVLE5HfZ5ctavz8N86tx4pF4iKqTzybKdVszz5ijWjiPTGxonSL2lK TOFWRo4O0NH3TT7x6bFOLfrV43jpMZSgltZtIo9Ae7lRo7ZXerLkGbvZQsRuJ74cYPjh m4gCn21bYDPSBe3nY1Y2+Wnf0l4DMoIEwRV6meGiqZGcpIvoUxk0ITZholz8ULdA7z37 dQgqvrf0t+IzdVRdAjQGgIIJUADbu/o+ZuPt4hkPlXrckX7+fOafbD0aY0pTbiSnpW9x 2GNA== X-Gm-Message-State: AC+VfDws+oIMQVeol8BOVmBB5OyRmLhPz9n05w94KbvfuhH5hWFKPrWZ 7G0JosbNUo4Jz9TdGw+zYDmg8EhXX8senB9tDAgTsw== X-Google-Smtp-Source: ACHHUZ76u0FN9fNaxiO1W7GOw2uP9P1f3W6qclLjKIM/6XSG2QCNUaoPUBQ72ZTmAnH+Mh8gzqS9Rg== X-Received: by 2002:a25:a82:0:b0:b95:8f10:5936 with SMTP id 124-20020a250a82000000b00b958f105936mr13682047ybk.54.1682956429743; Mon, 01 May 2023 08:53:49 -0700 (PDT) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id j6-20020a5b0686000000b00b8f3b826e58sm6788976ybq.19.2023.05.01.08.53.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 01 May 2023 08:53:49 -0700 (PDT) Date: Mon, 1 May 2023 11:53:48 -0400 From: Taylor Blau To: git@vger.kernel.org Cc: Jeff King , Junio C Hamano , Matthew John Cheetham , Johannes Schindelin , Derrick Stolee Subject: [PATCH 1/7] credential.c: store "wwwauth[]" values in `credential_read()` Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Teach git-credential to read "wwwauth[]" value(s) when parsing the output of a credential helper. These extra headers are not needed for Git's own HTTP support to use the feature internally, but the feature would not be available for a scripted caller (say, git-remote-mediawiki providing the header in the same way). As a bonus, this also makes it easier to use wwwauth[] in synthetic credential inputs in our test suite. Co-authored-by: Jeff King Signed-off-by: Jeff King Signed-off-by: Taylor Blau --- credential.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/credential.c b/credential.c index e6417bf880..96fc0daa35 100644 --- a/credential.c +++ b/credential.c @@ -238,6 +238,8 @@ int credential_read(struct credential *c, FILE *fp) } else if (!strcmp(key, "path")) { free(c->path); c->path = xstrdup(value); + } else if (!strcmp(key, "wwwauth[]")) { + strvec_push(&c->wwwauth_headers, value); } else if (!strcmp(key, "password_expiry_utc")) { errno = 0; c->password_expiry_utc = parse_timestamp(value, NULL, 10); From patchwork Mon May 1 15:53:51 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 13227601 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E4232C77B61 for ; Mon, 1 May 2023 15:54:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232881AbjEAPyD (ORCPT ); Mon, 1 May 2023 11:54:03 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38974 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232878AbjEAPx7 (ORCPT ); Mon, 1 May 2023 11:53:59 -0400 Received: from mail-yb1-xb32.google.com (mail-yb1-xb32.google.com [IPv6:2607:f8b0:4864:20::b32]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BE54F1713 for ; Mon, 1 May 2023 08:53:53 -0700 (PDT) Received: by mail-yb1-xb32.google.com with SMTP id 3f1490d57ef6-b9246a5f3feso4623952276.1 for ; Mon, 01 May 2023 08:53:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20221208.gappssmtp.com; s=20221208; t=1682956433; x=1685548433; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=XAYAVhtVaU1kWJcxIhfjrv6K3XY3552kPu9enqY2Yyw=; b=PTfQB5RaW0NcZWt5OnlUFQ7s6p1ybK1r63OEZ8uaq75wpwYdQzesG60Vwy95z0reoJ SBFLX2wimGKdVTnoIMHXxggCZTrMoQT7fO0C6shw+gA21tPX/rQ/eUMSQSD6bShRDO71 rdgsdh/OZFhp14IaIpeN4rduqfobWo1/jxWxa0r8PGjyaa4wpb6HgeEURF/EJaqEJ7ot m2I4YN4LTxVin+p9aVn6Uqks8RqCh6SeFYlA/SfSlDc8PXmj3DX+rSAdnR4FjhLYze/I EUy/5yNrP9CF8XJWxNtdL4AOGvDJkPJfcrsE09q0TZIZ2oUIWR9GJQ/eWmtKXrjN6lBD KzMw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682956433; x=1685548433; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=XAYAVhtVaU1kWJcxIhfjrv6K3XY3552kPu9enqY2Yyw=; b=ZiLx5ur8i2/qY8HR5eNPq7VVyavFzySE8ckyFREWiuDaADyT0E/a6sXM5mCulsh0XR DIm7Wf/zif7VQIjgmrt1UpK62RUzYGh0zwp0yhcgOTBwyl6ERXuu8s6pCHSLXv+mpnZe XTU4qCLsvXdNAssZY4aOCM3iI92iq/OXZZOOA0uAeNfU0CggsBlwGbeWmk7hfeZ8T0YV zgLNE0JEJPSG+qJxwUecMd8hyaK9SfbcHCqrRAXjiYfSLBHHp6voz1wPiJ+HZW90SnST I0uUGcuCX/7sUSUw4KmWoVklo/BFZ+qzkqtcvx9Mxy3xZLtS4w0fqXm0dhPGHJoBtBbj kqvg== X-Gm-Message-State: AC+VfDywUceqazC/JLs4qy1Rx1IP+qNdC6lEa84fRBFm4DGfkp1pvFBZ YVz8MyO9i4PTeZU5nWzDZTyt/Yqb/ieVOmFn3jxn6A== X-Google-Smtp-Source: ACHHUZ7n1lWtvejXefY+zr1vHGSLRc/OqXfmL669QFiX6QH1my82Mpi0X8OwFxtc1S9lyvAtS3kEfw== X-Received: by 2002:a25:ad12:0:b0:b99:4af6:185d with SMTP id y18-20020a25ad12000000b00b994af6185dmr12504769ybi.6.1682956432810; Mon, 01 May 2023 08:53:52 -0700 (PDT) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id h1-20020a5b0a81000000b00b8f54571fc0sm6727544ybq.5.2023.05.01.08.53.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 01 May 2023 08:53:52 -0700 (PDT) Date: Mon, 1 May 2023 11:53:51 -0400 From: Taylor Blau To: git@vger.kernel.org Cc: Jeff King , Junio C Hamano , Matthew John Cheetham , Johannes Schindelin , Derrick Stolee Subject: [PATCH 2/7] t/lib-credential.sh: ensure credential helpers handle long headers Message-ID: <73800b548a39fb657a41bf5d7061ce5406f09efd.1682956419.git.me@ttaylorr.com> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Add a test ensuring that the "wwwauth[]" field cannot be used to inject malicious data into the credential helper stream. Many of the credential helpers in contrib/credential read the newline-delimited protocol stream one line at a time by repeatedly calling fgets() into a fixed-size buffer. This assumes that each line is no more than 1024 characters long, since each iteration of the loop assumes that it is parsing starting at the beginning of a new line in the stream. However, similar to a5bb10fd5e (config: avoid fixed-sized buffer when renaming/deleting a section, 2023-04-06), if a line is longer than 1024 characters, a malicious actor can embed another command within an existing line, bypassing the usual checks introduced in 9a6bbee800 (credential: avoid writing values with newlines, 2020-03-11). As with the problem fixed in that commit, specially crafted input can cause the helper to return the credential for the wrong host, letting an attacker trick the victim into sending credentials for one host to another. Luckily, all parts of the credential helper protocol that are available in a tagged release of Git are immune to this attack: - "protocol" is restricted to known values, and is thus immune. - "host" is immune because curl will reject hostnames that have a '=' character in them, which would be required to carry out this attack. - "username" is immune, because the buffer characters to fill out the first `fgets()` call would pollute the `username` field, causing the credential helper to return nothing (because it would match a username if present, and the username of the credential to be stolen is likely not 1024 characters). - "password" is immune because providing a password instructs credential helpers to avoid filling credentials in the first place. - "path" is similar to username; if present, it is not likely to match any credential the victim is storing. It's also not enabled by default; the victim would have to set credential.useHTTPPath explicitly. However, the new "wwwauth[]" field introduced via 5f2117b24f (credential: add WWW-Authenticate header to cred requests, 2023-02-27) can be used to inject data into the credential helper stream. For example, running: { printf 'HTTP/1.1 401\r\n' printf 'WWW-Authenticate: basic realm=' perl -e 'print "a" x 1024' printf 'host=victim.com\r\n' } | nc -Nlp 8080 in one terminal, and then: git clone http://localhost:8080 in another would result in a line like: wwwauth[]=basic realm=aaa[...]aaahost=victim.com being sent to the credential helper. If we tweak that "1024" to align our output with the helper's buffer size and the rest of the data on the line, it can cause the helper to see "host=victim.com" on its own line, allowing motivated attackers to exfiltrate credentials belonging to "victim.com". The below test demonstrates these failures and provides us with a test to ensure that our fix is correct. That said, it has a couple of shortcomings: - it's in t0303, since that's the only mechanism we have for testing random helpers. But that means nobody is going to run it under normal circumstances. - to get the attack right, it has to line up the stuffed name with the buffer size, so we depend on the exact buffer size. I parameterized it so it could be used to test other helpers, but in practice it's not likely for anybody to do that. Still, it's the best we can do, and will help us confirm the presence of the problem (and our fixes) in the new few patches. Co-authored-by: Jeff King Signed-off-by: Jeff King Signed-off-by: Taylor Blau --- t/lib-credential.sh | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/t/lib-credential.sh b/t/lib-credential.sh index 5ea8bc9f1d..d7d03c3cd9 100644 --- a/t/lib-credential.sh +++ b/t/lib-credential.sh @@ -270,6 +270,35 @@ helper_test() { password= EOF ' + + : ${GIT_TEST_LONG_CRED_BUFFER:=1024} + # 23 bytes accounts for "wwwauth[]=basic realm=" plus NUL + LONG_VALUE_LEN=$((GIT_TEST_LONG_CRED_BUFFER - 23)) + LONG_VALUE=$(perl -e 'print "a" x shift' $LONG_VALUE_LEN) + + test_expect_success "helper ($HELPER) not confused by long header" ' + check approve $HELPER <<-\EOF && + protocol=https + host=victim.example.com + username=user + password=to-be-stolen + EOF + + check fill $HELPER <<-EOF + protocol=https + host=badguy.example.com + wwwauth[]=basic realm=${LONG_VALUE}host=victim.example.com + -- + protocol=https + host=badguy.example.com + username=askpass-username + password=askpass-password + wwwauth[]=basic realm=${LONG_VALUE}host=victim.example.com + -- + askpass: Username for '\''https://badguy.example.com'\'': + askpass: Password for '\''https://askpass-username@badguy.example.com'\'': + EOF + ' } helper_test_timeout() { From patchwork Mon May 1 15:53:54 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 13227602 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6392EC77B73 for ; Mon, 1 May 2023 15:54:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232888AbjEAPyS (ORCPT ); Mon, 1 May 2023 11:54:18 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39164 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232883AbjEAPyI (ORCPT ); Mon, 1 May 2023 11:54:08 -0400 Received: from mail-yw1-x1131.google.com (mail-yw1-x1131.google.com [IPv6:2607:f8b0:4864:20::1131]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1478B1BCA for ; Mon, 1 May 2023 08:53:57 -0700 (PDT) Received: by mail-yw1-x1131.google.com with SMTP id 00721157ae682-5529f3b8623so24803447b3.2 for ; Mon, 01 May 2023 08:53:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20221208.gappssmtp.com; s=20221208; t=1682956436; x=1685548436; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=4Rj7sYJXjVNVdcz5mH90uvdN31oyZO1DhIrx/Ty//rE=; b=1XHY7wesAsr5xOoaBQ8SkQ1L4s/S0ZAiaqiEU097cLKwYe776d1Y1l6lrjjwJfIwk4 pr1MQvTSY+GhLC0jB2kpF0tAwqgnsC9jAsbcs/b/FZfzTGWpXjdyRG0c1idt8t7yLe7c +cM38YD//RD9rrIssc5xgIOcjMRVdB5/CYu5RPY+vroB7jOvqrIVpsqAnqxV+QBdOR0A 4zZn2Q585t25lRmKCZJLBTTV0CuVTCKpFXLg7a64/m8RsCEtbB1B3whTr6qPBqODWLG2 JCrsOQPvAF4iVo+TG1y4MsuPB6WOzp0Nhjnct5Pk4QM6iIxbuCNi7Z+b7/f56I5MmTsU IHCQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682956436; x=1685548436; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=4Rj7sYJXjVNVdcz5mH90uvdN31oyZO1DhIrx/Ty//rE=; b=MRcN2Jq18XFjNKcdW8r8SdVNJ2GogPxzPTEqGZk4CavcIcN21AzDl9ubGWWq8Dd2SC ykRu7f15eT5P8KFR/PRsRs6vJmckHdtGPxpQFaA/NYXXMo0Yv2zmAjVRMfg9iR15NhB8 gqJ7YK/BtLgJ9iLVlEqR6EZ0YKrnQk/3u8c4Lj5zbi5327XitcJE8ru9oiRZqTZmQn8K HHLHIaSejhWSchR2SCd2XJUucUvlJ6Y/6ouUONV3UVWKt7zaK8jPnkaOgdMcHEWM00RO aet5/3JRSVllLOMyP41vlfk3OC6E96nVaolS/Eve4w+CUFiV/B69R6PJS7FR5PiHh628 h+dA== X-Gm-Message-State: AC+VfDwBhW6DKp1wRbxvURGD/ZULQb8SVJ3LN0GzgVd9cwao+FsN/ob7 FUx+XN9KjbBk06u/T64zOf/Dk9L3yZuSPRXNU6tTYg== X-Google-Smtp-Source: ACHHUZ4HJF19UqhILQrW/eJ2gcYT8m23W4fYkE3Spo4/T06woVq2H/XYMGVBg2xxPYqnpDKD7PVWfQ== X-Received: by 2002:a25:5345:0:b0:b97:4b7b:945c with SMTP id h66-20020a255345000000b00b974b7b945cmr12811421ybb.57.1682956435702; Mon, 01 May 2023 08:53:55 -0700 (PDT) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id s62-20020a255e41000000b00b92579d3d7csm2608525ybb.52.2023.05.01.08.53.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 01 May 2023 08:53:55 -0700 (PDT) Date: Mon, 1 May 2023 11:53:54 -0400 From: Taylor Blau To: git@vger.kernel.org Cc: Jeff King , Junio C Hamano , Matthew John Cheetham , Johannes Schindelin , Derrick Stolee Subject: [PATCH 3/7] contrib/credential: avoid fixed-size buffer in osxkeychain Message-ID: <301bc27a5b4500ff6681bbf5a113d763b8f0b4c9.1682956419.git.me@ttaylorr.com> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org The macOS Keychain-based credential helper reads the newline-delimited protocol stream one line at a time by repeatedly calling fgets() into a fixed-size buffer, and is thus affected by the vulnerability described in the previous commit. To mitigate this attack, avoid using a fixed-size buffer, and instead rely on getline() to allocate a buffer as large as necessary to fit the entire content of the line, preventing any protocol injection. We solved a similar problem in a5bb10fd5e (config: avoid fixed-sized buffer when renaming/deleting a section, 2023-04-06) by switching to strbuf_getline(). We can't do that here because the contrib helpers do not link with the rest of Git, and so can't use a strbuf. But we can use the system getline() directly, which works similarly. In most parts of Git we don't assume that every platform has getline(). But this helper is run only on OS X, and that platform added support in 10.7 ("Lion") which was released in 2011. Tested-by: Taylor Blau Co-authored-by: Jeff King Signed-off-by: Jeff King Signed-off-by: Taylor Blau --- .../osxkeychain/git-credential-osxkeychain.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/contrib/credential/osxkeychain/git-credential-osxkeychain.c b/contrib/credential/osxkeychain/git-credential-osxkeychain.c index e29cc28779..5f2e5f16c8 100644 --- a/contrib/credential/osxkeychain/git-credential-osxkeychain.c +++ b/contrib/credential/osxkeychain/git-credential-osxkeychain.c @@ -113,14 +113,16 @@ static void add_internet_password(void) static void read_credential(void) { - char buf[1024]; + char *buf = NULL; + size_t alloc; + ssize_t line_len; - while (fgets(buf, sizeof(buf), stdin)) { + while ((line_len = getline(&buf, &alloc, stdin)) > 0) { char *v; if (!strcmp(buf, "\n")) break; - buf[strlen(buf)-1] = '\0'; + buf[line_len-1] = '\0'; v = strchr(buf, '='); if (!v) @@ -165,6 +167,8 @@ static void read_credential(void) * learn new lines, and the helpers are updated to match. */ } + + free(buf); } int main(int argc, const char **argv) From patchwork Mon May 1 15:53:57 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 13227603 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 25F28C77B73 for ; Mon, 1 May 2023 15:54:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232193AbjEAPyY (ORCPT ); Mon, 1 May 2023 11:54:24 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39206 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232859AbjEAPyS (ORCPT ); Mon, 1 May 2023 11:54:18 -0400 Received: from mail-yb1-xb29.google.com (mail-yb1-xb29.google.com [IPv6:2607:f8b0:4864:20::b29]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D0F1D199 for ; Mon, 1 May 2023 08:53:59 -0700 (PDT) Received: by mail-yb1-xb29.google.com with SMTP id 3f1490d57ef6-b9d87dffadfso2096887276.3 for ; Mon, 01 May 2023 08:53:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20221208.gappssmtp.com; s=20221208; t=1682956439; x=1685548439; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=9mV6bruf5iOXfF9MBhazeJqDknG+ed//kgRlEScXJzo=; b=Z3t+W7Vbu2AICE3F6WpExLbPuRWaeddzDclduv2YNl8l1npf0gajexmrV6/pbbrmSW za4D+x6AaKLIhJtrgGoL0Op/dsE08Y0zD7X4J5dONor3nVwcOs9SdVUVWwHiUhjUh3Yo YI4lbJ43OEXGR4feTAxn1C1Zkx2d1Rd0uU2MNNA9/ojYyMkY8RodltDuEAlI2zahMGrS sTo5/Rc6waoKhKW5mwEjDvEe/c8ZAZq0iLdEoRdjkk63L9EfmzfjQ+nEqv9eDBCZK2oR XtpuK/TCmht/Y6WKlpZdqf0Clsoxt2Y5UAqar46urcAH6oEDlCVBxCsYSkzORa/O+JM5 jJQQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682956439; x=1685548439; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=9mV6bruf5iOXfF9MBhazeJqDknG+ed//kgRlEScXJzo=; b=dtZ1YJ/z/St/YScfh5XjtuUy9dCpf4Y9QFEsMxOyv3qKX5/ebDEG1q/lFZ4X7vjJ24 yF8JWP/sF5I5mqu19Lv1OS+dZedP5DKIq8F3f1lWsS0Sg/7f9wixLj+SBgVBQW+GCx5w BEzdqtgf5xqjD8z5n9c88w/iBWtEVtJ2Gsynu7cVO+Nb0Dsc3vwLqLlzZs6VgXi1deQO E3MRysBBpi9DnQJA9+Q+9vHGcbkCx15TtYEn07MEDr2DrmjrNwU0xnqC3gCJPBVAaHuc bk3hx6NSY9bNiBIiQK+JlJZZk9B0BV4Azel0cpAYBSk+LkaXUS8DRfzLWbZsg8tGhT4o F5Ng== X-Gm-Message-State: AC+VfDzMl0j6TPYjT/HYSJkqxqgtvefkS1jtnLwEgn1vPRKC3hPBlA5o NLlZjmH0tBKsIUsbk0JhmOBzgsaqVu+yt8vWDpQqRw== X-Google-Smtp-Source: ACHHUZ4S5lcl0Ph6SxpTt/SEwb8rdCMcpMzg5IiQi5oGQS4wm19eP5Apj1D2urehs6yTWO4uTAfpUA== X-Received: by 2002:a05:6902:1003:b0:b8c:a9c:ac6c with SMTP id w3-20020a056902100300b00b8c0a9cac6cmr15191612ybt.4.1682956438814; Mon, 01 May 2023 08:53:58 -0700 (PDT) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id a82-20020a251a55000000b00b8f3e224dcesm906885yba.13.2023.05.01.08.53.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 01 May 2023 08:53:58 -0700 (PDT) Date: Mon, 1 May 2023 11:53:57 -0400 From: Taylor Blau To: git@vger.kernel.org Cc: Jeff King , Junio C Hamano , Matthew John Cheetham , Johannes Schindelin , Derrick Stolee Subject: [PATCH 4/7] contrib/credential: remove 'gnome-keyring' credential helper Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org libgnome-keyring was deprecated in 2014 (in favor of libsecret), more than nine years ago [1]. The credential helper implemented using libgnome-keyring has had a small handful of commits since 2013, none of which implemented or changed any functionality. The last commit to do substantial work in this area was 15f7221686 (contrib/git-credential-gnome-keyring.c: support really ancient gnome-keyring, 2013-09-23), just shy of nine years ago. This credential helper suffers from the same `fgets()`-related injection attack (using the new "wwwauth[]" feature) as in the previous commit. Instead of patching it, let's remove this helper as deprecated. [1]: https://mail.gnome.org/archives/commits-list/2014-January/msg01585.html Co-authored-by: Jeff King Signed-off-by: Jeff King Signed-off-by: Taylor Blau --- contrib/credential/gnome-keyring/.gitignore | 1 - contrib/credential/gnome-keyring/Makefile | 25 - .../git-credential-gnome-keyring.c | 470 ------------------ 3 files changed, 496 deletions(-) delete mode 100644 contrib/credential/gnome-keyring/.gitignore delete mode 100644 contrib/credential/gnome-keyring/Makefile delete mode 100644 contrib/credential/gnome-keyring/git-credential-gnome-keyring.c diff --git a/contrib/credential/gnome-keyring/.gitignore b/contrib/credential/gnome-keyring/.gitignore deleted file mode 100644 index 88d8fcdbce..0000000000 --- a/contrib/credential/gnome-keyring/.gitignore +++ /dev/null @@ -1 +0,0 @@ -git-credential-gnome-keyring diff --git a/contrib/credential/gnome-keyring/Makefile b/contrib/credential/gnome-keyring/Makefile deleted file mode 100644 index 22c19df94b..0000000000 --- a/contrib/credential/gnome-keyring/Makefile +++ /dev/null @@ -1,25 +0,0 @@ -MAIN:=git-credential-gnome-keyring -all:: $(MAIN) - -CC = gcc -RM = rm -f -CFLAGS = -g -O2 -Wall -PKG_CONFIG = pkg-config - --include ../../../config.mak.autogen --include ../../../config.mak - -INCS:=$(shell $(PKG_CONFIG) --cflags gnome-keyring-1 glib-2.0) -LIBS:=$(shell $(PKG_CONFIG) --libs gnome-keyring-1 glib-2.0) - -SRCS:=$(MAIN).c -OBJS:=$(SRCS:.c=.o) - -%.o: %.c - $(CC) $(CFLAGS) $(CPPFLAGS) $(INCS) -o $@ -c $< - -$(MAIN): $(OBJS) - $(CC) -o $@ $(LDFLAGS) $^ $(LIBS) - -clean: - @$(RM) $(MAIN) $(OBJS) diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c deleted file mode 100644 index 5927e27ae6..0000000000 --- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c +++ /dev/null @@ -1,470 +0,0 @@ -/* - * Copyright (C) 2011 John Szakmeister - * 2012 Philipp A. Hartmann - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, see . - */ - -/* - * Credits: - * - GNOME Keyring API handling originally written by John Szakmeister - * - ported to credential helper API by Philipp A. Hartmann - */ - -#include -#include -#include -#include -#include - -#ifdef GNOME_KEYRING_DEFAULT - - /* Modern gnome-keyring */ - -#include - -#else - - /* - * Support ancient gnome-keyring, circ. RHEL 5.X. - * GNOME_KEYRING_DEFAULT seems to have been introduced with Gnome 2.22, - * and the other features roughly around Gnome 2.20, 6 months before. - * Ubuntu 8.04 used Gnome 2.22 (I think). Not sure any distro used 2.20. - * So the existence/non-existence of GNOME_KEYRING_DEFAULT seems like - * a decent thing to use as an indicator. - */ - -#define GNOME_KEYRING_DEFAULT NULL - -/* - * ancient gnome-keyring returns DENIED when an entry is not found. - * Setting NO_MATCH to DENIED will prevent us from reporting DENIED - * errors during get and erase operations, but we will still report - * DENIED errors during a store. - */ -#define GNOME_KEYRING_RESULT_NO_MATCH GNOME_KEYRING_RESULT_DENIED - -#define gnome_keyring_memory_alloc g_malloc -#define gnome_keyring_memory_free gnome_keyring_free_password -#define gnome_keyring_memory_strdup g_strdup - -static const char *gnome_keyring_result_to_message(GnomeKeyringResult result) -{ - switch (result) { - case GNOME_KEYRING_RESULT_OK: - return "OK"; - case GNOME_KEYRING_RESULT_DENIED: - return "Denied"; - case GNOME_KEYRING_RESULT_NO_KEYRING_DAEMON: - return "No Keyring Daemon"; - case GNOME_KEYRING_RESULT_ALREADY_UNLOCKED: - return "Already UnLocked"; - case GNOME_KEYRING_RESULT_NO_SUCH_KEYRING: - return "No Such Keyring"; - case GNOME_KEYRING_RESULT_BAD_ARGUMENTS: - return "Bad Arguments"; - case GNOME_KEYRING_RESULT_IO_ERROR: - return "IO Error"; - case GNOME_KEYRING_RESULT_CANCELLED: - return "Cancelled"; - case GNOME_KEYRING_RESULT_ALREADY_EXISTS: - return "Already Exists"; - default: - return "Unknown Error"; - } -} - -/* - * Support really ancient gnome-keyring, circ. RHEL 4.X. - * Just a guess for the Glib version. Glib 2.8 was roughly Gnome 2.12 ? - * Which was released with gnome-keyring 0.4.3 ?? - */ -#if GLIB_MAJOR_VERSION == 2 && GLIB_MINOR_VERSION < 8 - -static void gnome_keyring_done_cb(GnomeKeyringResult result, gpointer user_data) -{ - gpointer *data = (gpointer *)user_data; - int *done = (int *)data[0]; - GnomeKeyringResult *r = (GnomeKeyringResult *)data[1]; - - *r = result; - *done = 1; -} - -static void wait_for_request_completion(int *done) -{ - GMainContext *mc = g_main_context_default(); - while (!*done) - g_main_context_iteration(mc, TRUE); -} - -static GnomeKeyringResult gnome_keyring_item_delete_sync(const char *keyring, guint32 id) -{ - int done = 0; - GnomeKeyringResult result; - gpointer data[] = { &done, &result }; - - gnome_keyring_item_delete(keyring, id, gnome_keyring_done_cb, data, - NULL); - - wait_for_request_completion(&done); - - return result; -} - -#endif -#endif - -/* - * This credential struct and API is simplified from git's credential.{h,c} - */ -struct credential { - char *protocol; - char *host; - unsigned short port; - char *path; - char *username; - char *password; -}; - -#define CREDENTIAL_INIT { 0 } - -typedef int (*credential_op_cb)(struct credential *); - -struct credential_operation { - char *name; - credential_op_cb op; -}; - -#define CREDENTIAL_OP_END { NULL, NULL } - -/* ----------------- GNOME Keyring functions ----------------- */ - -/* create a special keyring option string, if path is given */ -static char *keyring_object(struct credential *c) -{ - if (!c->path) - return NULL; - - if (c->port) - return g_strdup_printf("%s:%hd/%s", c->host, c->port, c->path); - - return g_strdup_printf("%s/%s", c->host, c->path); -} - -static int keyring_get(struct credential *c) -{ - char *object = NULL; - GList *entries; - GnomeKeyringNetworkPasswordData *password_data; - GnomeKeyringResult result; - - if (!c->protocol || !(c->host || c->path)) - return EXIT_FAILURE; - - object = keyring_object(c); - - result = gnome_keyring_find_network_password_sync( - c->username, - NULL /* domain */, - c->host, - object, - c->protocol, - NULL /* authtype */, - c->port, - &entries); - - g_free(object); - - if (result == GNOME_KEYRING_RESULT_NO_MATCH) - return EXIT_SUCCESS; - - if (result == GNOME_KEYRING_RESULT_CANCELLED) - return EXIT_SUCCESS; - - if (result != GNOME_KEYRING_RESULT_OK) { - g_critical("%s", gnome_keyring_result_to_message(result)); - return EXIT_FAILURE; - } - - /* pick the first one from the list */ - password_data = (GnomeKeyringNetworkPasswordData *)entries->data; - - gnome_keyring_memory_free(c->password); - c->password = gnome_keyring_memory_strdup(password_data->password); - - if (!c->username) - c->username = g_strdup(password_data->user); - - gnome_keyring_network_password_list_free(entries); - - return EXIT_SUCCESS; -} - - -static int keyring_store(struct credential *c) -{ - guint32 item_id; - char *object = NULL; - GnomeKeyringResult result; - - /* - * Sanity check that what we are storing is actually sensible. - * In particular, we can't make a URL without a protocol field. - * Without either a host or pathname (depending on the scheme), - * we have no primary key. And without a username and password, - * we are not actually storing a credential. - */ - if (!c->protocol || !(c->host || c->path) || - !c->username || !c->password) - return EXIT_FAILURE; - - object = keyring_object(c); - - result = gnome_keyring_set_network_password_sync( - GNOME_KEYRING_DEFAULT, - c->username, - NULL /* domain */, - c->host, - object, - c->protocol, - NULL /* authtype */, - c->port, - c->password, - &item_id); - - g_free(object); - - if (result != GNOME_KEYRING_RESULT_OK && - result != GNOME_KEYRING_RESULT_CANCELLED) { - g_critical("%s", gnome_keyring_result_to_message(result)); - return EXIT_FAILURE; - } - - return EXIT_SUCCESS; -} - -static int keyring_erase(struct credential *c) -{ - char *object = NULL; - GList *entries; - GnomeKeyringNetworkPasswordData *password_data; - GnomeKeyringResult result; - - /* - * Sanity check that we actually have something to match - * against. The input we get is a restrictive pattern, - * so technically a blank credential means "erase everything". - * But it is too easy to accidentally send this, since it is equivalent - * to empty input. So explicitly disallow it, and require that the - * pattern have some actual content to match. - */ - if (!c->protocol && !c->host && !c->path && !c->username) - return EXIT_FAILURE; - - object = keyring_object(c); - - result = gnome_keyring_find_network_password_sync( - c->username, - NULL /* domain */, - c->host, - object, - c->protocol, - NULL /* authtype */, - c->port, - &entries); - - g_free(object); - - if (result == GNOME_KEYRING_RESULT_NO_MATCH) - return EXIT_SUCCESS; - - if (result == GNOME_KEYRING_RESULT_CANCELLED) - return EXIT_SUCCESS; - - if (result != GNOME_KEYRING_RESULT_OK) { - g_critical("%s", gnome_keyring_result_to_message(result)); - return EXIT_FAILURE; - } - - /* pick the first one from the list (delete all matches?) */ - password_data = (GnomeKeyringNetworkPasswordData *)entries->data; - - result = gnome_keyring_item_delete_sync( - password_data->keyring, password_data->item_id); - - gnome_keyring_network_password_list_free(entries); - - if (result != GNOME_KEYRING_RESULT_OK) { - g_critical("%s", gnome_keyring_result_to_message(result)); - return EXIT_FAILURE; - } - - return EXIT_SUCCESS; -} - -/* - * Table with helper operation callbacks, used by generic - * credential helper main function. - */ -static struct credential_operation const credential_helper_ops[] = { - { "get", keyring_get }, - { "store", keyring_store }, - { "erase", keyring_erase }, - CREDENTIAL_OP_END -}; - -/* ------------------ credential functions ------------------ */ - -static void credential_init(struct credential *c) -{ - memset(c, 0, sizeof(*c)); -} - -static void credential_clear(struct credential *c) -{ - g_free(c->protocol); - g_free(c->host); - g_free(c->path); - g_free(c->username); - gnome_keyring_memory_free(c->password); - - credential_init(c); -} - -static int credential_read(struct credential *c) -{ - char *buf; - size_t line_len; - char *key; - char *value; - - key = buf = gnome_keyring_memory_alloc(1024); - - while (fgets(buf, 1024, stdin)) { - line_len = strlen(buf); - - if (line_len && buf[line_len-1] == '\n') - buf[--line_len] = '\0'; - - if (!line_len) - break; - - value = strchr(buf, '='); - if (!value) { - g_warning("invalid credential line: %s", key); - gnome_keyring_memory_free(buf); - return -1; - } - *value++ = '\0'; - - if (!strcmp(key, "protocol")) { - g_free(c->protocol); - c->protocol = g_strdup(value); - } else if (!strcmp(key, "host")) { - g_free(c->host); - c->host = g_strdup(value); - value = strrchr(c->host, ':'); - if (value) { - *value++ = '\0'; - c->port = atoi(value); - } - } else if (!strcmp(key, "path")) { - g_free(c->path); - c->path = g_strdup(value); - } else if (!strcmp(key, "username")) { - g_free(c->username); - c->username = g_strdup(value); - } else if (!strcmp(key, "password")) { - gnome_keyring_memory_free(c->password); - c->password = gnome_keyring_memory_strdup(value); - while (*value) - *value++ = '\0'; - } - /* - * Ignore other lines; we don't know what they mean, but - * this future-proofs us when later versions of git do - * learn new lines, and the helpers are updated to match. - */ - } - - gnome_keyring_memory_free(buf); - - return 0; -} - -static void credential_write_item(FILE *fp, const char *key, const char *value) -{ - if (!value) - return; - fprintf(fp, "%s=%s\n", key, value); -} - -static void credential_write(const struct credential *c) -{ - /* only write username/password, if set */ - credential_write_item(stdout, "username", c->username); - credential_write_item(stdout, "password", c->password); -} - -static void usage(const char *name) -{ - struct credential_operation const *try_op = credential_helper_ops; - const char *basename = strrchr(name, '/'); - - basename = (basename) ? basename + 1 : name; - fprintf(stderr, "usage: %s <", basename); - while (try_op->name) { - fprintf(stderr, "%s", (try_op++)->name); - if (try_op->name) - fprintf(stderr, "%s", "|"); - } - fprintf(stderr, "%s", ">\n"); -} - -int main(int argc, char *argv[]) -{ - int ret = EXIT_SUCCESS; - - struct credential_operation const *try_op = credential_helper_ops; - struct credential cred = CREDENTIAL_INIT; - - if (!argv[1]) { - usage(argv[0]); - exit(EXIT_FAILURE); - } - - g_set_application_name("Git Credential Helper"); - - /* lookup operation callback */ - while (try_op->name && strcmp(argv[1], try_op->name)) - try_op++; - - /* unsupported operation given -- ignore silently */ - if (!try_op->name || !try_op->op) - goto out; - - ret = credential_read(&cred); - if (ret) - goto out; - - /* perform credential operation */ - ret = (*try_op->op)(&cred); - - credential_write(&cred); - -out: - credential_clear(&cred); - return ret; -} From patchwork Mon May 1 15:54:00 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 13227604 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4D1F1C77B7C for ; Mon, 1 May 2023 15:54:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232793AbjEAPy0 (ORCPT ); Mon, 1 May 2023 11:54:26 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39346 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232874AbjEAPyV (ORCPT ); Mon, 1 May 2023 11:54:21 -0400 Received: from mail-yw1-x112a.google.com (mail-yw1-x112a.google.com [IPv6:2607:f8b0:4864:20::112a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C2AF712C for ; Mon, 1 May 2023 08:54:02 -0700 (PDT) Received: by mail-yw1-x112a.google.com with SMTP id 00721157ae682-555e853d3c5so24171377b3.2 for ; Mon, 01 May 2023 08:54:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20221208.gappssmtp.com; s=20221208; t=1682956442; x=1685548442; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=83J8iMBnkUeKwuKz06wa79htp85qJvw5LSSb2dLSpgg=; b=TiqTJZwv4LhRPgcqTRtN1bekf9YMKpU4W09pKeBSYIQmlRrTU0xi9T3yx0bJ4cs1bt cKsebGJvRrIJOSsYKrfDVsVKJ3zJJdNtfJulCwTK5okqLQ0GvnmXVIiKxMc7mDx20n8t ACh6SO6bmCOKoHgrX068tjZE0GNJb29Ux220lp7Dx/FxLWF1C6XVcUt5yuWXuD0DE3N/ anMCbyHMtgKDFlhuUu+S4poXyeRWatZ00zDdBkKz7LXJtvjSS2m4bQd4vum9DKl2JSGu stDvE9QcS6udQgL1E1lFTJg/hg6t17kjQLXjtjnpoS7AQ/2RfEV+kkmbW8R9AHMv3AkD EdVg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682956442; x=1685548442; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=83J8iMBnkUeKwuKz06wa79htp85qJvw5LSSb2dLSpgg=; b=YmMwbrLyUOU6vYEr+wxPzfRfrwDG6cu+LKvbX9bstJbshmLcZD4e+tG0SW1pvUxkP2 jW6nh+V5lx99M5MkdpL7iIX2Sxaqz1KP7FNS9qIukCc16Zqeg4dnK2/Nlz9ve0cmtNza K9I7rBG5qoVfU1Xh+K1mveVafeXFRXxlaPWBwjm2sQu5nPijONXwDaYgypacW2pXwoCe GPvAYA3fJ3QDgXSU+oBadECsspGcrokC8gQxx5oIUUAM9TzSumGckGEam4V8mtWSNXvN Qj6Lwl7RBLbOCnHfFVs4O0Ur6E/r+IXypgT7Yi40VU1PGRNu4NCIjU3RuVG/X0OKSzF/ e9hA== X-Gm-Message-State: AC+VfDzXm/yYXiaX4gytR9C9cmyKgWkuBv1Qu2IWE1eDVDTDkGD39qoi D5vLskS+JXcUSCy5QvSJpCzg0csKiHGc0IujrW4gKA== X-Google-Smtp-Source: ACHHUZ4pV1EUT9CY812/bnq0Ibbi89wofwhAXAd3GxDMVgtt3bRo8ODhVAjzccRV6Phspn+SBT0MCw== X-Received: by 2002:a81:6584:0:b0:556:dedf:758f with SMTP id z126-20020a816584000000b00556dedf758fmr12825778ywb.48.1682956441842; Mon, 01 May 2023 08:54:01 -0700 (PDT) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id m66-20020a0de345000000b0054bfc94a10dsm7406981ywe.47.2023.05.01.08.54.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 01 May 2023 08:54:01 -0700 (PDT) Date: Mon, 1 May 2023 11:54:00 -0400 From: Taylor Blau To: git@vger.kernel.org Cc: Jeff King , Junio C Hamano , Matthew John Cheetham , Johannes Schindelin , Derrick Stolee Subject: [PATCH 5/7] contrib/credential: .gitignore libsecret build artifacts Message-ID: <60bab84359fd453c231adaae9a11b20bb19173db.1682956419.git.me@ttaylorr.com> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org The libsecret credential helper does not mark its build artifact as ignored, so running "make" results in a dirty working tree. Mark the "git-credential-libsecret" binary as ignored to avoid the above. Co-authored-by: Jeff King Signed-off-by: Jeff King Signed-off-by: Taylor Blau --- contrib/credential/libsecret/.gitignore | 1 + 1 file changed, 1 insertion(+) create mode 100644 contrib/credential/libsecret/.gitignore diff --git a/contrib/credential/libsecret/.gitignore b/contrib/credential/libsecret/.gitignore new file mode 100644 index 0000000000..4fa22359e2 --- /dev/null +++ b/contrib/credential/libsecret/.gitignore @@ -0,0 +1 @@ +git-credential-libsecret From patchwork Mon May 1 15:54:03 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 13227605 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id F40DFC77B73 for ; Mon, 1 May 2023 15:54:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232877AbjEAPye (ORCPT ); Mon, 1 May 2023 11:54:34 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39376 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232897AbjEAPyX (ORCPT ); Mon, 1 May 2023 11:54:23 -0400 Received: from mail-yw1-x112b.google.com (mail-yw1-x112b.google.com [IPv6:2607:f8b0:4864:20::112b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1ACA01BE3 for ; Mon, 1 May 2023 08:54:05 -0700 (PDT) Received: by mail-yw1-x112b.google.com with SMTP id 00721157ae682-54fae5e9ec7so37356127b3.1 for ; Mon, 01 May 2023 08:54:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20221208.gappssmtp.com; s=20221208; t=1682956445; x=1685548445; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=Wmtv5RU1wZ45UPVBfLEuHugAxBCPpEdNpIApXNNuQWU=; b=d2rZ7/JldvrRi1E7HvkiFm4O9o9GBKexyhUuytPmvQZ/qlL/SeAZjuM/vY+syyTDry 1M6i1eHsu855W64/e1BfGgGLZJJOYqvnlOPPakMyl9Afr+maArX/HREvXO7za2UF0Vuf lFkmVviHigSTCyL8dE8vpPmjoyI67BEgaxN+/wXxM9JEbsKab/fBrB0TyHpRvQ7lYbNc eDz0G4bSAcS5Lu/H0CUhl6TlIJsc4k0zOdiimiAz/b3o5yVQI/6YjmpElTkY+Vf1NPUL Z+uhDCYegB+wJSpp3sVHfHZLp/yt+u5omndb2sNHkLm7deNb0yuC5bioQYWfTlCMHZxF P05A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682956445; x=1685548445; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=Wmtv5RU1wZ45UPVBfLEuHugAxBCPpEdNpIApXNNuQWU=; b=S3TuBOURvuxla498zR2LZq3Dxni1kNHkZ9hzk1U8N08tsS+lYMhTHw5lOUSmkF56cA wvx/MMPo0fHLUXQRoiSFzBD5iRNk//hMSGThudOfv/KwK8myYUcvzu333amh/Nz4XZ6h hq50jlt4A1eRtGASHrQmfgNiXUwrShGL1jZ79pXMCTXK2ZEbCUvRgR8dVjcdcySiSfAX 1g/6ZeOi1QM9J2dvMTHSJ20w/CmNz7DNnIdxx2XesPd1xG3EnSYbtMRzBuZ9jxhZ9QGo p+6c46+Vkxfs/266+CJuYZUtDJam8RmQtGVFhv4xZA1zTrZkh+EZQOnGqPNYUIk7nfXt Sh9Q== X-Gm-Message-State: AC+VfDwQXTrW0C/4iaKrox2sITRop26MGzmMQ8+dPG4Yby0nYLZio3gy wRzspADumYxL8VFOCQiWv4oAExKOlItktpB770g+2A== X-Google-Smtp-Source: ACHHUZ4I2YDm/pe/hNVx2AErTBN6RCxWgu0r1V5nKpJCFB8ewrn0RRSay5NU6efwL36bCjCflZhreg== X-Received: by 2002:a0d:cc0e:0:b0:54f:bec1:c118 with SMTP id o14-20020a0dcc0e000000b0054fbec1c118mr13412289ywd.38.1682956444901; Mon, 01 May 2023 08:54:04 -0700 (PDT) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id c199-20020a814ed0000000b0055a777e3c50sm293216ywb.62.2023.05.01.08.54.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 01 May 2023 08:54:04 -0700 (PDT) Date: Mon, 1 May 2023 11:54:03 -0400 From: Taylor Blau To: git@vger.kernel.org Cc: Jeff King , Junio C Hamano , Matthew John Cheetham , Johannes Schindelin , Derrick Stolee Subject: [PATCH 6/7] contrib/credential: avoid fixed-size buffer in libsecret Message-ID: <9b9e2c0dd170eee96aa2c39eae061f8c92732bc4.1682956419.git.me@ttaylorr.com> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org The libsecret credential helper reads the newline-delimited protocol stream one line at a time by repeatedly calling fgets() into a fixed-size buffer, and is thus affected by the vulnerability described in the previous commit. To mitigate this attack, avoid using a fixed-size buffer, and instead rely on getline() to allocate a buffer as large as necessary to fit the entire content of the line, preventing any protocol injection. In most parts of Git we don't assume that every platform has getline(). But libsecret is primarily used on Linux, where we do already assume it (using a knob in config.mak.uname). POSIX also added getline() in 2008, so we'd expect other recent Unix-like operating systems to have it (e.g., FreeBSD also does). Note that the buffer was already allocated on the heap in this case, but we'll swap `g_free()` for `free()`, since it will now be allocated by the system `getline()`, rather than glib's `g_malloc()`. Tested-by: Jeff King Co-authored-by: Jeff King Signed-off-by: Jeff King Signed-off-by: Taylor Blau --- .../libsecret/git-credential-libsecret.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/contrib/credential/libsecret/git-credential-libsecret.c b/contrib/credential/libsecret/git-credential-libsecret.c index 2c5d76d789..ef681f29d5 100644 --- a/contrib/credential/libsecret/git-credential-libsecret.c +++ b/contrib/credential/libsecret/git-credential-libsecret.c @@ -244,17 +244,16 @@ static void credential_clear(struct credential *c) static int credential_read(struct credential *c) { - char *buf; - size_t line_len; + char *buf = NULL; + size_t alloc; + ssize_t line_len; char *key; char *value; - key = buf = g_malloc(1024); + while ((line_len = getline(&buf, &alloc, stdin)) > 0) { + key = buf; - while (fgets(buf, 1024, stdin)) { - line_len = strlen(buf); - - if (line_len && buf[line_len-1] == '\n') + if (buf[line_len-1] == '\n') buf[--line_len] = '\0'; if (!line_len) @@ -298,7 +297,7 @@ static int credential_read(struct credential *c) */ } - g_free(buf); + free(buf); return 0; } From patchwork Mon May 1 15:54:06 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 13227606 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 66483C77B7F for ; Mon, 1 May 2023 15:54:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232870AbjEAPyg (ORCPT ); Mon, 1 May 2023 11:54:36 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39218 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232859AbjEAPyZ (ORCPT ); Mon, 1 May 2023 11:54:25 -0400 Received: from mail-yb1-xb2d.google.com (mail-yb1-xb2d.google.com [IPv6:2607:f8b0:4864:20::b2d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D1C81172B for ; Mon, 1 May 2023 08:54:08 -0700 (PDT) Received: by mail-yb1-xb2d.google.com with SMTP id 3f1490d57ef6-b9d881ad689so4198039276.2 for ; Mon, 01 May 2023 08:54:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20221208.gappssmtp.com; s=20221208; t=1682956448; x=1685548448; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=fG7Yv4P5PQIN04A48RlcwOIWt1Cjuq+u1BBNvjRjdrk=; b=xobWlNYAYOU6PuqhsB1XLIwcZC907CtB0ohqgUoZJXmdhh60+hqk3naCGg5rNbJCfH wWciwbgoZ6wNt83HgSkFRUbG/fB31pNwjE2oVt3yr4hM17mUF0rYBg2WkbLVhOzhRN9K yBM5Rem5mn2UoP+zAdxWLIDpTSYrY/haGu3u+TEGUsIlZ4wvPUJD6qlEd/zy4tU6QS3w f8LUK6tIq2/6Dl2dqai8U0Sbud/w3X5kSAlZ2jbpQsSUNumGrunGMBF6xexwg9QUyhpj K4m3dn/gF+aQnVTYYqns50esVZuUKxsI1el4k/3MugNCSQdwb5wvWWXvMrATGw2rnfWu K82A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682956448; x=1685548448; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=fG7Yv4P5PQIN04A48RlcwOIWt1Cjuq+u1BBNvjRjdrk=; b=RzcOTDnRy5VWhI3WC9x8qrHV6bA6yX5VaaS3ZPu3Wkfn/qx5Oxd3SIz/AXab2N8bmp uZflKaN6kBJMiR0IiRIDgMJjzRaVC3o65H8WXVJOWnhbMIJfDOTif3v6uEUWCV98NMRz JDF7e5J6L+ZrFUxgtqIJc/ISIUpzIdfBYFex9PtIEIDn4hDXDwJgIsHbvWgkcD6XtXns kEdvhnROJKLzCjt2By2CRK9VjYVamTlD+JOz1qf04TPr6QBQDdIlSNPOts4bl+pciehq n2tLvkptyaO5BMBZ2M2fqTeQ93iA4Handne9BPTIs8kv3qclQ2L1FAP4ymu7sfaQRgSW GTQA== X-Gm-Message-State: AC+VfDyQGLCy7BcjVFaNWrt/e5dSKb87sVDuAOi59vhPeqjK4LnhMudV zWQrk6TJgOWBqVeqDnYViLl3dAymlNiwwjMaCN7xNA== X-Google-Smtp-Source: ACHHUZ7JM4H53iZyhhQ4CwY4XFN4fnGaHLpKi8JZipVKSPQJ6/KUU4kpfazi1RYZMVC104CL6MRgtg== X-Received: by 2002:a25:d04c:0:b0:b95:5682:7db0 with SMTP id h73-20020a25d04c000000b00b9556827db0mr15993141ybg.13.1682956447895; Mon, 01 May 2023 08:54:07 -0700 (PDT) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id d15-20020a25eb0f000000b00b8f52b11de6sm6760603ybs.42.2023.05.01.08.54.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 01 May 2023 08:54:07 -0700 (PDT) Date: Mon, 1 May 2023 11:54:06 -0400 From: Taylor Blau To: git@vger.kernel.org Cc: Jeff King , Junio C Hamano , Matthew John Cheetham , Johannes Schindelin , Derrick Stolee Subject: [PATCH 7/7] contrib/credential: embiggen fixed-size buffer in wincred Message-ID: <59a934a256a177e1d47f05a772f53f40ff015ced.1682956419.git.me@ttaylorr.com> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org As in previous commits, harden the wincred credential helper against the aforementioned protocol injection attack. Unlike the approached used for osxkeychain and libsecret, where a fixed-size buffer was replaced with `getline()`, we must take a different approach here. There is no `getline()` equivalent in Windows, and the function is not available to us with ordinary compiler settings. Instead, allocate a larger (still fixed-size) buffer in which to process each line. The value of 100 KiB is chosen to match the maximum-length header that curl will allow, CURL_MAX_HTTP_HEADER. To ensure that we are reading complete lines at a time, and that we aren't susceptible to a similar injection attack (albeit with more padding), ensure that each read terminates at a newline (i.e., that no line is more than 100 KiB long). Note that it isn't sufficient to turn the old loop into something like: while (len && strchr("\r\n", buf[len - 1])) { buf[--len] = 0; ends_in_newline = 1; } because if an attacker sends something like: [aaaaa.....]\r host=example.com\r\n the credential helper would fill its buffer after reading up through the first '\r', call fgets() again, and then see "host=example.com\r\n" on its line. Note that the original code was written in a way that would trim an arbitrary number of "\r" and "\n" from the end of the string. We should get only a single "\n" (since the point of `fgets()` is to return the buffer to us when it sees one), and likewise would not expect to see more than one associated "\r". The new code trims a single "\r\n", which matches the original intent. [1]: https://curl.se/libcurl/c/CURLOPT_HEADERFUNCTION.html Tested-by: Matthew John Cheetham Helped-by: Matthew John Cheetham Co-authored-by: Jeff King Signed-off-by: Jeff King Signed-off-by: Taylor Blau --- .../wincred/git-credential-wincred.c | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c index ead6e267c7..32ebef7f65 100644 --- a/contrib/credential/wincred/git-credential-wincred.c +++ b/contrib/credential/wincred/git-credential-wincred.c @@ -249,16 +249,27 @@ static WCHAR *utf8_to_utf16_dup(const char *str) return wstr; } +#define KB (1024) + static void read_credential(void) { - char buf[1024]; + size_t alloc = 100 * KB; + char *buf = calloc(alloc, sizeof(*buf)); - while (fgets(buf, sizeof(buf), stdin)) { + while (fgets(buf, alloc, stdin)) { char *v; - int len = strlen(buf); + size_t len = strlen(buf); + int ends_in_newline = 0; /* strip trailing CR / LF */ - while (len && strchr("\r\n", buf[len - 1])) + if (len && buf[len - 1] == '\n') { buf[--len] = 0; + ends_in_newline = 1; + } + if (len && buf[len - 1] == '\r') + buf[--len] = 0; + + if (!ends_in_newline) + die("bad input: %s", buf); if (!*buf) break; @@ -284,6 +295,8 @@ static void read_credential(void) * learn new lines, and the helpers are updated to match. */ } + + free(buf); } int main(int argc, char *argv[])