Make URL parser stricter
(1) By anonymous on 2021-09-28 13:50:38 [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 [link] [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