# segfault in http.c when https URL is mistyped 3 messages from 2016-09-07 to 2016-09-07. Participants: Lars Wendler, Jeff King. Thread: https://gitlist.dev/t/44031 ## Lars Wendler, 2016-09-07 13:44 Subject: segfault in http.c when https URL is mistyped Message-ID: <20160907154404.107fe6e6@abudhabi.paradoxon.rec> URL: https://gitlist.dev/e/20160907154404.107fe6e6%40abudhabi.paradoxon.rec In-Reply-To: <20160907151607.2b288034@abudhabi.paradoxon.rec> ``` Hi, we at Gentoo got a bug report [1] about git-remote-https segfaulting when the URL has been mistyped. This seems to only be triggered when git was compiled with curl support: git clone https::/some.example-site.net/test.git Observe the "https::/" instead of "https://". As soon as you run such a command, syslog receives a message like: kernel: git-remote-http[8766]: segfault at 0 ip 0000000000408abd sp 00007ffd70adf7c0 error 4 in git-remote-https[400000+103000] Kind regards Lars [1] https://bugs.gentoo.org/592522 -- Lars Wendler Gentoo package maintainer GPG: 21CC CF02 4586 0A07 ED93 9F68 498F E765 960E 9B39 Attention! New gpg key! See https://www.gentoofan.org/blog/index.php?/archives/9-New-gpg-keys.html ``` ## Jeff King, 2016-09-07 20:06 Subject: Re: segfault in http.c when https URL is mistyped Message-ID: <20160907200642.5o3otxxt7ybw6x4c@sigill.intra.peff.net> URL: https://gitlist.dev/e/20160907200642.5o3otxxt7ybw6x4c%40sigill.intra.peff.net In-Reply-To: <20160907154404.107fe6e6@abudhabi.paradoxon.rec> ``` On Wed, Sep 07, 2016 at 03:44:04PM +0200, Lars Wendler wrote: > we at Gentoo got a bug report [1] about git-remote-https segfaulting > when the URL has been mistyped. > This seems to only be triggered when git was compiled with curl > support: > > git clone https::/some.example-site.net/test.git Thanks, this was easy to reproduce. It's a regression in v2.8.0. The fix is below. -- >8 -- Subject: [PATCH] remote-curl: handle URLs without protocol Generally remote-curl would never see a URL that did not have "proto:" at the beginning, as that is what tells git to run the "git-remote-proto" helper (and git-remote-http, etc, are aliases for git-remote-curl). However, the special syntax "proto::something" will run git-remote-proto with only "something" as the URL. So a malformed URL like: http::/example.com/repo.git will feed the URL "/example.com/repo.git" to git-remote-http. The resulting URL has no protocol, but the code added by 372370f (http: use credential API to handle proxy authentication, 2016-01-26) does not handle this case and segfaults. For the purposes of this code, we don't really care what the exact protocol; only whether or not it is https. So let's just assume that a missing protocol is not, and curl will handle the real error (which is that the URL is nonsense). Signed-off-by: Jeff King --- I looked around for other similar over-assumptions about the URL parsing but didn't see any. http.c | 2 +- t/t5550-http-fetch-dumb.sh | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/http.c b/http.c index cd40b01..edce47c 100644 --- a/http.c +++ b/http.c @@ -723,7 +723,7 @@ static CURL *get_curl_handle(void) * precedence here, as in CURL. */ if (!curl_http_proxy) { - if (!strcmp(http_auth.protocol, "https")) { + if (http_auth.protocol && !strcmp(http_auth.protocol, "https")) { var_override(&curl_http_proxy, getenv("HTTPS_PROXY")); var_override(&curl_http_proxy, getenv("https_proxy")); } else { diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh index 3484b6f..01bb633 100755 --- a/t/t5550-http-fetch-dumb.sh +++ b/t/t5550-http-fetch-dumb.sh @@ -299,5 +299,13 @@ test_expect_success 'git client does not send an empty Accept-Language' ' ! grep "^Accept-Language:" stderr ' +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 +' + stop_httpd test_done -- 2.10.0.rc2.154.gb4a4b8b ``` ## Jeff King, 2016-09-07 20:11 Subject: Re: segfault in http.c when https URL is mistyped Message-ID: <20160907201107.sgb7upqpzyap45bd@sigill.intra.peff.net> URL: https://gitlist.dev/e/20160907201107.sgb7upqpzyap45bd%40sigill.intra.peff.net In-Reply-To: <20160907200642.5o3otxxt7ybw6x4c@sigill.intra.peff.net> ``` On Wed, Sep 07, 2016 at 04:06:42PM -0400, Jeff King wrote: > +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 > +' Actually, I guess remote-http actually sees just "/example.com/repo.git" for the case we're discussing. It segfaults with either input without this patch, and works with it, though. (I didn't test "git clone" directly because it hides the segfault made by the remote helper, so we cannot tell the difference between a segfault and "this URL is broken"). -Peff ```