@matheusbsbs / matheus-bagliano / commits / a2b26ffb1a
Fork 0

fsck: convert gitmodules url to URL passed to curl

In 07259e74ec1 (fsck: detect gitmodules URLs with embedded newlines, 2020-03-11), git fsck learned to check whether URLs in .gitmodules could be understood by the credential machinery when they are handled by git-remote-curl. However, the check is overbroad: it checks all URLs instead of only URLs that would be passed to git-remote-curl. In principle a git:// or file:/// URL does not need to follow the same conventions as an http:// URL; in particular, git:// and file:// protocols are not succeptible to issues in the credential API because they do not support attaching credentials. In the HTTP case, the URL in .gitmodules does not always match the URL that would be passed to git-remote-curl and the credential machinery: Git's URL syntax allows specifying a remote helper followed by a "::" delimiter and a URL to be passed to it, so that git ls-remote http::https://example.com/repo.git invokes git-remote-http with https://example.com/repo.git as its URL argument. With today's checks, that distinction does not make a difference, but for a check we are about to introduce (for empty URL schemes) it will matter. .gitmodules files also support relative URLs. To ensure coverage for the https based embedded-newline attack, urldecode and check them directly for embedded newlines. Helped-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:52 UTC a2b26ffb1a81aa23dd14453f4db05d8fe24ee7cc
2 files changed +118 -5
fsck.c
+89 -5
@@ -7,6 +7,7 @@
7 #include "tag.h"
8 #include "fsck.h"
9 #include "refs.h"
10 +#include "url.h"
11 #include "utf8.h"
12 #include "sha1-array.h"
13 #include "decorate.h"
@@ -942,17 +943,100 @@ static int fsck_tag(struct tag *tag, const char *data,
943 return fsck_tag_buffer(tag, data, size, options);
944 }
945
946 +/*
947 + * Like builtin/submodule--helper.c's starts_with_dot_slash, but without
948 + * relying on the platform-dependent is_dir_sep helper.
949 + *
950 + * This is for use in checking whether a submodule URL is interpreted as
951 + * relative to the current directory on any platform, since \ is a
952 + * directory separator on Windows but not on other platforms.
953 + */
954 +static int starts_with_dot_slash(const char *str)
955 +{
956 + return str[0] == '.' && (str[1] == '/' || str[1] == '\\');
957 +}
958 +
959 +/*
960 + * Like starts_with_dot_slash, this is a variant of submodule--helper's
961 + * helper of the same name with the twist that it accepts backslash as a
962 + * directory separator even on non-Windows platforms.
963 + */
964 +static int starts_with_dot_dot_slash(const char *str)
965 +{
966 + return str[0] == '.' && starts_with_dot_slash(str + 1);
967 +}
968 +
969 +static int submodule_url_is_relative(const char *url)
970 +{
971 + return starts_with_dot_slash(url) || starts_with_dot_dot_slash(url);
972 +}
973 +
974 +/*
975 + * Check whether a transport is implemented by git-remote-curl.
976 + *
977 + * If it is, returns 1 and writes the URL that would be passed to
978 + * git-remote-curl to the "out" parameter.
979 + *
980 + * Otherwise, returns 0 and leaves "out" untouched.
981 + *
982 + * Examples:
983 + * http::https://example.com/repo.git -> 1, https://example.com/repo.git
984 + * https://example.com/repo.git -> 1, https://example.com/repo.git
985 + * git://example.com/repo.git -> 0
986 + *
987 + * This is for use in checking for previously exploitable bugs that
988 + * required a submodule URL to be passed to git-remote-curl.
989 + */
990 +static int url_to_curl_url(const char *url, const char **out)
991 +{
992 + /*
993 + * We don't need to check for case-aliases, "http.exe", and so
994 + * on because in the default configuration, is_transport_allowed
995 + * prevents URLs with those schemes from being cloned
996 + * automatically.
997 + */
998 + if (skip_prefix(url, "http::", out) ||
999 + skip_prefix(url, "https::", out) ||
1000 + skip_prefix(url, "ftp::", out) ||
1001 + skip_prefix(url, "ftps::", out))
1002 + return 1;
1003 + if (starts_with(url, "http://") ||
1004 + starts_with(url, "https://") ||
1005 + starts_with(url, "ftp://") ||
1006 + starts_with(url, "ftps://")) {
1007 + *out = url;
1008 + return 1;
1009 + }
1010 + return 0;
1011 +}
1012 +
1013 static int check_submodule_url(const char *url)
1014 {
947 - struct credential c = CREDENTIAL_INIT;
948 - int ret;
1015 + const char *curl_url;
1016
1017 if (looks_like_command_line_option(url))
1018 return -1;
1019
953 - ret = credential_from_url_gently(&c, url, 1);
954 - credential_clear(&c);
955 - return ret;
1020 + if (submodule_url_is_relative(url)) {
1021 + /*
1022 + * This could be appended to an http URL and url-decoded;
1023 + * check for malicious characters.
1024 + */
1025 + char *decoded = url_decode(url);
1026 + int has_nl = !!strchr(decoded, '\n');
1027 + free(decoded);
1028 + if (has_nl)
1029 + return -1;
1030 + }
1031 +
1032 + else if (url_to_curl_url(url, &curl_url)) {
1033 + struct credential c = CREDENTIAL_INIT;
1034 + int ret = credential_from_url_gently(&c, curl_url, 1);
1035 + credential_clear(&c);
1036 + return ret;
1037 + }
1038 +
1039 + return 0;
1040 }
1041
1042 struct fsck_gitmodules_data {
t/t7416-submodule-dash-url.sh
+29
@@ -60,6 +60,20 @@ test_expect_success 'trailing backslash is handled correctly' '
60 test_i18ngrep ! "unknown option" err
61 '
62
63 +test_expect_success 'fsck permits embedded newline with unrecognized scheme' '
64 + git checkout --orphan newscheme &&
65 + cat >.gitmodules <<-\EOF &&
66 + [submodule "foo"]
67 + url = "data://acjbkd%0akajfdickajkd"
68 + EOF
69 + git add .gitmodules &&
70 + git commit -m "gitmodules with unrecognized scheme" &&
71 + test_when_finished "rm -rf dst" &&
72 + git init --bare dst &&
73 + git -C dst config transfer.fsckObjects true &&
74 + git push dst HEAD
75 +'
76 +
77 test_expect_success 'fsck rejects embedded newline in url' '
78 # create an orphan branch to avoid existing .gitmodules objects
79 git checkout --orphan newline &&
@@ -76,4 +90,19 @@ test_expect_success 'fsck rejects embedded newline in url' '
90 grep gitmodulesUrl err
91 '
92
93 +test_expect_success 'fsck rejects embedded newline in relative url' '
94 + git checkout --orphan relative-newline &&
95 + cat >.gitmodules <<-\EOF &&
96 + [submodule "foo"]
97 + url = "./%0ahost=two.example.com/foo.git"
98 + EOF
99 + git add .gitmodules &&
100 + git commit -m "relative url with newline" &&
101 + test_when_finished "rm -rf dst" &&
102 + git init --bare dst &&
103 + git -C dst config transfer.fsckObjects true &&
104 + test_must_fail git push dst HEAD 2>err &&
105 + grep gitmodulesUrl err
106 +'
107 +
108 test_done