segfault in http.c when https URL is mistyped

segfault in http.c when https URL is mistyped

[Lars Wendler]

[-- Attachment #1: Type: text/plain, Size: 761 bytes --]

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

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

[Jeff King]

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 <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

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

[Jeff King]

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