@matheusbsbs / matheus-bagliano / commits / c44088ecc4

Fork 0

credential: treat URL without scheme as invalid

libcurl permits making requests without a URL scheme specified. In this case, it guesses the URL from the hostname, so I can run git ls-remote http::ftp.example.com/path/to/repo and it would make an FTP request. Any user intentionally using such a URL is likely to have made a typo. Unfortunately, credential_from_url is not able to determine the host and protocol in order to determine appropriate credentials to send, and until "credential: refuse to operate when missing host or protocol", this resulted in another host's credentials being leaked to the named host. Teach credential_from_url_gently to consider such a URL to be invalid so that fsck can detect and block gitmodules files with such URLs, allowing server operators to avoid serving them to downstream users running older versions of Git. This also means that when such URLs are passed on the command line, Git will print a clearer error so affected users can switch to the simpler URL that explicitly specifies the host and protocol they intend. One subtlety: .gitmodules files can contain relative URLs, representing a URL relative to the URL they were cloned from. The relative URL resolver used for .gitmodules can follow ".." components out of the path part and past the host part of a URL, meaning that such a relative URL can be used to traverse from a https://foo.example.com/innocent superproject to a https::attacker.example.com/exploit submodule. Fortunately a leading ':' in the first path component after a series of leading './' and '../' components is unlikely to show up in other contexts, so we can catch this by detecting that pattern. Reported-by: Jeff King <peff@peff.net> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Reviewed-by: Jeff King <peff@peff.net>

Jonathan Nieder committed Apr 18, 2020 at 20:54 UTC c44088ecc4b0722636e0a305f9608d3047197282

4 files changed +84 -9

credential.c

+5 -2

@@ -357,8 +357,11 @@ int credential_from_url_gently(struct credential *c, const char *url,

357 * (3) proto://<user>:<pass>@<host>/...

358 */

359 proto_end = strstr(url, "://");

360 - if (!proto_end)

361 - return 0;

360 + if (!proto_end) {

361 + if (!quiet)

362 + warning(_("url has no scheme: %s"), url);

363 + return -1;

364 + }

365 cp = proto_end + 3;

366 at = strchr(cp, '@');

367 colon = strchr(cp, ':');

fsck.c

+45 -2

@@ -971,6 +971,34 @@ static int submodule_url_is_relative(const char *url)

971 return starts_with_dot_slash(url) || starts_with_dot_dot_slash(url);

972 }

973

974 +/*

975 + * Count directory components that a relative submodule URL should chop

976 + * from the remote_url it is to be resolved against.

977 + *

978 + * In other words, this counts "../" components at the start of a

979 + * submodule URL.

980 + *

981 + * Returns the number of directory components to chop and writes a

982 + * pointer to the next character of url after all leading "./" and

983 + * "../" components to out.

984 + */

985 +static int count_leading_dotdots(const char *url, const char **out)

986 +{

987 + int result = 0;

988 + while (1) {

989 + if (starts_with_dot_dot_slash(url)) {

990 + result++;

991 + url += strlen("../");

992 + continue;

993 + }

994 + if (starts_with_dot_slash(url)) {

995 + url += strlen("./");

996 + continue;

997 + }

998 + *out = url;

999 + return result;

1000 + }

1001 +}

1002 /*

1003 * Check whether a transport is implemented by git-remote-curl.

1004 *

@@ -1018,15 +1046,30 @@ static int check_submodule_url(const char *url)

1046 return -1;

1047

1048 if (submodule_url_is_relative(url)) {

1049 + char *decoded;

1050 + const char *next;

1051 + int has_nl;

1052 +

1053 /*

1054 * This could be appended to an http URL and url-decoded;

1055 * check for malicious characters.

1056 */

1025 - char *decoded = url_decode(url);

1026 - int has_nl = !!strchr(decoded, '\n');

1057 + decoded = url_decode(url);

1058 + has_nl = !!strchr(decoded, '\n');

1059 +

1060 free(decoded);

1061 if (has_nl)

1062 return -1;

1063 +

1064 + /*

1065 + * URLs which escape their root via "../" can overwrite

1066 + * the host field and previous components, resolving to

1067 + * URLs like https::example.com/submodule.git that were

1068 + * susceptible to CVE-2020-11008.

1069 + */

1070 + if (count_leading_dotdots(url, &next) > 0 &&

1071 + *next == ':')

1072 + return -1;

1073 }

1074

1075 else if (url_to_curl_url(url, &curl_url)) {

t/t5550-http-fetch-dumb.sh

+2 -5

@@ -310,11 +310,8 @@ test_expect_success 'git client does not send an empty Accept-Language' '

310 '

311

312 test_expect_success 'remote-http complains cleanly about malformed urls' '

313 - # do not actually issue "list" or other commands, as we do not

314 - # want to rely on what curl would actually do with such a broken

315 - # URL. This is just about making sure we do not segfault during

316 - # initialization.

317 - test_must_fail git remote-http http::/example.com/repo.git

313 + test_must_fail git remote-http http::/example.com/repo.git 2>stderr &&

314 + test_i18ngrep "url has no scheme" stderr

315 '

316

317 test_expect_success 'redirects can be forbidden/allowed' '

t/t7416-submodule-dash-url.sh

+32

@@ -60,6 +60,38 @@ test_expect_success 'trailing backslash is handled correctly' '

60 test_i18ngrep ! "unknown option" err

61 '

63 +test_expect_success 'fsck rejects missing URL scheme' '

64 + git checkout --orphan missing-scheme &&

65 + cat >.gitmodules <<-\EOF &&

66 + [submodule "foo"]

67 + url = http::one.example.com/foo.git

68 + EOF

69 + git add .gitmodules &&

70 + test_tick &&

71 + git commit -m "gitmodules with missing URL scheme" &&

72 + test_when_finished "rm -rf dst" &&

73 + git init --bare dst &&

74 + git -C dst config transfer.fsckObjects true &&

75 + test_must_fail git push dst HEAD 2>err &&

76 + grep gitmodulesUrl err

77 +'

78 +

79 +test_expect_success 'fsck rejects relative URL resolving to missing scheme' '

80 + git checkout --orphan relative-missing-scheme &&

81 + cat >.gitmodules <<-\EOF &&

82 + [submodule "foo"]

83 + url = "..\\../.\\../:one.example.com/foo.git"

84 + EOF

85 + git add .gitmodules &&

86 + test_tick &&

87 + git commit -m "gitmodules with relative URL that strips off scheme" &&

88 + test_when_finished "rm -rf dst" &&

89 + git init --bare dst &&

90 + git -C dst config transfer.fsckObjects true &&

91 + test_must_fail git push dst HEAD 2>err &&

92 + grep gitmodulesUrl err

93 +'

94 +

95 test_expect_success 'fsck permits embedded newline with unrecognized scheme' '

96 git checkout --orphan newscheme &&

97 cat >.gitmodules <<-\EOF &&