Make URL parser stricter

(1) By anonymous on 2021-09-28 13:50:38 [link] [source]

This forum post mentions it is an error if fossil would do an request like this:

POST home HTTP/1.0

But this can actually happen when using port number in the URL like so:

$ fossil clone http://127.0.0.1:8080home

This can be observed either with nc -l 127.0.0.1 8080 or test-urlparser.

The reason for this is because the parsing of the hostname stops at 0, ':' or '/' but after parsing the port number it is not checked if the next character is 0 or '/'. With this change an error is printed in this case. The help text for the clone command also suggests that such URLs should be invalid.

Index: src/url.c
==================================================================
--- src/url.c
+++ src/url.c
@@ -196,10 +196,13 @@
       i++;
       while( (c = zUrl[i])!=0 && fossil_isdigit(c) ){
         pUrlData->port = pUrlData->port*10 + c - '0';
         i++;
       }
+      if( c!=0 && c!='/' ){
+        fossil_fatal("expected '/' after port number");
+      }
       pUrlData->hostname = mprintf("%s:%d", pUrlData->name, pUrlData->port);
     }else{
       pUrlData->port = pUrlData->dfltPort;
       pUrlData->hostname = pUrlData->name;
     }

(2.5) By Andy Bradford (andybradford) on 2021-09-29 03:13:07 edited from 2.4 in reply to 1 [source]

> But this can actually happen when using port number in the URL like
> so:
>
> $ fossil clone http://127.0.0.1:8080home

Nice find! However, just to clarify, whether or not that results in a
404 depends on how the Fossil server on the remote side is operating.
When operating in "file mode", it does not, in fact, result in an error
(mostly because the server doesn't actually have to parse the path to
determine which repository to open---it's forcing the repository to be
the one named on the command line, and consquently, any path is as good
as another):

$ fossil ser /tmp/new.fossil
Listening for HTTP requests on TCP port 8080

$ fossil clone http://localhost:8080whatever
Round-trips: 2 Artifacts sent: 0 received: 4
...
opening the new ./localhost:8080whatever.fossil repository in directory ./localhost:8080whatever...
Autosync: http://localhost:8080whatever
...
repository: /tmp/clone/localhost:8080whatever.fossil
local-root: /tmp/clone/localhost:8080whatever/

So, despite the malformed URL, it actually does result in a successful
clone. The result is still pretty ugly in my opinion as it created a
weird looking clone and directory:

$ ls -1
localhost:8080whatever/
localhost:8080whatever.fossil

Definitely not what one would expect!

Again, I can have whatever I want for the path when Fossil server is
operating in "file mode":

$ fossil clone http://localhost:8080/some/absurdly/wrong/path
...
opening the new ./path.fossil repository in directory ./path...
Autosync: http://localhost:8080/some/absurdly/wrong/path
...
repository: /tmp/clone/path.fossil

It only results in a 404 when the Fossil server is operating in
"directory mode" serving multiple fossils:

$ fossil ser /tmp
Listening for HTTP requests on TCP port 8080

$ fossil clone http://localhost:8080new
server says: 404 Not Found
Clone done, wire bytes sent: 266 received: 350 ip: 127.0.0.1
server returned an error - clone aborted

In both cases the client actually sent a POST that had no / as a prefix
(because there was none in the path found in the parsing of the URL).

Still, I think it's a legitimate URL parsing bug. Kudos!

Andy