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
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
|
From 68acf8724e9cb2f67664dd980581c0022401daf0 Mon Sep 17 00:00:00 2001
From: Jonathan Nieder <jrnieder@gmail.com>
Date: Sat, 18 Apr 2020 20:54:13 -0700
Subject: [PATCH 10/12] 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>
Upstream-Status: Backport
CVE: CVE-2020-11008 (7)
Signed-off-by: Li Zhou <li.zhou@windriver.com>
---
credential.c | 7 +++++--
fsck.c | 47 +++++++++++++++++++++++++++++++++++++++++--
t/t5550-http-fetch-dumb.sh | 7 ++-----
t/t7416-submodule-dash-url.sh | 32 +++++++++++++++++++++++++++++
4 files changed, 84 insertions(+), 9 deletions(-)
diff --git a/credential.c b/credential.c
index 22649d5..1e1aed5 100644
--- a/credential.c
+++ b/credential.c
@@ -360,8 +360,11 @@ int credential_from_url_gently(struct credential *c, const char *url,
* (3) proto://<user>:<pass>@<host>/...
*/
proto_end = strstr(url, "://");
- if (!proto_end)
- return 0;
+ if (!proto_end) {
+ if (!quiet)
+ warning(_("url has no scheme: %s"), url);
+ return -1;
+ }
cp = proto_end + 3;
at = strchr(cp, '@');
colon = strchr(cp, ':');
diff --git a/fsck.c b/fsck.c
index 0f21eb1..30eac29 100644
--- a/fsck.c
+++ b/fsck.c
@@ -978,6 +978,34 @@ static int submodule_url_is_relative(const char *url)
}
/*
+ * Count directory components that a relative submodule URL should chop
+ * from the remote_url it is to be resolved against.
+ *
+ * In other words, this counts "../" components at the start of a
+ * submodule URL.
+ *
+ * Returns the number of directory components to chop and writes a
+ * pointer to the next character of url after all leading "./" and
+ * "../" components to out.
+ */
+static int count_leading_dotdots(const char *url, const char **out)
+{
+ int result = 0;
+ while (1) {
+ if (starts_with_dot_dot_slash(url)) {
+ result++;
+ url += strlen("../");
+ continue;
+ }
+ if (starts_with_dot_slash(url)) {
+ url += strlen("./");
+ continue;
+ }
+ *out = url;
+ return result;
+ }
+}
+/*
* Check whether a transport is implemented by git-remote-curl.
*
* If it is, returns 1 and writes the URL that would be passed to
@@ -1024,15 +1052,30 @@ static int check_submodule_url(const char *url)
return -1;
if (submodule_url_is_relative(url)) {
+ char *decoded;
+ const char *next;
+ int has_nl;
+
/*
* This could be appended to an http URL and url-decoded;
* check for malicious characters.
*/
- char *decoded = url_decode(url);
- int has_nl = !!strchr(decoded, '\n');
+ decoded = url_decode(url);
+ has_nl = !!strchr(decoded, '\n');
+
free(decoded);
if (has_nl)
return -1;
+
+ /*
+ * URLs which escape their root via "../" can overwrite
+ * the host field and previous components, resolving to
+ * URLs like https::example.com/submodule.git that were
+ * susceptible to CVE-2020-11008.
+ */
+ if (count_leading_dotdots(url, &next) > 0 &&
+ *next == ':')
+ return -1;
}
else if (url_to_curl_url(url, &curl_url)) {
diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
index b811d89..1c9e5d3 100755
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -321,11 +321,8 @@ test_expect_success 'git client does not send an empty Accept-Language' '
'
test_expect_success 'remote-http complains cleanly about malformed urls' '
- # do not actually issue "list" or other commands, as we do not
- # want to rely on what curl would actually do with such a broken
- # URL. This is just about making sure we do not segfault during
- # initialization.
- test_must_fail git remote-http http::/example.com/repo.git
+ test_must_fail git remote-http http::/example.com/repo.git 2>stderr &&
+ test_i18ngrep "url has no scheme" stderr
'
test_expect_success 'redirects can be forbidden/allowed' '
diff --git a/t/t7416-submodule-dash-url.sh b/t/t7416-submodule-dash-url.sh
index afdd255..249dc3d 100755
--- a/t/t7416-submodule-dash-url.sh
+++ b/t/t7416-submodule-dash-url.sh
@@ -60,6 +60,38 @@ test_expect_success 'trailing backslash is handled correctly' '
test_i18ngrep ! "unknown option" err
'
+test_expect_success 'fsck rejects missing URL scheme' '
+ git checkout --orphan missing-scheme &&
+ cat >.gitmodules <<-\EOF &&
+ [submodule "foo"]
+ url = http::one.example.com/foo.git
+ EOF
+ git add .gitmodules &&
+ test_tick &&
+ git commit -m "gitmodules with missing URL scheme" &&
+ test_when_finished "rm -rf dst" &&
+ git init --bare dst &&
+ git -C dst config transfer.fsckObjects true &&
+ test_must_fail git push dst HEAD 2>err &&
+ grep gitmodulesUrl err
+'
+
+test_expect_success 'fsck rejects relative URL resolving to missing scheme' '
+ git checkout --orphan relative-missing-scheme &&
+ cat >.gitmodules <<-\EOF &&
+ [submodule "foo"]
+ url = "..\\../.\\../:one.example.com/foo.git"
+ EOF
+ git add .gitmodules &&
+ test_tick &&
+ git commit -m "gitmodules with relative URL that strips off scheme" &&
+ test_when_finished "rm -rf dst" &&
+ git init --bare dst &&
+ git -C dst config transfer.fsckObjects true &&
+ test_must_fail git push dst HEAD 2>err &&
+ grep gitmodulesUrl err
+'
+
test_expect_success 'fsck permits embedded newline with unrecognized scheme' '
git checkout --orphan newscheme &&
cat >.gitmodules <<-\EOF &&
--
1.9.1
|
