From: Stefan Sperling Subject: Re: gotwebd: percent-encode querystrings To: Omar Polo Cc: gameoftrees@openbsd.org Date: Wed, 7 Sep 2022 07:04:24 +0200 On Tue, Sep 06, 2022 at 11:52:16PM +0200, Omar Polo wrote: > On 2022/09/06 18:50:03 +0200, Stefan Sperling wrote: > > Should we also refuse attempts to encode control-characters into URLs > > (byte values between 1 and 31, excluding 9 which is '\t')? > > See https://daniel.haxx.se/blog/2022/09/05/a-bug-that-was-23-years-old-or-not/ > > for reasons why we might want to block them. > > Thanks for linking the post! It was interesting to read, i wasn't > aware of that issue. > > It doesn't directly touches us as the request is parsed by httpd(8) > (or whatever frontend people are using) that sends us the data parsed > via fastcgi params. I'm expecting the web server to catch and deal > with this kinds of issues. Yes, it may well be that we can leave such checks to httpd(8). I don't know how close the FCGI backend is to raw request processing. But I somewhat doubt that httpd(8) would really care about semantics of the data which it is passing on towards the backend. So I still wanted to bring this up, just in case. Whenever responsibility for such safety checks is left up to another piece of software, there is a potential that we end up lacking _any_ such checks while pointing fingers at the other component for responsibility. > I don't think it's worth adding stuff to gotwebd to add this case -- > we don't even use cookies :D -- but however what we could do is to: > > - reject the request if the provided hostname has funny characters in > it > > don't really see a point in it, we just read the provided > SERVER_NAME fastcgi parameter and match it against the list of > servers. but it's easy to do if wanted. Perhaps just do it anyway since it's a cheap check? > - reject the request if the querystring has invalid characters in it > > this has nothing to do with the linked post, but I think that > gotweb_urldecode should error out on invalid characters in the > querystring. I haven't thought about it when first writing the > decoder but now I'm quite confident it should fail. Will look into > it. Thanks. I believe rejecting control chars in the query we receive would be a good thing to do.