Commit 93a9044ff663684da278d4d0fc5111a293e6555b
2020-01-31T08:49:34
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.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125
diff --git a/src/fetchhead.c b/src/fetchhead.c
index a3f6fbc..ea610f3 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 c236223..6cea6d1 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 4f7be7e..32e7419 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);
+}