Skip to content

Commit

Permalink
fetchhead: strip credentials from remote URL
Browse files Browse the repository at this point in the history
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:[email protected]/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.
  • Loading branch information
pks-t committed Jan 31, 2020
1 parent a1bff63 commit 93a9044
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 4 deletions.
38 changes: 35 additions & 3 deletions src/fetchhead.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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,
Expand All @@ -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;

Expand Down
20 changes: 19 additions & 1 deletion tests/fetchhead/nonetwork.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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:[email protected]/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:[email protected]/libgit2/TestGitRepository"));
cl_assert_equal_s(ref->remote_url, "https://github.com/libgit2/TestGitRepository");
git_fetchhead_ref_free(ref);
}
17 changes: 17 additions & 0 deletions tests/online/fetchhead.c
Original file line number Diff line number Diff line change
Expand Up @@ -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:[email protected]/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);
}

0 comments on commit 93a9044

Please sign in to comment.