threads / discuss / 44031

segfault in http.c when https URL is mistyped

Subject: segfault in http.c when https URL is mistyped

## tl;dr

3 messages between Sep 7, 2016 and Sep 7, 2016.

replies: 2people: 2as markdown or json

Lars Wendler· Sep 7, 2016, 13:44 UTC · lore
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· Sep 7, 2016, 20:06 UTC · re: Lars Wendler · lore

Re: segfault in http.c when https URL is mistyped

On Wed, Sep 07, 2016 at 03:44:04PM +0200, Lars Wendler wrote:
Show 6 quoted lines
> 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 <peff@peff.net>
---
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· Sep 7, 2016, 20:11 UTC · re: Jeff King · lore

Re: segfault in http.c when https URL is mistyped

On Wed, Sep 07, 2016 at 04:06:42PM -0400, Jeff King wrote:
Show 7 quoted lines
> +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

← back to recent threads