Skip to content

Commit 13320ff

Browse files
vdyegitster
authored andcommitted
submodule-config.h: move check_submodule_url
Move 'check_submodule_url' out of 'fsck.c' and into 'submodule-config.h' as a public method, similar to 'check_submodule_name'. With the function now accessible outside of 'fsck', it can be used in a later commit to extend 'test-tool submodule' to check the validity of submodule URLs as it does with names in the 'check-name' subcommand. Other than its location, no changes are made to 'check_submodule_url' in this patch. Signed-off-by: Victoria Dye <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 564d025 commit 13320ff

File tree

3 files changed

+137
-133
lines changed

3 files changed

+137
-133
lines changed

fsck.c

Lines changed: 0 additions & 133 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
#include "packfile.h"
2222
#include "submodule-config.h"
2323
#include "config.h"
24-
#include "credential.h"
2524
#include "help.h"
2625

2726
static ssize_t max_tree_entry_len = 4096;
@@ -1048,138 +1047,6 @@ int fsck_tag_standalone(const struct object_id *oid, const char *buffer,
10481047
return ret;
10491048
}
10501049

1051-
static int starts_with_dot_slash(const char *const path)
1052-
{
1053-
return path_match_flags(path, PATH_MATCH_STARTS_WITH_DOT_SLASH |
1054-
PATH_MATCH_XPLATFORM);
1055-
}
1056-
1057-
static int starts_with_dot_dot_slash(const char *const path)
1058-
{
1059-
return path_match_flags(path, PATH_MATCH_STARTS_WITH_DOT_DOT_SLASH |
1060-
PATH_MATCH_XPLATFORM);
1061-
}
1062-
1063-
static int submodule_url_is_relative(const char *url)
1064-
{
1065-
return starts_with_dot_slash(url) || starts_with_dot_dot_slash(url);
1066-
}
1067-
1068-
/*
1069-
* Count directory components that a relative submodule URL should chop
1070-
* from the remote_url it is to be resolved against.
1071-
*
1072-
* In other words, this counts "../" components at the start of a
1073-
* submodule URL.
1074-
*
1075-
* Returns the number of directory components to chop and writes a
1076-
* pointer to the next character of url after all leading "./" and
1077-
* "../" components to out.
1078-
*/
1079-
static int count_leading_dotdots(const char *url, const char **out)
1080-
{
1081-
int result = 0;
1082-
while (1) {
1083-
if (starts_with_dot_dot_slash(url)) {
1084-
result++;
1085-
url += strlen("../");
1086-
continue;
1087-
}
1088-
if (starts_with_dot_slash(url)) {
1089-
url += strlen("./");
1090-
continue;
1091-
}
1092-
*out = url;
1093-
return result;
1094-
}
1095-
}
1096-
/*
1097-
* Check whether a transport is implemented by git-remote-curl.
1098-
*
1099-
* If it is, returns 1 and writes the URL that would be passed to
1100-
* git-remote-curl to the "out" parameter.
1101-
*
1102-
* Otherwise, returns 0 and leaves "out" untouched.
1103-
*
1104-
* Examples:
1105-
* http::https://example.com/repo.git -> 1, https://example.com/repo.git
1106-
* https://example.com/repo.git -> 1, https://example.com/repo.git
1107-
* git://example.com/repo.git -> 0
1108-
*
1109-
* This is for use in checking for previously exploitable bugs that
1110-
* required a submodule URL to be passed to git-remote-curl.
1111-
*/
1112-
static int url_to_curl_url(const char *url, const char **out)
1113-
{
1114-
/*
1115-
* We don't need to check for case-aliases, "http.exe", and so
1116-
* on because in the default configuration, is_transport_allowed
1117-
* prevents URLs with those schemes from being cloned
1118-
* automatically.
1119-
*/
1120-
if (skip_prefix(url, "http::", out) ||
1121-
skip_prefix(url, "https::", out) ||
1122-
skip_prefix(url, "ftp::", out) ||
1123-
skip_prefix(url, "ftps::", out))
1124-
return 1;
1125-
if (starts_with(url, "http://") ||
1126-
starts_with(url, "https://") ||
1127-
starts_with(url, "ftp://") ||
1128-
starts_with(url, "ftps://")) {
1129-
*out = url;
1130-
return 1;
1131-
}
1132-
return 0;
1133-
}
1134-
1135-
static int check_submodule_url(const char *url)
1136-
{
1137-
const char *curl_url;
1138-
1139-
if (looks_like_command_line_option(url))
1140-
return -1;
1141-
1142-
if (submodule_url_is_relative(url) || starts_with(url, "git://")) {
1143-
char *decoded;
1144-
const char *next;
1145-
int has_nl;
1146-
1147-
/*
1148-
* This could be appended to an http URL and url-decoded;
1149-
* check for malicious characters.
1150-
*/
1151-
decoded = url_decode(url);
1152-
has_nl = !!strchr(decoded, '\n');
1153-
1154-
free(decoded);
1155-
if (has_nl)
1156-
return -1;
1157-
1158-
/*
1159-
* URLs which escape their root via "../" can overwrite
1160-
* the host field and previous components, resolving to
1161-
* URLs like https::example.com/submodule.git and
1162-
* https:///example.com/submodule.git that were
1163-
* susceptible to CVE-2020-11008.
1164-
*/
1165-
if (count_leading_dotdots(url, &next) > 0 &&
1166-
(*next == ':' || *next == '/'))
1167-
return -1;
1168-
}
1169-
1170-
else if (url_to_curl_url(url, &curl_url)) {
1171-
struct credential c = CREDENTIAL_INIT;
1172-
int ret = 0;
1173-
if (credential_from_url_gently(&c, curl_url, 1) ||
1174-
!*c.host)
1175-
ret = -1;
1176-
credential_clear(&c);
1177-
return ret;
1178-
}
1179-
1180-
return 0;
1181-
}
1182-
11831050
struct fsck_gitmodules_data {
11841051
const struct object_id *oid;
11851052
struct fsck_options *options;

submodule-config.c

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
#include "parse-options.h"
1515
#include "thread-utils.h"
1616
#include "tree-walk.h"
17+
#include "url.h"
18+
#include "credential.h"
1719

1820
/*
1921
* submodule cache lookup structure
@@ -228,6 +230,138 @@ int check_submodule_name(const char *name)
228230
return 0;
229231
}
230232

233+
static int starts_with_dot_slash(const char *const path)
234+
{
235+
return path_match_flags(path, PATH_MATCH_STARTS_WITH_DOT_SLASH |
236+
PATH_MATCH_XPLATFORM);
237+
}
238+
239+
static int starts_with_dot_dot_slash(const char *const path)
240+
{
241+
return path_match_flags(path, PATH_MATCH_STARTS_WITH_DOT_DOT_SLASH |
242+
PATH_MATCH_XPLATFORM);
243+
}
244+
245+
static int submodule_url_is_relative(const char *url)
246+
{
247+
return starts_with_dot_slash(url) || starts_with_dot_dot_slash(url);
248+
}
249+
250+
/*
251+
* Count directory components that a relative submodule URL should chop
252+
* from the remote_url it is to be resolved against.
253+
*
254+
* In other words, this counts "../" components at the start of a
255+
* submodule URL.
256+
*
257+
* Returns the number of directory components to chop and writes a
258+
* pointer to the next character of url after all leading "./" and
259+
* "../" components to out.
260+
*/
261+
static int count_leading_dotdots(const char *url, const char **out)
262+
{
263+
int result = 0;
264+
while (1) {
265+
if (starts_with_dot_dot_slash(url)) {
266+
result++;
267+
url += strlen("../");
268+
continue;
269+
}
270+
if (starts_with_dot_slash(url)) {
271+
url += strlen("./");
272+
continue;
273+
}
274+
*out = url;
275+
return result;
276+
}
277+
}
278+
/*
279+
* Check whether a transport is implemented by git-remote-curl.
280+
*
281+
* If it is, returns 1 and writes the URL that would be passed to
282+
* git-remote-curl to the "out" parameter.
283+
*
284+
* Otherwise, returns 0 and leaves "out" untouched.
285+
*
286+
* Examples:
287+
* http::https://example.com/repo.git -> 1, https://example.com/repo.git
288+
* https://example.com/repo.git -> 1, https://example.com/repo.git
289+
* git://example.com/repo.git -> 0
290+
*
291+
* This is for use in checking for previously exploitable bugs that
292+
* required a submodule URL to be passed to git-remote-curl.
293+
*/
294+
static int url_to_curl_url(const char *url, const char **out)
295+
{
296+
/*
297+
* We don't need to check for case-aliases, "http.exe", and so
298+
* on because in the default configuration, is_transport_allowed
299+
* prevents URLs with those schemes from being cloned
300+
* automatically.
301+
*/
302+
if (skip_prefix(url, "http::", out) ||
303+
skip_prefix(url, "https::", out) ||
304+
skip_prefix(url, "ftp::", out) ||
305+
skip_prefix(url, "ftps::", out))
306+
return 1;
307+
if (starts_with(url, "http://") ||
308+
starts_with(url, "https://") ||
309+
starts_with(url, "ftp://") ||
310+
starts_with(url, "ftps://")) {
311+
*out = url;
312+
return 1;
313+
}
314+
return 0;
315+
}
316+
317+
int check_submodule_url(const char *url)
318+
{
319+
const char *curl_url;
320+
321+
if (looks_like_command_line_option(url))
322+
return -1;
323+
324+
if (submodule_url_is_relative(url) || starts_with(url, "git://")) {
325+
char *decoded;
326+
const char *next;
327+
int has_nl;
328+
329+
/*
330+
* This could be appended to an http URL and url-decoded;
331+
* check for malicious characters.
332+
*/
333+
decoded = url_decode(url);
334+
has_nl = !!strchr(decoded, '\n');
335+
336+
free(decoded);
337+
if (has_nl)
338+
return -1;
339+
340+
/*
341+
* URLs which escape their root via "../" can overwrite
342+
* the host field and previous components, resolving to
343+
* URLs like https::example.com/submodule.git and
344+
* https:///example.com/submodule.git that were
345+
* susceptible to CVE-2020-11008.
346+
*/
347+
if (count_leading_dotdots(url, &next) > 0 &&
348+
(*next == ':' || *next == '/'))
349+
return -1;
350+
}
351+
352+
else if (url_to_curl_url(url, &curl_url)) {
353+
struct credential c = CREDENTIAL_INIT;
354+
int ret = 0;
355+
if (credential_from_url_gently(&c, curl_url, 1) ||
356+
!*c.host)
357+
ret = -1;
358+
credential_clear(&c);
359+
return ret;
360+
}
361+
362+
return 0;
363+
}
364+
231365
static int name_and_item_from_var(const char *var, struct strbuf *name,
232366
struct strbuf *item)
233367
{

submodule-config.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,9 @@ int config_set_in_gitmodules_file_gently(const char *key, const char *value);
9191
*/
9292
int check_submodule_name(const char *name);
9393

94+
/* Returns 0 if the URL valid per RFC3986 and -1 otherwise. */
95+
int check_submodule_url(const char *url);
96+
9497
/*
9598
* Note: these helper functions exist solely to maintain backward
9699
* compatibility with 'fetch' and 'update_clone' storing configuration in

0 commit comments

Comments
 (0)