{"thread":{"id":"44031","subject":"segfault in http.c when https URL is mistyped","startedAt":"2016-09-07T13:44:19Z","lastAt":"2016-09-07T20:11:47Z","messageCount":3,"participants":["Lars Wendler","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"301264","messageId":"20160907154404.107fe6e6@abudhabi.paradoxon.rec","threadId":"44031","inReplyTo":"20160907151607.2b288034@abudhabi.paradoxon.rec","subject":"segfault in http.c when https URL is mistyped","fromName":"Lars Wendler","fromEmail":"polynomial-c@gentoo.org","sentAt":"2016-09-07T13:44:04Z","receivedAt":"2016-09-07T13:44:19Z","isPatch":false,"sender":{"key":"polynomial-c@gentoo.org","avatar":null},"body":"Hi,\n\nwe at Gentoo got a bug report [1] about git-remote-https segfaulting\nwhen the URL has been mistyped. \nThis seems to only be triggered when git was compiled with curl\nsupport:\n\n  git clone https::/some.example-site.net/test.git\n\nObserve the \"https::/\" instead of \"https://\". As soon as you run such a\ncommand, syslog receives a message like:\n\n  kernel: git-remote-http[8766]: segfault at 0 ip 0000000000408abd sp\n  00007ffd70adf7c0 error 4 in git-remote-https[400000+103000]\n\nKind regards\nLars\n\n[1] https://bugs.gentoo.org/592522\n\n-- \nLars Wendler\nGentoo package maintainer\nGPG: 21CC CF02 4586 0A07 ED93  9F68 498F E765 960E 9B39\n\nAttention! New gpg key! See\nhttps://www.gentoofan.org/blog/index.php?/archives/9-New-gpg-keys.html\n"},{"id":"301322","messageId":"20160907200642.5o3otxxt7ybw6x4c@sigill.intra.peff.net","threadId":"44031","inReplyTo":"20160907154404.107fe6e6@abudhabi.paradoxon.rec","subject":"Re: segfault in http.c when https URL is mistyped","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-07T20:06:42Z","receivedAt":"2016-09-07T20:08:14Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 07, 2016 at 03:44:04PM +0200, Lars Wendler wrote:\n\n> we at Gentoo got a bug report [1] about git-remote-https segfaulting\n> when the URL has been mistyped. \n> This seems to only be triggered when git was compiled with curl\n> support:\n> \n>   git clone https::/some.example-site.net/test.git\n\nThanks, this was easy to reproduce. It's a regression in v2.8.0. The fix\nis below.\n\n-- >8 --\nSubject: [PATCH] remote-curl: handle URLs without protocol\n\nGenerally remote-curl would never see a URL that did not\nhave \"proto:\" at the beginning, as that is what tells git to\nrun the \"git-remote-proto\" helper (and git-remote-http, etc,\nare aliases for git-remote-curl).\n\nHowever, the special syntax \"proto::something\" will run\ngit-remote-proto with only \"something\" as the URL. So a\nmalformed URL like:\n\n  http::/example.com/repo.git\n\nwill feed the URL \"/example.com/repo.git\" to\ngit-remote-http. The resulting URL has no protocol, but the\ncode added by 372370f (http: use credential API to handle\nproxy authentication, 2016-01-26) does not handle this case\nand segfaults.\n\nFor the purposes of this code, we don't really care what the\nexact protocol; only whether or not it is https. So let's\njust assume that a missing protocol is not, and curl will\nhandle the real error (which is that the URL is nonsense).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI looked around for other similar over-assumptions about the URL parsing\nbut didn't see any.\n\n http.c                     | 2 +-\n t/t5550-http-fetch-dumb.sh | 8 ++++++++\n 2 files changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/http.c b/http.c\nindex cd40b01..edce47c 100644\n--- a/http.c\n+++ b/http.c\n@@ -723,7 +723,7 @@ static CURL *get_curl_handle(void)\n \t * precedence here, as in CURL.\n \t */\n \tif (!curl_http_proxy) {\n-\t\tif (!strcmp(http_auth.protocol, \"https\")) {\n+\t\tif (http_auth.protocol && !strcmp(http_auth.protocol, \"https\")) {\n \t\t\tvar_override(&curl_http_proxy, getenv(\"HTTPS_PROXY\"));\n \t\t\tvar_override(&curl_http_proxy, getenv(\"https_proxy\"));\n \t\t} else {\ndiff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh\nindex 3484b6f..01bb633 100755\n--- a/t/t5550-http-fetch-dumb.sh\n+++ b/t/t5550-http-fetch-dumb.sh\n@@ -299,5 +299,13 @@ test_expect_success 'git client does not send an empty Accept-Language' '\n \t! grep \"^Accept-Language:\" stderr\n '\n \n+test_expect_success 'remote-http complains cleanly about malformed urls' '\n+\t# do not actually issue \"list\" or other commands, as we do not\n+\t# want to rely on what curl would actually do with such a broken\n+\t# URL. This is just about making sure we do not segfault during\n+\t# initialization.\n+\ttest_must_fail git remote-http http::/example.com/repo.git\n+'\n+\n stop_httpd\n test_done\n-- \n2.10.0.rc2.154.gb4a4b8b\n\n"},{"id":"301323","messageId":"20160907201107.sgb7upqpzyap45bd@sigill.intra.peff.net","threadId":"44031","inReplyTo":"20160907200642.5o3otxxt7ybw6x4c@sigill.intra.peff.net","subject":"Re: segfault in http.c when https URL is mistyped","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-07T20:11:07Z","receivedAt":"2016-09-07T20:11:47Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 07, 2016 at 04:06:42PM -0400, Jeff King wrote:\n\n> +test_expect_success 'remote-http complains cleanly about malformed urls' '\n> +\t# do not actually issue \"list\" or other commands, as we do not\n> +\t# want to rely on what curl would actually do with such a broken\n> +\t# URL. This is just about making sure we do not segfault during\n> +\t# initialization.\n> +\ttest_must_fail git remote-http http::/example.com/repo.git\n> +'\n\nActually, I guess remote-http actually sees just \"/example.com/repo.git\"\nfor the case we're discussing. It segfaults with either input without\nthis patch, and works with it, though.\n\n(I didn't test \"git clone\" directly because it hides the segfault made\nby the remote helper, so we cannot tell the difference between a\nsegfault and \"this URL is broken\").\n\n-Peff\n"}]}