On Tue, Sep 06, 2022 at 11:52:16PM +0200, Omar Polo wrote:
> On 2022/09/06 18:50:03 +0200, Stefan Sperling <stsp@stsp.name> 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.