From 93a9044ff663684da278d4d0fc5111a293e6555b Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 31 Jan 2020 08:49:34 +0100 Subject: fetchhead: strip credentials from remote URL If fetching from an anonymous remote via its URL, then the URL gets written into the FETCH_HEAD reference. This is mainly done to give valuable context to some commands, like for example git-merge(1), which will put the URL into the generated MERGE_MSG. As a result, what gets written into FETCH_HEAD may become public in some cases. This is especially important considering that URLs may contain credentials, e.g. when cloning 'https://foo:bar@example.com/repo' we persist the complete URL into FETCH_HEAD and put it without any kind of sanitization into the MERGE_MSG. This is obviously bad, as your login data has now just leaked as soon as you do git-push(1). When writing the URL into FETCH_HEAD, upstream git does strip credentials first. Let's do the same by trying to parse the remote URL as a "real" URL, removing any credentials and then re-formatting the URL. In case this fails, e.g. when it's a file path or not a valid URL, we just fall back to using the URL as-is without any sanitization. Add tests to verify our behaviour. --- src/fetchhead.c | 38 +++++++++++++++++++++++++++++++++++--- tests/fetchhead/nonetwork.c | 20 +++++++++++++++++++- tests/online/fetchhead.c | 17 +++++++++++++++++ 3 files changed, 71 insertions(+), 4 deletions(-) diff --git a/src/fetchhead.c b/src/fetchhead.c index a3f6fbc7a..ea610f39a 100644 --- a/src/fetchhead.c +++ b/src/fetchhead.c @@ -14,6 +14,7 @@ #include "futils.h" #include "filebuf.h" #include "refs.h" +#include "net.h" #include "repository.h" int git_fetchhead_ref_cmp(const void *a, const void *b) @@ -36,6 +37,33 @@ int git_fetchhead_ref_cmp(const void *a, const void *b) return 0; } +static char *sanitized_remote_url(const char *remote_url) +{ + git_net_url url = GIT_NET_URL_INIT; + char *sanitized = NULL; + int error; + + if (git_net_url_parse(&url, remote_url) == 0) { + git_buf buf = GIT_BUF_INIT; + + git__free(url.username); + git__free(url.password); + url.username = url.password = NULL; + + if ((error = git_net_url_fmt(&buf, &url)) < 0) + goto fallback; + + sanitized = git_buf_detach(&buf); + } + +fallback: + if (!sanitized) + sanitized = git__strdup(remote_url); + + git_net_url_dispose(&url); + return sanitized; +} + int git_fetchhead_ref_create( git_fetchhead_ref **out, git_oid *oid, @@ -57,11 +85,15 @@ int git_fetchhead_ref_create( git_oid_cpy(&fetchhead_ref->oid, oid); fetchhead_ref->is_merge = is_merge; - if (ref_name) + if (ref_name) { fetchhead_ref->ref_name = git__strdup(ref_name); + GIT_ERROR_CHECK_ALLOC(fetchhead_ref->ref_name); + } - if (remote_url) - fetchhead_ref->remote_url = git__strdup(remote_url); + if (remote_url) { + fetchhead_ref->remote_url = sanitized_remote_url(remote_url); + GIT_ERROR_CHECK_ALLOC(fetchhead_ref->remote_url); + } *out = fetchhead_ref; diff --git a/tests/fetchhead/nonetwork.c b/tests/fetchhead/nonetwork.c index c23622392..6cea6d166 100644 --- a/tests/fetchhead/nonetwork.c +++ b/tests/fetchhead/nonetwork.c @@ -108,7 +108,7 @@ void test_fetchhead_nonetwork__write(void) typedef struct { git_vector *fetchhead_vector; size_t idx; -} fetchhead_ref_cb_data; +} fetchhead_ref_cb_data; static int fetchhead_ref_cb(const char *name, const char *url, const git_oid *oid, unsigned int is_merge, void *payload) @@ -493,3 +493,21 @@ void test_fetchhead_nonetwork__create_with_multiple_refspecs(void) git_remote_free(remote); git_buf_dispose(&path); } + +void test_fetchhead_nonetwork__credentials_are_stripped(void) +{ + git_fetchhead_ref *ref; + git_oid oid; + + cl_git_pass(git_oid_fromstr(&oid, "49322bb17d3acc9146f98c97d078513228bbf3c0")); + cl_git_pass(git_fetchhead_ref_create(&ref, &oid, 0, + "refs/tags/commit_tree", "http://foo:bar@github.com/libgit2/TestGitRepository")); + cl_assert_equal_s(ref->remote_url, "http://github.com/libgit2/TestGitRepository"); + git_fetchhead_ref_free(ref); + + cl_git_pass(git_oid_fromstr(&oid, "49322bb17d3acc9146f98c97d078513228bbf3c0")); + cl_git_pass(git_fetchhead_ref_create(&ref, &oid, 0, + "refs/tags/commit_tree", "https://foo:bar@github.com/libgit2/TestGitRepository")); + cl_assert_equal_s(ref->remote_url, "https://github.com/libgit2/TestGitRepository"); + git_fetchhead_ref_free(ref); +} diff --git a/tests/online/fetchhead.c b/tests/online/fetchhead.c index 4f7be7e38..32e7419ae 100644 --- a/tests/online/fetchhead.c +++ b/tests/online/fetchhead.c @@ -154,3 +154,20 @@ void test_online_fetchhead__colon_only_dst_refspec_creates_no_branch(void) cl_assert_equal_i(refs, count_references()); } + +void test_online_fetchhead__creds_get_stripped(void) +{ + git_buf buf = GIT_BUF_INIT; + git_remote *remote; + + cl_git_pass(git_repository_init(&g_repo, "./foo", 0)); + cl_git_pass(git_remote_create_anonymous(&remote, g_repo, "https://foo:bar@github.com/libgit2/TestGitRepository")); + cl_git_pass(git_remote_fetch(remote, NULL, NULL, NULL)); + + cl_git_pass(git_futils_readbuffer(&buf, "./foo/.git/FETCH_HEAD")); + cl_assert_equal_s(buf.ptr, + "49322bb17d3acc9146f98c97d078513228bbf3c0\t\thttps://github.com/libgit2/TestGitRepository\n"); + + git_remote_free(remote); + git_buf_dispose(&buf); +} -- cgit v1.2.1