curl / Mailing Lists / curl-library / Single Mail

curl-library

Re: stricter host name requirements for file:// URLs (was Re: [SECURITY ADVISORY] curl invalid URL parsing with '#')

From: Kamil Dudka <kdudka_at_redhat.com>
Date: Fri, 11 Nov 2016 16:20:56 +0100

On Friday, November 11, 2016 15:59:52 Daniel Stenberg wrote:
> On Fri, 11 Nov 2016, Kamil Dudka wrote:
> > Yes, we do not expect a "valid host name". What about the following
> > wording?>
> > "Expected 'localhost' or empty string as host name in file:// URL"
>
> Since it doesn't mention the trailing slash, it makes the error string very
> weird for this URL: "file://localhost". But maybe good enough.
>
> >> It actually has to be changed to
> >>
> >> if(ptr[0] && ('/' == ptr[1]))
> >
> > Now I am confused. Which case should it actually cover?
>
> for "file://localhost/", ptr is made to point to "&path[10]" which is the
> byte following the third slash. So ptr[0] is the trailing zero byte and we
> cannot read ptr[1] as that is beyond our given string.
>
> > By reverse engineering I made up a URL "file://localhost///etc/fstab" to
> > get there ... but it does not look like a case we need to handle
> > specially :-) So what is the purpose of this condition? Which URLs do we
> > need it for?
> Now you talk about the need, or no need, for the memmove() there. Yeah, it
> is an attempt to avoid having two initial slashes but you're right that for
> *nix at least having several slashes isn't a problem... I'm not sure how
> that is treated by other systems though.

Nope. We need memmove() to translate "localhost/etc/fstab" to "etc/fstab"
in URL "file://localhost/etc/fstab". I am asking for an example of a sensible
URL that would be parsed differently after applying the following patch:

--- a/lib/url.c
+++ b/lib/url.c
@@ -4090,15 +4090,10 @@ static CURLcode parseurlandfillconn(struct Curl_easy
*data,
          slash preceding foo is a separator and not a slash for the path,
          a URL as file://localhost//foo must be valid as well, to refer to
          the same file with an absolute path.
       */

- if(ptr[0] && ('/' == ptr[1]))
- /* if there was two slashes, we skip the first one as that is then
- used truly as a separator */
- ptr++;
-
       /* This cannot be made with strcpy, as the memory chunks overlap! */
       memmove(path, ptr, strlen(ptr)+1);
     }

     protop = "file"; /* protocol string */
-------------------------------------------------------------------
List admin: https://cool.haxx.se/list/listinfo/curl-library
Etiquette: https://curl.haxx.se/mail/etiquette.html
Received on 2016-11-11