On Wed, Mar 16, 2016 at 11:54:07AM +0100, Anton Wuerfel wrote: > When passing a malformed URL to http_init() in http.c, git dies from a null > pointer dereference. An example for a malformed URL is http:/git-scm.com (note > the single slash after the protocol). > This patch adds simple error handling as git notices the malformed URL already, > but never checks the error value. > > When passing a malformed URL, credential_from_url(struct credential *c, const char *url) > initializes *c with null values. When the existence of `://` in url is checked, > the function returns without further change of *c. > The null pointer dereference occurs in get_curl_handle () at http.c:593, when > the `protocol` field of struct credential is strcmp'ed: So I think the most direct bug here is that line 593 assumes that http_auth.protocol is not NULL, when it might very well be (if we could not parse the protocol). Your solution is to detect early that we don't have a URL that curl can parse, and bail. I think that's probably a reasonable thing to do in general. But it doesn't make me certain that there's a case that curl might parse that our credential url-parser might not. And the code in question does not even care about credentials at all! It's just piggy-backing on the earlier parse done by the credential code. I think it would make much more sense for it to rely on the normalized url we produce. IOW, to do something like: if (starts_with(normalized_url, "https://";)) /* https stuff */ else if (starts_with(normalized_url, "http://";)) /* http stuff */ else /* other stuff */ Note that the current code doesn't actually check for "http" (versus other protocols; despite the name http_init(), this code gets run for the probably-never-used-these-days git-over-ftp protocol). I suspect we are respecting http_proxy for ftp connections, which is silly. Note that normalized_url is freed before this point, so we may have to hold onto it longer. Or it may be possible to use the broken-down representation from config.url; I didn't look. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html