{"thread":{"id":"55386","subject":"[PATCH] daemon: sanitize all directory separators","startedAt":"2021-03-25T16:22:28Z","lastAt":"2021-03-26T14:48:10Z","messageCount":3,"participants":["René Scharfe.","Jeff King","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"420194","messageId":"314451a4-ee8f-fb68-016f-66a4f3bea409@web.de","threadId":"55386","inReplyTo":null,"subject":"[PATCH] daemon: sanitize all directory separators","fromName":"René Scharfe.","fromEmail":"l.s.r@web.de","sentAt":"2021-03-25T16:21:24Z","receivedAt":"2021-03-25T16:22:28Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"When sanitizing client-supplied strings on Windows, also strip off\nbackslashes, not just slashes.\n\nSigned-off-by: René Scharfe <l.s.r@web.de>\n---\n daemon.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/daemon.c b/daemon.c\nindex 2ab7ea82eb..0561c19ee8 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -566,14 +566,14 @@ static void parse_host_and_port(char *hostport, char **host,\n\n /*\n  * Sanitize a string from the client so that it's OK to be inserted into a\n- * filesystem path. Specifically, we disallow slashes, runs of \"..\", and\n- * trailing and leading dots, which means that the client cannot escape\n- * our base path via \"..\" traversal.\n+ * filesystem path. Specifically, we disallow directory separators, runs\n+ * of \"..\", and trailing and leading dots, which means that the client\n+ * cannot escape our base path via \"..\" traversal.\n  */\n static void sanitize_client(struct strbuf *out, const char *in)\n {\n \tfor (; *in; in++) {\n-\t\tif (*in == '/')\n+\t\tif (is_dir_sep(*in))\n \t\t\tcontinue;\n \t\tif (*in == '.' && (!out->len || out->buf[out->len - 1] == '.'))\n \t\t\tcontinue;\n--\n2.30.2\n"},{"id":"420230","messageId":"YF1hkaoFU4CV9zna@coredump.intra.peff.net","threadId":"55386","inReplyTo":"314451a4-ee8f-fb68-016f-66a4f3bea409@web.de","subject":"Re: [PATCH] daemon: sanitize all directory separators","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-03-26T04:22:41Z","receivedAt":"2021-03-26T04:23:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 25, 2021 at 05:21:24PM +0100, René Scharfe. wrote:\n\n> When sanitizing client-supplied strings on Windows, also strip off\n> backslashes, not just slashes.\n> [...]\n>  static void sanitize_client(struct strbuf *out, const char *in)\n>  {\n>  \tfor (; *in; in++) {\n> -\t\tif (*in == '/')\n> +\t\tif (is_dir_sep(*in))\n\nYeah, this seems like the obviously correct thing to be doing.\n\n-Peff\n"},{"id":"420262","messageId":"nycvar.QRO.7.76.6.2103261543550.52@tvgsbejvaqbjf.bet","threadId":"55386","inReplyTo":"xmqq5z1fuen6.fsf@gitster.g","subject":"Re: [PATCH] daemon: sanitize all directory separators","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-03-26T14:47:14Z","receivedAt":"2021-03-26T14:48:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio & René,\n\nOn Thu, 25 Mar 2021, Junio C Hamano wrote:\n\n> René Scharfe. <l.s.r@web.de> writes:\n>\n> > When sanitizing client-supplied strings on Windows, also strip off\n> > backslashes, not just slashes.\n> >\n> > Signed-off-by: René Scharfe <l.s.r@web.de>\n> > ---\n> >  daemon.c | 8 ++++----\n> >  1 file changed, 4 insertions(+), 4 deletions(-)\n>\n> I do not know how common is it to run \"git daemon\" on Windows, but\n> it would be nice to have at least an ack from Windows person.\n\nHere is my ACK.\n\nI do not have any precise numbers, of course, as we do not have any\ntelemetry in Git for Windows (for better or worse).\n\nThere _are_ occasional reports about something in `git daemon` not\nworking; Most notably, you have to turn off the sideband to make `push`\nwork.\n\n> > diff --git a/daemon.c b/daemon.c\n> > index 2ab7ea82eb..0561c19ee8 100644\n> > --- a/daemon.c\n> > +++ b/daemon.c\n> > @@ -566,14 +566,14 @@ static void parse_host_and_port(char *hostport, char **host,\n> >\n> >  /*\n> >   * Sanitize a string from the client so that it's OK to be inserted into a\n> > - * filesystem path. Specifically, we disallow slashes, runs of \"..\", and\n> > - * trailing and leading dots, which means that the client cannot escape\n> > - * our base path via \"..\" traversal.\n> > + * filesystem path. Specifically, we disallow directory separators, runs\n> > + * of \"..\", and trailing and leading dots, which means that the client\n> > + * cannot escape our base path via \"..\" traversal.\n>\n> Not a new problem, but \"runs of '..'\" confused me.  If I am reading\n> the code right, we disallow directory separators (by ignoring) and\n> two or more '.' in a row (by squashing them into a single '.').\n\nIndeed, the code is a bit funny in that respect. But at least it keeps us\nsomewhat safe: there is currently no way to break out of the directory to\nthe parent directory, whether with this path or not. (Phew!)\n\nThanks,\nDscho\n\n>\n> >   */\n> >  static void sanitize_client(struct strbuf *out, const char *in)\n> >  {\n> >  \tfor (; *in; in++) {\n> > -\t\tif (*in == '/')\n> > +\t\tif (is_dir_sep(*in))\n> >  \t\t\tcontinue;\n> >  \t\tif (*in == '.' && (!out->len || out->buf[out->len - 1] == '.'))\n> >  \t\t\tcontinue;\n> > --\n> > 2.30.2\n>\n> Thanks.\n>\n"}]}