Re: [PATCH 1/1] gnu: libupnp: Fix CVE-2016-6255.

[Leo Famulari]

On Thu, Oct 06, 2016 at 09:28:34PM +0200, Ludovic Courtès wrote:
> Leo Famulari <address@hidden> skribis:
> > +From d64d6a44906b5aa5306bdf1708531d698654dda5 Mon Sep 17 00:00:00 2001
> > +From: Matthew Garrett <address@hidden>
> > +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