Thanks Ben. I agree, I think we should try to make as limited a change here as possible. If I’m reading your comment correctly, you’re saying you’ll allow an absolute URI redirect when all of the following conditions are met:


The hostname portion of the URI is the same

The scheme is the same OR the scheme is changing from HTTP to HTTPS (an upgrade)


I do not think we want to allow HTTPS to HTTP redirects (a downgrade).


Thanks,

Philip





From: Ben Straub
Sent: ‎Monday‎, ‎September‎ ‎23‎, ‎2013 ‎7‎:‎18‎ ‎PM
To: libgit2/libgit2
Cc: Philip Kelley




In src/transports/http.c:
> @@ -279,6 +295,38 @@ static int on_headers_complete(http_parser *parser)
>  		}
>  	}
>  
> +	/* Check for a redirect.
> +	 * Right now we only permit a redirect to the same hostname. */
> +	if ((parser->status_code == 301 ||
> +		 parser->status_code == 302 ||
> +		 (parser->status_code == 303 && get_verb == s->verb) ||
> +		 parser->status_code == 307) &&
> +		t->location) {
> +
> +		if (s->redirect_count >= 7) {
> +			giterr_set(GITERR_NET, "Too many redirects");
> +			return t->parse_error = PARSE_ERROR_GENERIC;
> +		}
> +
> +		if (t->location[0] != '/') {
> +			giterr_set(GITERR_NET, "Only relative redirects are supported");


Well, it's not that bad. This is kind of a security issue, right? Like a MITM attack, where I can have your git fetch http://foo.com/myrepo redirect to http://evil.com/badstuff. So limiting redirects to the same hostname makes sense, and allowing a transition to HTTPS makes sense.

But yeah, switching protocols mid-connection sounds like a terrible idea, so I'll make sure the new URL has http somewhere in the protocol.  :smiley: 

—
Reply to this email directly or view it on GitHub.