{"thread":{"id":"41703","subject":"[PATCH 0/1] http: Fix crash when passing malformed URL","startedAt":"2016-03-16T10:54:06Z","lastAt":"2016-03-16T21:42:00Z","messageCount":4,"participants":["Anton Wuerfel","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"280883","messageId":"1458125647-32707-1-git-send-email-anton.wuerfel@fau.de","threadId":"41703","inReplyTo":null,"subject":"[PATCH 0/1] http: Fix crash when passing malformed URL","fromName":"Anton Wuerfel","fromEmail":"anton.wuerfel@fau.de","sentAt":"2016-03-16T10:54:06Z","receivedAt":"2016-03-16T10:54:06Z","isPatch":true,"sender":{"key":"anton.wuerfel@fau.de","avatar":null},"body":"When passing a malformed URL to http_init() in http.c, git dies from a null\npointer dereference. An example for a malformed URL is http:/git-scm.com (note\nthe single slash after the protocol).\n\nI could not reproduce this error within any functions of git - I just noticed it\nduring development of a git extension together with Phillip Raffeck.\n\nAnton Wuerfel (1):\n  http: Fix crash when passing malformed URL\n\n http.c | 3 +++\n 1 file changed, 3 insertions(+)\n\n-- \n2.8.0.rc1.108.g7827469\n"},{"id":"280884","messageId":"1458125647-32707-2-git-send-email-anton.wuerfel@fau.de","threadId":"41703","inReplyTo":"1458125647-32707-1-git-send-email-anton.wuerfel@fau.de","subject":"[PATCH 1/1] http: Fix crash when passing malformed URL","fromName":"Anton Wuerfel","fromEmail":"anton.wuerfel@fau.de","sentAt":"2016-03-16T10:54:07Z","receivedAt":"2016-03-16T10:54:07Z","isPatch":true,"sender":{"key":"anton.wuerfel@fau.de","avatar":null},"body":"When passing a malformed URL to http_init() in http.c, git dies from a null\npointer dereference. An example for a malformed URL is http:/git-scm.com (note\nthe single slash after the protocol).\nThis patch adds simple error handling as git notices the malformed URL already,\nbut never checks the error value.\n\nWhen passing a malformed URL, credential_from_url(struct credential *c, const char *url)\ninitializes *c with null values. When the existence of `://` in url is checked,\nthe function returns without further change of *c.\nThe null pointer dereference occurs in get_curl_handle () at http.c:593, when\nthe `protocol` field of struct credential is strcmp'ed:\n\nProgram received signal SIGSEGV, Segmentation fault.\n0x0000000000405efd in get_curl_handle () at http.c:593\n593                     if (!strcmp(http_auth.protocol, \"https\")) {\n---\n http.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/http.c b/http.c\nindex 69da445..80cf752 100644\n--- a/http.c\n+++ b/http.c\n@@ -660,6 +660,9 @@ void http_init(struct remote *remote, const char *url, int proactive_auth)\n \n \thttp_is_verbose = 0;\n \tnormalized_url = url_normalize(url, &config.url);\n+\t\n+\tif (config.url.err)\n+\t\tdie(_(\"libcurl: %s, URL: %s\"), config.url.err, url);\n \n \tgit_config(urlmatch_config_entry, &config);\n \tfree(normalized_url);\n-- \n2.8.0.rc1.108.g7827469\n"},{"id":"280970","messageId":"20160316212938.GA4441@sigill.intra.peff.net","threadId":"41703","inReplyTo":"1458125647-32707-1-git-send-email-anton.wuerfel@fau.de","subject":"Re: [PATCH 0/1] http: Fix crash when passing malformed URL","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-16T21:29:38Z","receivedAt":"2016-03-16T21:29:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 16, 2016 at 11:54:06AM +0100, Anton Wuerfel wrote:\n\n> When passing a malformed URL to http_init() in http.c, git dies from a null\n> pointer dereference. An example for a malformed URL is http:/git-scm.com (note\n> the single slash after the protocol).\n> \n> I could not reproduce this error within any functions of git - I just noticed it\n> during development of a git extension together with Phillip Raffeck.\n\nYou can reproduce it with:\n\n  git clone https::bogus\n\nNormally we recognize \"https://\" as the signal to send the URL to the\ngit-remote-https helper, but you can override the helper by specifying\n\"whatever::\", and then the rest of the string will be passed to it.\n\n-Peff\n"},{"id":"280975","messageId":"20160316214200.GB4441@sigill.intra.peff.net","threadId":"41703","inReplyTo":"1458125647-32707-2-git-send-email-anton.wuerfel@fau.de","subject":"Re: [PATCH 1/1] http: Fix crash when passing malformed URL","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-16T21:42:00Z","receivedAt":"2016-03-16T21:42:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 16, 2016 at 11:54:07AM +0100, Anton Wuerfel wrote:\n\n> When passing a malformed URL to http_init() in http.c, git dies from a null\n> pointer dereference. An example for a malformed URL is http:/git-scm.com (note\n> the single slash after the protocol).\n> This patch adds simple error handling as git notices the malformed URL already,\n> but never checks the error value.\n> \n> When passing a malformed URL, credential_from_url(struct credential *c, const char *url)\n> initializes *c with null values. When the existence of `://` in url is checked,\n> the function returns without further change of *c.\n> The null pointer dereference occurs in get_curl_handle () at http.c:593, when\n> the `protocol` field of struct credential is strcmp'ed:\n\nSo I think the most direct bug here is that line 593 assumes that\nhttp_auth.protocol is not NULL, when it might very well be (if we could\nnot parse the protocol). Your solution is to detect early that we don't\nhave a URL that curl can parse, and bail.\n\nI think that's probably a reasonable thing to do in general. But it\ndoesn't make me certain that there's a case that curl might parse that\nour credential url-parser might not. And the code in question does not\neven care about credentials at all! It's just piggy-backing on the\nearlier parse done by the credential code.\n\nI think it would make much more sense for it to rely on the normalized\nurl we produce. IOW, to do something like:\n\n  if (starts_with(normalized_url, \"https://\"))\n\t/* https stuff */\n  else if (starts_with(normalized_url, \"http://\"))\n\t/* http stuff */\n  else\n\t/* other stuff */\n\nNote that the current code doesn't actually check for \"http\" (versus\nother protocols; despite the name http_init(), this code gets run for\nthe probably-never-used-these-days git-over-ftp protocol). I suspect we\nare respecting http_proxy for ftp connections, which is silly.\n\nNote that normalized_url is freed before this point, so we may have to\nhold onto it longer. Or it may be possible to use the broken-down\nrepresentation from config.url; I didn't look.\n\n-Peff\n"}]}