From 0d606d8c2a387189bd5cf453e64519c314283a96 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 9 Sep 2024 16:00:20 -0700 Subject: [PATCH 01/13] ci: remove 'Upload failed tests' directories' step from linux32 jobs Linux32 jobs seem to be getting: Error: This request has been automatically failed because it uses a deprecated version of `actions/upload-artifact: v1`. Learn more: https://github.blog/changelog/2024-02-13-deprecation-notice-v1-and-v2-of-the-artifact-actions/ before doing anything useful. For now, disable the step. Ever since actions/upload-artifact@v1 got disabled, mentioning the offending version of it seems to stop anything from happening. At least this should run the same build and test. See https://github.com/git/git/actions/runs/10780030750/job/29894867249 for example. [Backported from 90f2c7240cc (ci: remove 'Upload failed tests' directories' step from linux32 jobs, 2024-09-09).] Signed-off-by: Junio C Hamano Signed-off-by: Johannes Schindelin --- .github/workflows/main.yml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index bd390ab587..8eedf35011 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -334,12 +334,6 @@ jobs: with: name: failed-tests-${{matrix.vector.jobname}} path: ${{env.FAILED_TEST_ARTIFACTS}} - - name: Upload failed tests' directories - if: failure() && env.FAILED_TEST_ARTIFACTS != '' && matrix.vector.jobname == 'linux32' - uses: actions/upload-artifact@v1 # cannot be upgraded because Node.js Actions aren't supported in this container - with: - name: failed-tests-${{matrix.vector.jobname}} - path: ${{env.FAILED_TEST_ARTIFACTS}} static-analysis: needs: ci-config if: needs.ci-config.outputs.enabled == 'yes' From 5d828879f320501ba8fa75099ee01dc6f4c3cf77 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 19 Jun 2024 06:09:58 +0000 Subject: [PATCH 02/13] mingw: drop bogus (and unneeded) declaration of `_pgmptr` In 08809c09aa13 (mingw: add a helper function to attach GDB to the current process, 2020-02-13), I added a declaration that was not needed. Back then, that did not matter, but now that the declaration of that symbol was changed in mingw-w64's headers, it causes the following compile error: CC compat/mingw.o compat/mingw.c: In function 'open_in_gdb': compat/mingw.c:35:9: error: function declaration isn't a prototype [-Werror=strict-prototypes] 35 | extern char *_pgmptr; | ^~~~~~ In file included from C:/git-sdk-64/usr/src/git/build-installers/mingw64/lib/gcc/x86_64-w64-mingw32/14.1.0/include/mm_malloc.h:27, from C:/git-sdk-64/usr/src/git/build-installers/mingw64/lib/gcc/x86_64-w64-mingw32/14.1.0/include/xmmintrin.h:34, from C:/git-sdk-64/usr/src/git/build-installers/mingw64/lib/gcc/x86_64-w64-mingw32/14.1.0/include/immintrin.h:31, from C:/git-sdk-64/usr/src/git/build-installers/mingw64/lib/gcc/x86_64-w64-mingw32/14.1.0/include/x86intrin.h:32, from C:/git-sdk-64/usr/src/git/build-installers/mingw64/include/winnt.h:1658, from C:/git-sdk-64/usr/src/git/build-installers/mingw64/include/minwindef.h:163, from C:/git-sdk-64/usr/src/git/build-installers/mingw64/include/windef.h:9, from C:/git-sdk-64/usr/src/git/build-installers/mingw64/include/windows.h:69, from C:/git-sdk-64/usr/src/git/build-installers/mingw64/include/winsock2.h:23, from compat/../git-compat-util.h:215, from compat/mingw.c:1: compat/mingw.c:35:22: error: '__p__pgmptr' redeclared without dllimport attribute: previous dllimport ignored [-Werror=attributes] 35 | extern char *_pgmptr; | ^~~~~~~ Let's just drop the declaration and get rid of this compile error. [Backported from 3c295c87c25 (mingw: drop bogus (and unneeded) declaration of `_pgmptr`, 2024-06-19).] Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano Signed-off-by: Johannes Schindelin --- compat/mingw.c | 1 - 1 file changed, 1 deletion(-) diff --git a/compat/mingw.c b/compat/mingw.c index e433740381..e08db82635 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -20,7 +20,6 @@ static const int delay[] = { 0, 1, 10, 20, 40 }; void open_in_gdb(void) { static struct child_process cp = CHILD_PROCESS_INIT; - extern char *_pgmptr; strvec_pushl(&cp.args, "mintty", "gdb", NULL); strvec_pushf(&cp.args, "--pid=%d", getpid()); From 7e6073d27083054773f3c3b21a608f400cf7348d Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 11 May 2024 23:25:04 -0700 Subject: [PATCH 03/13] compat/regex: fix argument order to calloc(3) Windows compiler suddenly started complaining that calloc(3) takes its arguments in order. Indeed, there are many calls that has their arguments in a _wrong_ order. Fix them all. A sample breakage can be seen at https://github.com/git/git/actions/runs/9046793153/job/24857988702#step:4:272 [Backported from f01301aabe1 (compat/regex: fix argument order to calloc(3), 2024-05-11).] Signed-off-by: Junio C Hamano Signed-off-by: Johannes Schindelin --- compat/regex/regcomp.c | 12 ++++++------ compat/regex/regex_internal.c | 4 ++-- compat/regex/regexec.c | 10 +++++----- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/compat/regex/regcomp.c b/compat/regex/regcomp.c index d1bc09e49b..2bc0f1187a 100644 --- a/compat/regex/regcomp.c +++ b/compat/regex/regcomp.c @@ -868,7 +868,7 @@ init_dfa (re_dfa_t *dfa, size_t pat_len) if (table_size > pat_len) break; - dfa->state_table = calloc (sizeof (struct re_state_table_entry), table_size); + dfa->state_table = calloc (table_size, sizeof (struct re_state_table_entry)); dfa->state_hash_mask = table_size - 1; dfa->mb_cur_max = MB_CUR_MAX; @@ -936,7 +936,7 @@ init_dfa (re_dfa_t *dfa, size_t pat_len) { int i, j, ch; - dfa->sb_char = (re_bitset_ptr_t) calloc (sizeof (bitset_t), 1); + dfa->sb_char = (re_bitset_ptr_t) calloc (1, sizeof (bitset_t)); if (BE (dfa->sb_char == NULL, 0)) return REG_ESPACE; @@ -3079,9 +3079,9 @@ parse_bracket_exp (re_string_t *regexp, re_dfa_t *dfa, re_token_t *token, _NL_COLLATE_SYMB_EXTRAMB); } #endif - sbcset = (re_bitset_ptr_t) calloc (sizeof (bitset_t), 1); + sbcset = (re_bitset_ptr_t) calloc (1, sizeof (bitset_t)); #ifdef RE_ENABLE_I18N - mbcset = (re_charset_t *) calloc (sizeof (re_charset_t), 1); + mbcset = (re_charset_t *) calloc (1, sizeof (re_charset_t)); #endif /* RE_ENABLE_I18N */ #ifdef RE_ENABLE_I18N if (BE (sbcset == NULL || mbcset == NULL, 0)) @@ -3626,9 +3626,9 @@ build_charclass_op (re_dfa_t *dfa, RE_TRANSLATE_TYPE trans, re_token_t br_token; bin_tree_t *tree; - sbcset = (re_bitset_ptr_t) calloc (sizeof (bitset_t), 1); + sbcset = (re_bitset_ptr_t) calloc (1, sizeof (bitset_t)); #ifdef RE_ENABLE_I18N - mbcset = (re_charset_t *) calloc (sizeof (re_charset_t), 1); + mbcset = (re_charset_t *) calloc (1, sizeof (re_charset_t)); #endif /* RE_ENABLE_I18N */ #ifdef RE_ENABLE_I18N diff --git a/compat/regex/regex_internal.c b/compat/regex/regex_internal.c index ec51cf3446..ec5cc5d2dd 100644 --- a/compat/regex/regex_internal.c +++ b/compat/regex/regex_internal.c @@ -1628,7 +1628,7 @@ create_ci_newstate (const re_dfa_t *dfa, const re_node_set *nodes, reg_errcode_t err; re_dfastate_t *newstate; - newstate = (re_dfastate_t *) calloc (sizeof (re_dfastate_t), 1); + newstate = (re_dfastate_t *) calloc (1, sizeof (re_dfastate_t)); if (BE (newstate == NULL, 0)) return NULL; err = re_node_set_init_copy (&newstate->nodes, nodes); @@ -1678,7 +1678,7 @@ create_cd_newstate (const re_dfa_t *dfa, const re_node_set *nodes, reg_errcode_t err; re_dfastate_t *newstate; - newstate = (re_dfastate_t *) calloc (sizeof (re_dfastate_t), 1); + newstate = (re_dfastate_t *) calloc (1, sizeof (re_dfastate_t)); if (BE (newstate == NULL, 0)) return NULL; err = re_node_set_init_copy (&newstate->nodes, nodes); diff --git a/compat/regex/regexec.c b/compat/regex/regexec.c index 49358ae475..e92be5741d 100644 --- a/compat/regex/regexec.c +++ b/compat/regex/regexec.c @@ -2796,8 +2796,8 @@ get_subexp (re_match_context_t *mctx, int bkref_node, int bkref_str_idx) continue; /* No. */ if (sub_top->path == NULL) { - sub_top->path = calloc (sizeof (state_array_t), - sl_str - sub_top->str_idx + 1); + sub_top->path = calloc (sl_str - sub_top->str_idx + 1, + sizeof (state_array_t)); if (sub_top->path == NULL) return REG_ESPACE; } @@ -3361,7 +3361,7 @@ build_trtable (const re_dfa_t *dfa, re_dfastate_t *state) if (ndests == 0) { state->trtable = (re_dfastate_t **) - calloc (sizeof (re_dfastate_t *), SBC_MAX); + calloc (SBC_MAX, sizeof (re_dfastate_t *)); return 1; } return 0; @@ -3457,7 +3457,7 @@ out_free: discern by looking at the character code: allocate a 256-entry transition table. */ trtable = state->trtable = - (re_dfastate_t **) calloc (sizeof (re_dfastate_t *), SBC_MAX); + (re_dfastate_t **) calloc (SBC_MAX, sizeof (re_dfastate_t *)); if (BE (trtable == NULL, 0)) goto out_free; @@ -3488,7 +3488,7 @@ out_free: transition tables, one starting at trtable[0] and one starting at trtable[SBC_MAX]. */ trtable = state->word_trtable = - (re_dfastate_t **) calloc (sizeof (re_dfastate_t *), 2 * SBC_MAX); + (re_dfastate_t **) calloc (2 * SBC_MAX, sizeof (re_dfastate_t *)); if (BE (trtable == NULL, 0)) goto out_free; From 83b08eb19f05710a92d565124606dcaf68bcc68b Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 9 Oct 2024 15:25:18 +0200 Subject: [PATCH 04/13] t7300: work around platform-specific behaviour with long paths on MinGW Windows by default has a restriction in place to only allow paths up to 260 characters. This restriction can nowadays be lifted by setting a registry key, but is still active by default. In t7300 we have one test that exercises the behaviour of git-clean(1) with such long paths. Interestingly enough, this test fails on my system that uses Windows 10 with mingw-w64 installed via MSYS2: instead of observing ENAMETOOLONG, we observe ENOENT. This behaviour is consistent across multiple different environments I have tried. I cannot say why exactly we observe a different error here, but I would not be surprised if this was either dependent on the Windows version, the version of MinGW, the current working directory of Git or any kind of combination of these. Work around the issue by handling both errors. [Backported from 106834e34a2 (t7300: work around platform-specific behaviour with long paths on MinGW, 2024-10-09).] Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano Signed-off-by: Johannes Schindelin --- t/t7300-clean.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh index c975eb54d2..a831438f74 100755 --- a/t/t7300-clean.sh +++ b/t/t7300-clean.sh @@ -735,7 +735,7 @@ test_expect_success MINGW 'handle clean & core.longpaths = false nicely' ' test_must_fail git clean -xdf 2>.git/err && # grepping for a strerror string is unportable but it is OK here with # MINGW prereq - test_i18ngrep "too long" .git/err + test_i18ngrep -e "too long" -e "No such file or directory" .git/err ' test_expect_success 'clean untracked paths by pathspec' ' From c903985bf7e772e2d08275c1a95c8a55ab011577 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 7 Nov 2024 08:57:52 +0100 Subject: [PATCH 05/13] credential_format(): also encode [:] An upcoming change wants to sanitize the credential password prompt where a URL is displayed that may potentially come from a `.gitmodules` file. To this end, the `credential_format()` function is employed. To sanitize the host name (and optional port) part of the URL, we need a new mode of the `strbuf_add_percentencode()` function because the current mode is both too strict and too lenient: too strict because it encodes `:`, `[` and `]` (which should be left unencoded in `:` and in IPv6 addresses), and too lenient because it does not encode invalid host name characters `/`, `_` and `~`. So let's introduce and use a new mode specifically to encode the host name and optional port part of a URI, leaving alpha-numerical characters, periods, colons and brackets alone and encoding all others. This only leads to a change of behavior for URLs that contain invalid host names. Signed-off-by: Johannes Schindelin --- credential.c | 3 ++- strbuf.c | 4 +++- strbuf.h | 1 + t/t0300-credentials.sh | 13 +++++++++++++ 4 files changed, 19 insertions(+), 2 deletions(-) diff --git a/credential.c b/credential.c index f32011343f..572f1785da 100644 --- a/credential.c +++ b/credential.c @@ -164,7 +164,8 @@ static void credential_format(struct credential *c, struct strbuf *out) strbuf_addch(out, '@'); } if (c->host) - strbuf_addstr(out, c->host); + strbuf_add_percentencode(out, c->host, + STRBUF_ENCODE_HOST_AND_PORT); if (c->path) { strbuf_addch(out, '/'); strbuf_add_percentencode(out, c->path, 0); diff --git a/strbuf.c b/strbuf.c index c383f41a3c..756b96c561 100644 --- a/strbuf.c +++ b/strbuf.c @@ -492,7 +492,9 @@ void strbuf_add_percentencode(struct strbuf *dst, const char *src, int flags) unsigned char ch = src[i]; if (ch <= 0x1F || ch >= 0x7F || (ch == '/' && (flags & STRBUF_ENCODE_SLASH)) || - strchr(URL_UNSAFE_CHARS, ch)) + ((flags & STRBUF_ENCODE_HOST_AND_PORT) ? + !isalnum(ch) && !strchr("-.:[]", ch) : + !!strchr(URL_UNSAFE_CHARS, ch))) strbuf_addf(dst, "%%%02X", (unsigned char)ch); else strbuf_addch(dst, ch); diff --git a/strbuf.h b/strbuf.h index f6dbb9681e..f9f8bb0381 100644 --- a/strbuf.h +++ b/strbuf.h @@ -380,6 +380,7 @@ size_t strbuf_expand_dict_cb(struct strbuf *sb, void strbuf_addbuf_percentquote(struct strbuf *dst, const struct strbuf *src); #define STRBUF_ENCODE_SLASH 1 +#define STRBUF_ENCODE_HOST_AND_PORT 2 /** * Append the contents of a string to a strbuf, percent-encoding any characters diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh index c66d91e82d..cb91be1427 100755 --- a/t/t0300-credentials.sh +++ b/t/t0300-credentials.sh @@ -514,6 +514,19 @@ test_expect_success 'match percent-encoded values in username' ' EOF ' +test_expect_success 'match percent-encoded values in hostname' ' + test_config "credential.https://a%20b%20c/.helper" "$HELPER" && + check fill <<-\EOF + url=https://a b c/ + -- + protocol=https + host=a b c + username=foo + password=bar + -- + EOF +' + test_expect_success 'fetch with multiple path components' ' test_unconfig credential.helper && test_config credential.https://example.com/foo/repo.git.helper "verbatim foo bar" && From 7725b8100ffbbff2750ee4d61a0fcc1f53a086e8 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 30 Oct 2024 13:26:10 +0100 Subject: [PATCH 06/13] credential: sanitize the user prompt When asking the user interactively for credentials, we want to avoid misleading them e.g. via control sequences that pretend that the URL targets a trusted host when it does not. While Git learned, over the course of the preceding commits, to disallow URLs containing URL-encoded control characters by default, credential helpers are still allowed to specify values very freely (apart from Line Feed and NUL characters, anything is allowed), and this would allow, say, a username containing control characters to be specified that would then be displayed in the interactive terminal prompt asking the user for the password, potentially sending those control characters directly to the terminal. This is undesirable because control characters can be used to mislead users to divulge secret information to untrusted sites. To prevent such an attack vector, let's add a `git_prompt()` that forces the displayed text to be sanitized, i.e. displaying question marks instead of control characters. Note: While this commit's diff changes a lot of `user@host` strings to `user%40host`, which may look suspicious on the surface, there is a good reason for that: this string specifies a user name, not a @ combination! In the context of t5541, the actual combination looks like this: `user%40@127.0.0.1:5541`. Therefore, these string replacements document a net improvement introduced by this commit, as `user@host@127.0.0.1` could have left readers wondering where the user name ends and where the host name begins. Hinted-at-by: Jeff King Signed-off-by: Johannes Schindelin --- Documentation/config/credential.txt | 6 ++++++ credential.c | 7 ++++++- credential.h | 4 +++- t/t0300-credentials.sh | 20 ++++++++++++++++++++ t/t5541-http-push-smart.sh | 6 +++--- t/t5550-http-fetch-dumb.sh | 14 +++++++------- t/t5551-http-fetch-smart.sh | 16 ++++++++-------- 7 files changed, 53 insertions(+), 20 deletions(-) diff --git a/Documentation/config/credential.txt b/Documentation/config/credential.txt index 512f31876e..fd8113d6d4 100644 --- a/Documentation/config/credential.txt +++ b/Documentation/config/credential.txt @@ -14,6 +14,12 @@ credential.useHttpPath:: or https URL to be important. Defaults to false. See linkgit:gitcredentials[7] for more information. +credential.sanitizePrompt:: + By default, user names and hosts that are shown as part of the + password prompt are not allowed to contain control characters (they + will be URL-encoded by default). Configure this setting to `false` to + override that behavior. + credential.username:: If no username is set for a network authentication, use this username by default. See credential..* below, and diff --git a/credential.c b/credential.c index 572f1785da..1392a54d5c 100644 --- a/credential.c +++ b/credential.c @@ -67,6 +67,8 @@ static int credential_config_callback(const char *var, const char *value, } else if (!strcmp(key, "usehttppath")) c->use_http_path = git_config_bool(var, value); + else if (!strcmp(key, "sanitizeprompt")) + c->sanitize_prompt = git_config_bool(var, value); return 0; } @@ -179,7 +181,10 @@ static char *credential_ask_one(const char *what, struct credential *c, struct strbuf prompt = STRBUF_INIT; char *r; - credential_describe(c, &desc); + if (c->sanitize_prompt) + credential_format(c, &desc); + else + credential_describe(c, &desc); if (desc.len) strbuf_addf(&prompt, "%s for '%s': ", what, desc.buf); else diff --git a/credential.h b/credential.h index 935b28a70f..0364d436d2 100644 --- a/credential.h +++ b/credential.h @@ -119,7 +119,8 @@ struct credential { configured:1, quit:1, use_http_path:1, - username_from_proto:1; + username_from_proto:1, + sanitize_prompt:1; char *username; char *password; @@ -132,6 +133,7 @@ struct credential { #define CREDENTIAL_INIT { \ .helpers = STRING_LIST_INIT_DUP, \ .password_expiry_utc = TIME_MAX, \ + .sanitize_prompt = 1, \ } /* Initialize a credential structure, setting all fields to empty. */ diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh index cb91be1427..b62c70c193 100755 --- a/t/t0300-credentials.sh +++ b/t/t0300-credentials.sh @@ -45,6 +45,10 @@ test_expect_success 'setup helper scripts' ' test -z "$pexpiry" || echo password_expiry_utc=$pexpiry EOF + write_script git-credential-cntrl-in-username <<-\EOF && + printf "username=\\007latrix Lestrange\\n" + EOF + PATH="$PWD:$PATH" ' @@ -825,4 +829,20 @@ test_expect_success 'credential config with partial URLs' ' test_i18ngrep "skipping credential lookup for key" stderr ' +BEL="$(printf '\007')" + +test_expect_success 'interactive prompt is sanitized' ' + check fill cntrl-in-username <<-EOF + protocol=https + host=example.org + -- + protocol=https + host=example.org + username=${BEL}latrix Lestrange + password=askpass-password + -- + askpass: Password for ${SQ}https://%07latrix%20Lestrange@example.org${SQ}: + EOF +' + test_done diff --git a/t/t5541-http-push-smart.sh b/t/t5541-http-push-smart.sh index d0211cd8be..2cd2e1a059 100755 --- a/t/t5541-http-push-smart.sh +++ b/t/t5541-http-push-smart.sh @@ -351,7 +351,7 @@ test_expect_success 'push over smart http with auth' ' git push "$HTTPD_URL"/auth/smart/test_repo.git && git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git" \ log -1 --format=%s >actual && - expect_askpass both user@host && + expect_askpass both user%40host && test_cmp expect actual ' @@ -363,7 +363,7 @@ test_expect_success 'push to auth-only-for-push repo' ' git push "$HTTPD_URL"/auth-push/smart/test_repo.git && git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git" \ log -1 --format=%s >actual && - expect_askpass both user@host && + expect_askpass both user%40host && test_cmp expect actual ' @@ -393,7 +393,7 @@ test_expect_success 'push into half-auth-complete requires password' ' git push "$HTTPD_URL/half-auth-complete/smart/half-auth.git" && git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/half-auth.git" \ log -1 --format=%s >actual && - expect_askpass both user@host && + expect_askpass both user%40host && test_cmp expect actual ' diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh index 8f182a3cbf..5d0e394609 100755 --- a/t/t5550-http-fetch-dumb.sh +++ b/t/t5550-http-fetch-dumb.sh @@ -90,13 +90,13 @@ test_expect_success 'http auth can use user/pass in URL' ' test_expect_success 'http auth can use just user in URL' ' set_askpass wrong pass@host && git clone "$HTTPD_URL_USER/auth/dumb/repo.git" clone-auth-pass && - expect_askpass pass user@host + expect_askpass pass user%40host ' test_expect_success 'http auth can request both user and pass' ' set_askpass user@host pass@host && git clone "$HTTPD_URL/auth/dumb/repo.git" clone-auth-both && - expect_askpass both user@host + expect_askpass both user%40host ' test_expect_success 'http auth respects credential helper config' ' @@ -114,14 +114,14 @@ test_expect_success 'http auth can get username from config' ' test_config_global "credential.$HTTPD_URL.username" user@host && set_askpass wrong pass@host && git clone "$HTTPD_URL/auth/dumb/repo.git" clone-auth-user && - expect_askpass pass user@host + expect_askpass pass user%40host ' test_expect_success 'configured username does not override URL' ' test_config_global "credential.$HTTPD_URL.username" wrong && set_askpass wrong pass@host && git clone "$HTTPD_URL_USER/auth/dumb/repo.git" clone-auth-user2 && - expect_askpass pass user@host + expect_askpass pass user%40host ' test_expect_success 'set up repo with http submodules' ' @@ -142,7 +142,7 @@ test_expect_success 'cmdline credential config passes to submodule via clone' ' set_askpass wrong pass@host && git -c "credential.$HTTPD_URL.username=user@host" \ clone --recursive super super-clone && - expect_askpass pass user@host + expect_askpass pass user%40host ' test_expect_success 'cmdline credential config passes submodule via fetch' ' @@ -153,7 +153,7 @@ test_expect_success 'cmdline credential config passes submodule via fetch' ' git -C super-clone \ -c "credential.$HTTPD_URL.username=user@host" \ fetch --recurse-submodules && - expect_askpass pass user@host + expect_askpass pass user%40host ' test_expect_success 'cmdline credential config passes submodule update' ' @@ -170,7 +170,7 @@ test_expect_success 'cmdline credential config passes submodule update' ' git -C super-clone \ -c "credential.$HTTPD_URL.username=user@host" \ submodule update && - expect_askpass pass user@host + expect_askpass pass user%40host ' test_expect_success 'fetch changes via http' ' diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh index 0908534f25..8a27768dfb 100755 --- a/t/t5551-http-fetch-smart.sh +++ b/t/t5551-http-fetch-smart.sh @@ -181,7 +181,7 @@ test_expect_success 'clone from password-protected repository' ' echo two >expect && set_askpass user@host pass@host && git clone --bare "$HTTPD_URL/auth/smart/repo.git" smart-auth && - expect_askpass both user@host && + expect_askpass both user%40host && git --git-dir=smart-auth log -1 --format=%s >actual && test_cmp expect actual ' @@ -199,7 +199,7 @@ test_expect_success 'clone from auth-only-for-objects repository' ' echo two >expect && set_askpass user@host pass@host && git clone --bare "$HTTPD_URL/auth-fetch/smart/repo.git" half-auth && - expect_askpass both user@host && + expect_askpass both user%40host && git --git-dir=half-auth log -1 --format=%s >actual && test_cmp expect actual ' @@ -224,14 +224,14 @@ test_expect_success 'redirects send auth to new location' ' set_askpass user@host pass@host && git -c credential.useHttpPath=true \ clone $HTTPD_URL/smart-redir-auth/repo.git repo-redir-auth && - expect_askpass both user@host auth/smart/repo.git + expect_askpass both user%40host auth/smart/repo.git ' test_expect_success 'GIT_TRACE_CURL redacts auth details' ' rm -rf redact-auth trace && set_askpass user@host pass@host && GIT_TRACE_CURL="$(pwd)/trace" git clone --bare "$HTTPD_URL/auth/smart/repo.git" redact-auth && - expect_askpass both user@host && + expect_askpass both user%40host && # Ensure that there is no "Basic" followed by a base64 string, but that # the auth details are redacted @@ -243,7 +243,7 @@ test_expect_success 'GIT_CURL_VERBOSE redacts auth details' ' rm -rf redact-auth trace && set_askpass user@host pass@host && GIT_CURL_VERBOSE=1 git clone --bare "$HTTPD_URL/auth/smart/repo.git" redact-auth 2>trace && - expect_askpass both user@host && + expect_askpass both user%40host && # Ensure that there is no "Basic" followed by a base64 string, but that # the auth details are redacted @@ -256,7 +256,7 @@ test_expect_success 'GIT_TRACE_CURL does not redact auth details if GIT_TRACE_RE set_askpass user@host pass@host && GIT_TRACE_REDACT=0 GIT_TRACE_CURL="$(pwd)/trace" \ git clone --bare "$HTTPD_URL/auth/smart/repo.git" redact-auth && - expect_askpass both user@host && + expect_askpass both user%40host && grep -i "Authorization: Basic [0-9a-zA-Z+/]" trace ' @@ -568,7 +568,7 @@ test_expect_success 'http auth remembers successful credentials' ' # the first request prompts the user... set_askpass user@host pass@host && git ls-remote "$HTTPD_URL/auth/smart/repo.git" >/dev/null && - expect_askpass both user@host && + expect_askpass both user%40host && # ...and the second one uses the stored value rather than # prompting the user. @@ -599,7 +599,7 @@ test_expect_success 'http auth forgets bogus credentials' ' # us to prompt the user again. set_askpass user@host pass@host && git ls-remote "$HTTPD_URL/auth/smart/repo.git" >/dev/null && - expect_askpass both user@host + expect_askpass both user%40host ' test_expect_success 'client falls back from v2 to v0 to match server' ' From b01b9b81d36759cdcd07305e78765199e1bc2060 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 4 Nov 2024 14:48:22 +0100 Subject: [PATCH 07/13] credential: disallow Carriage Returns in the protocol by default While Git has documented that the credential protocol is line-based, with newlines as terminators, the exact shape of a newline has not been documented. From Git's perspective, which is firmly rooted in the Linux ecosystem, it is clear that "a newline" means a Line Feed character. However, even Git's credential protocol respects Windows line endings (a Carriage Return character followed by a Line Feed character, "CR/LF") by virtue of using `strbuf_getline()`. There is a third category of line endings that has been used originally by MacOS, and that is respected by the default line readers of .NET and node.js: bare Carriage Returns. Git cannot handle those, and what is worse: Git's remedy against CVE-2020-5260 does not catch when credential helpers are used that interpret bare Carriage Returns as newlines. Git Credential Manager addressed this as CVE-2024-50338, but other credential helpers may still be vulnerable. So let's not only disallow Line Feed characters as part of the values in the credential protocol, but also disallow Carriage Return characters. In the unlikely event that a credential helper relies on Carriage Returns in the protocol, introduce an escape hatch via the `credential.protectProtocol` config setting. This addresses CVE-2024-52006. Signed-off-by: Johannes Schindelin --- Documentation/config/credential.txt | 5 +++++ credential.c | 21 ++++++++++++++------- credential.h | 4 +++- t/t0300-credentials.sh | 16 ++++++++++++++++ 4 files changed, 38 insertions(+), 8 deletions(-) diff --git a/Documentation/config/credential.txt b/Documentation/config/credential.txt index fd8113d6d4..9cadca7f73 100644 --- a/Documentation/config/credential.txt +++ b/Documentation/config/credential.txt @@ -20,6 +20,11 @@ credential.sanitizePrompt:: will be URL-encoded by default). Configure this setting to `false` to override that behavior. +credential.protectProtocol:: + By default, Carriage Return characters are not allowed in the protocol + that is used when Git talks to a credential helper. This setting allows + users to override this default. + credential.username:: If no username is set for a network authentication, use this username by default. See credential..* below, and diff --git a/credential.c b/credential.c index 1392a54d5c..b76a730901 100644 --- a/credential.c +++ b/credential.c @@ -69,6 +69,8 @@ static int credential_config_callback(const char *var, const char *value, c->use_http_path = git_config_bool(var, value); else if (!strcmp(key, "sanitizeprompt")) c->sanitize_prompt = git_config_bool(var, value); + else if (!strcmp(key, "protectprotocol")) + c->protect_protocol = git_config_bool(var, value); return 0; } @@ -262,7 +264,8 @@ int credential_read(struct credential *c, FILE *fp) return 0; } -static void credential_write_item(FILE *fp, const char *key, const char *value, +static void credential_write_item(const struct credential *c, + FILE *fp, const char *key, const char *value, int required) { if (!value && required) @@ -271,19 +274,23 @@ static void credential_write_item(FILE *fp, const char *key, const char *value, return; if (strchr(value, '\n')) die("credential value for %s contains newline", key); + if (c->protect_protocol && strchr(value, '\r')) + die("credential value for %s contains carriage return\n" + "If this is intended, set `credential.protectProtocol=false`", + key); fprintf(fp, "%s=%s\n", key, value); } void credential_write(const struct credential *c, FILE *fp) { - credential_write_item(fp, "protocol", c->protocol, 1); - credential_write_item(fp, "host", c->host, 1); - credential_write_item(fp, "path", c->path, 0); - credential_write_item(fp, "username", c->username, 0); - credential_write_item(fp, "password", c->password, 0); + credential_write_item(c, fp, "protocol", c->protocol, 1); + credential_write_item(c, fp, "host", c->host, 1); + credential_write_item(c, fp, "path", c->path, 0); + credential_write_item(c, fp, "username", c->username, 0); + credential_write_item(c, fp, "password", c->password, 0); if (c->password_expiry_utc != TIME_MAX) { char *s = xstrfmt("%"PRItime, c->password_expiry_utc); - credential_write_item(fp, "password_expiry_utc", s, 0); + credential_write_item(c, fp, "password_expiry_utc", s, 0); free(s); } } diff --git a/credential.h b/credential.h index 0364d436d2..2c0b39a925 100644 --- a/credential.h +++ b/credential.h @@ -120,7 +120,8 @@ struct credential { quit:1, use_http_path:1, username_from_proto:1, - sanitize_prompt:1; + sanitize_prompt:1, + protect_protocol:1; char *username; char *password; @@ -134,6 +135,7 @@ struct credential { .helpers = STRING_LIST_INIT_DUP, \ .password_expiry_utc = TIME_MAX, \ .sanitize_prompt = 1, \ + .protect_protocol = 1, \ } /* Initialize a credential structure, setting all fields to empty. */ diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh index b62c70c193..168ae76550 100755 --- a/t/t0300-credentials.sh +++ b/t/t0300-credentials.sh @@ -720,6 +720,22 @@ test_expect_success 'url parser rejects embedded newlines' ' test_cmp expect stderr ' +test_expect_success 'url parser rejects embedded carriage returns' ' + test_config credential.helper "!true" && + test_must_fail git credential fill 2>stderr <<-\EOF && + url=https://example%0d.com/ + EOF + cat >expect <<-\EOF && + fatal: credential value for host contains carriage return + If this is intended, set `credential.protectProtocol=false` + EOF + test_cmp expect stderr && + GIT_ASKPASS=true \ + git -c credential.protectProtocol=false credential fill <<-\EOF + url=https://example%0d.com/ + EOF +' + test_expect_success 'host-less URLs are parsed as empty host' ' check fill "verbatim foo bar" <<-\EOF url=cert:///path/to/cert.pem From 54a3711a9dd968a04249beef157393d64b579d64 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 30 Oct 2024 00:17:53 +0100 Subject: [PATCH 08/13] Git 2.40.4 Signed-off-by: Johannes Schindelin --- Documentation/RelNotes/2.40.4.txt | 5 +++++ GIT-VERSION-GEN | 2 +- RelNotes | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) create mode 100644 Documentation/RelNotes/2.40.4.txt diff --git a/Documentation/RelNotes/2.40.4.txt b/Documentation/RelNotes/2.40.4.txt new file mode 100644 index 0000000000..0ff29f3cfc --- /dev/null +++ b/Documentation/RelNotes/2.40.4.txt @@ -0,0 +1,5 @@ +Git v2.40.4 Release Notes +========================= + +This release lets Git refuse to accept URLs that contain control +sequences. This addresses CVE-2024-50349 and CVE-2024-52006. diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index b345e89cbe..4f4d294651 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,7 +1,7 @@ #!/bin/sh GVF=GIT-VERSION-FILE -DEF_VER=v2.40.3 +DEF_VER=v2.40.4 LF=' ' diff --git a/RelNotes b/RelNotes index afb47e28e0..b401038a56 120000 --- a/RelNotes +++ b/RelNotes @@ -1 +1 @@ -Documentation/RelNotes/2.40.3.txt \ No newline at end of file +Documentation/RelNotes/2.40.4.txt \ No newline at end of file From 6fd641a521a722e438b291ec7b852b7bb508b18b Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 30 Oct 2024 00:27:54 +0100 Subject: [PATCH 09/13] Git 2.41.3 Signed-off-by: Johannes Schindelin --- Documentation/RelNotes/2.41.3.txt | 6 ++++++ GIT-VERSION-GEN | 2 +- RelNotes | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 Documentation/RelNotes/2.41.3.txt diff --git a/Documentation/RelNotes/2.41.3.txt b/Documentation/RelNotes/2.41.3.txt new file mode 100644 index 0000000000..b5aba88790 --- /dev/null +++ b/Documentation/RelNotes/2.41.3.txt @@ -0,0 +1,6 @@ +Git v2.41.3 Release Notes +========================= + +This release merges up the fix that appears in v2.40.4 to address +the security issues CVE-2024-50349 and CVE-2024-52006; see the +release notes for that version for details. diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index 990ca9c643..664b521986 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,7 +1,7 @@ #!/bin/sh GVF=GIT-VERSION-FILE -DEF_VER=v2.41.2 +DEF_VER=v2.41.3 LF=' ' diff --git a/RelNotes b/RelNotes index 04f6c71310..5bd6fdad36 120000 --- a/RelNotes +++ b/RelNotes @@ -1 +1 @@ -Documentation/RelNotes/2.41.2.txt \ No newline at end of file +Documentation/RelNotes/2.41.3.txt \ No newline at end of file From 54ddf17f827969c989576f2411d9ff519fa8091f Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 30 Oct 2024 00:39:06 +0100 Subject: [PATCH 10/13] Git 2.42.4 Signed-off-by: Johannes Schindelin --- Documentation/RelNotes/2.42.4.txt | 6 ++++++ GIT-VERSION-GEN | 2 +- RelNotes | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 Documentation/RelNotes/2.42.4.txt diff --git a/Documentation/RelNotes/2.42.4.txt b/Documentation/RelNotes/2.42.4.txt new file mode 100644 index 0000000000..3129d76e75 --- /dev/null +++ b/Documentation/RelNotes/2.42.4.txt @@ -0,0 +1,6 @@ +Git v2.42.4 Release Notes +========================= + +This release merges up the fix that appears in v2.40.4 and v2.41.3 +to address the security issues CVE-2024-50349 and CVE-2024-52006; +see the release notes for these versions for details. diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index 070d22c6d1..69948fce1b 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,7 +1,7 @@ #!/bin/sh GVF=GIT-VERSION-FILE -DEF_VER=v2.42.3 +DEF_VER=v2.42.4 LF=' ' diff --git a/RelNotes b/RelNotes index 7af3372ac2..e0d3afd77a 120000 --- a/RelNotes +++ b/RelNotes @@ -1 +1 @@ -Documentation/RelNotes/2.42.3.txt \ No newline at end of file +Documentation/RelNotes/2.42.4.txt \ No newline at end of file From 664d4fa692cb8637a7c9297c94abf0de8593e585 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 30 Oct 2024 00:52:18 +0100 Subject: [PATCH 11/13] Git 2.43.6 Signed-off-by: Johannes Schindelin --- Documentation/RelNotes/2.43.6.txt | 7 +++++++ GIT-VERSION-GEN | 2 +- RelNotes | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 Documentation/RelNotes/2.43.6.txt diff --git a/Documentation/RelNotes/2.43.6.txt b/Documentation/RelNotes/2.43.6.txt new file mode 100644 index 0000000000..2114b9f78d --- /dev/null +++ b/Documentation/RelNotes/2.43.6.txt @@ -0,0 +1,7 @@ +Git v2.43.6 Release Notes +========================= + +This release merges up the fix that appears in v2.40.4, v2.41.3 +and v2.42.4 to address the security issues CVE-2024-50349 and +CVE-2024-52006; see the release notes for these versions for +details. diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index fcaa390a61..81630dde84 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,7 +1,7 @@ #!/bin/sh GVF=GIT-VERSION-FILE -DEF_VER=v2.43.5 +DEF_VER=v2.43.6 LF=' ' diff --git a/RelNotes b/RelNotes index 1abe69c298..0a9d8a03d7 120000 --- a/RelNotes +++ b/RelNotes @@ -1 +1 @@ -Documentation/RelNotes/2.43.5.txt \ No newline at end of file +Documentation/RelNotes/2.43.6.txt \ No newline at end of file From 2f323bb16219c105e0c576ea4c2ece9863f5d926 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 30 Oct 2024 00:55:00 +0100 Subject: [PATCH 12/13] Git 2.44.3 Signed-off-by: Johannes Schindelin --- Documentation/RelNotes/2.44.3.txt | 7 +++++++ GIT-VERSION-GEN | 2 +- RelNotes | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 Documentation/RelNotes/2.44.3.txt diff --git a/Documentation/RelNotes/2.44.3.txt b/Documentation/RelNotes/2.44.3.txt new file mode 100644 index 0000000000..5862845458 --- /dev/null +++ b/Documentation/RelNotes/2.44.3.txt @@ -0,0 +1,7 @@ +Git v2.44.3 Release Notes +========================= + +This release merges up the fix that appears in v2.40.4, v2.41.3, +v2.42.4 and v2.43.6 to address the security issues CVE-2024-50349 +and CVE-2024-52006; see the release notes for these versions +for details. diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index 9c631da5ce..33476e262d 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,7 +1,7 @@ #!/bin/sh GVF=GIT-VERSION-FILE -DEF_VER=v2.44.2 +DEF_VER=v2.44.3 LF=' ' diff --git a/RelNotes b/RelNotes index 6ba9e6f4ce..509eba5f1a 120000 --- a/RelNotes +++ b/RelNotes @@ -1 +1 @@ -Documentation/RelNotes/2.44.2.txt \ No newline at end of file +Documentation/RelNotes/2.44.3.txt \ No newline at end of file From fc16eb306c2ad25c080612ae85fdeba1fbcbfc98 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 30 Oct 2024 00:59:32 +0100 Subject: [PATCH 13/13] Git 2.45.3 Signed-off-by: Johannes Schindelin --- Documentation/RelNotes/2.45.3.txt | 7 ++++++- GIT-VERSION-GEN | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/Documentation/RelNotes/2.45.3.txt b/Documentation/RelNotes/2.45.3.txt index 2a1e9aa608..ddb3cb694b 100644 --- a/Documentation/RelNotes/2.45.3.txt +++ b/Documentation/RelNotes/2.45.3.txt @@ -1,7 +1,12 @@ Git v2.45.3 Release Notes ========================= -This primarily is to backport various small fixes accumulated on the +This release merges up the fix that appears in v2.40.4, v2.41.3, +v2.42.4, v2.43.6 and v2.44.3 to address the security issues +CVE-2024-50349 and CVE-2024-52006; see the release notes for +these versions for details. + +This version also backports various small fixes accumulated on the 'master' front during the development towards Git 2.46, the next feature release. diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index eb53b0ee01..f7c5d8f070 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,7 +1,7 @@ #!/bin/sh GVF=GIT-VERSION-FILE -DEF_VER=v2.45.2 +DEF_VER=v2.45.3 LF=' '