On Thu, Oct 06, 2016 at 09:28:34PM +0200, Ludovic Courtès wrote:
> Leo Famulari <l...@famulari.name> skribis:
> > +From d64d6a44906b5aa5306bdf1708531d698654dda5 Mon Sep 17 00:00:00 2001
> > +From: Matthew Garrett <mj...@srcf.ucam.org>
> > +Date: Tue, 23 Feb 2016 13:53:20 -0800
> > +Subject: [PATCH] Don't allow unhandled POSTs to write to the filesystem by
> > + default
> > +
> > +If there's no registered handler for a POST request, the default behaviour
> > +is to write it to the filesystem. Several million deployed devices appear
> > +to have this behaviour, making it possible to (at least) store arbitrary
> > +data on them. Add a configure option that enables this behaviour, and
> > change
> > +the default to just drop POSTs that aren't directly handled.
>
> Fun. :-)
Tons!
> > +diff --git a/configure.ac b/configure.ac
> > +index 9548913..a8731b5 100644
> > +--- a/configure.ac
> > ++++ b/configure.ac
>
> Shouldn’t it require an autoreconf phase?
Right, it would...
> I would suggest shrinking this patch to just:
>
> > +--- a/upnp/src/genlib/net/http/webserver.c
> > ++++ b/upnp/src/genlib/net/http/webserver.c
> > +@@ -1367,9 +1367,13 @@ static int http_RecvPostMessage(
> > + if (Fp == NULL)
> > + return HTTP_INTERNAL_SERVER_ERROR;
> > + } else {
> > ++#ifdef UPNP_ENABLE_POST_WRITE
> > + Fp = fopen(filename, "wb");
> > + if (Fp == NULL)
> > + return HTTP_UNAUTHORIZED;
> > ++#else
> > ++ return HTTP_NOT_FOUND;
> > ++#endif
>
> … with “#if 0” instead of “#ifdef UPNP_ENABLE_POST_WRITE”.
>
> WDYT?
I agree. Let's disable it unconditionally for now. When upstream cuts a
new release, the conditional feature handling will make it into our
package.
Thanks for the careful review.
> Feel free to commit adjusted as you see fit!
Done as 9e672bcc0b61a007ea29858517b58896dc1b9449
