Control: reassign -1 libupnp 1:1.6.24
Control: retitle -1 libupnp: SEGV in 
upnp/src/genlib/net/http/httpreadwrite.c:1662
Control: affects -1 gmediarender

Hallo Uwe,

I'm reassigning this bug as I'm suspecting it in the recent release of libupnp,
after I had debugged it a bit.

The bug does not trigger in 1.6.22.

How to reproduce:

Install gmediarender and (as a DLNA/uPnP control point) gupnp-tools.

run gmediarender , eg. gmediarender --logfile=/dev/stdout and then the DLNA
controlpoint, e.g  gupnp-av-cp

As soon as the the cp queries for the DLNA server, gmediarender crashes.

Debugging into it it segfaults in upnp/src/genlib/net/http/httpreadwrite.c:1662
deferencing a NULL pointer (extras being NULL); it is called from
upnp/src/genlib/net/http/webserver.c:1316; the relvant paremter is
"extra_headers", passed for the "E" command  (its NULL)

Looking at the diff from 1.6.22 it looks like that this has been touched:
- before, extra_headers was a string and if NULL it would have been set to an
  empty string ("" == '\0')
- now it is a struct which will be only allocated conditionally (line 1188 in
  webserver.c). Otherwise it is NULL.
- before, it used command "s" not "E"...

Adding this patch:

--- a/upnp/src/genlib/net/http/webserver.c
+++ b/upnp/src/genlib/net/http/webserver.c
@@ -1296,6 +1296,12 @@
                goto error_handler;
        }
 
+       if (!extra_headers) {
+               extra_headers = (struct Extra_Headers*) malloc(sizeof(struct 
Extra_Headers));
+               if (!extra_headers) goto error_handler;
+               extra_headers->name = NULL;
+       }
+
        /* Check if chunked encoding should be used. */
        if (using_virtual_dir && finfo.file_length == UPNP_USING_CHUNKED) {
                /* Chunked encoding is only supported by HTTP 1.1 clients */

seems to fix the crash, but as soon after the assertion on httpreadwrite:1862
will trigger. It looks like that in line 1672 a "else" is missing to correctly
chain up the commands after the new "E" command:

--- a/upnp/src/genlib/net/http/httpreadwrite.c
+++ b/upnp/src/genlib/net/http/httpreadwrite.c
@@ -1668,8 +1668,7 @@
                                }
                                extras++;
                        }
-               }
-               if (c == 's') {
+               } else if (c == 's') {
                        /* C string */
                        s = (char *)va_arg(argp, char *);
                        assert(s);


When applying the patch (attached), gmediarender stops crashing.
However, I cannot say if my changes are sane at all, thus not tagging
"patch". (and also I neglected return code checking of malloc)

Let me know if you need some details!

Cheers,
tobi 

--- a/upnp/src/genlib/net/http/webserver.c
+++ b/upnp/src/genlib/net/http/webserver.c
@@ -1296,6 +1296,12 @@
 		goto error_handler;
 	}
 
+	if (!extra_headers) {
+		extra_headers = (struct Extra_Headers*) malloc(sizeof(struct Extra_Headers));
+		if (!extra_headers) goto error_handler;
+		extra_headers->name = NULL;
+	}
+
 	/* Check if chunked encoding should be used. */
 	if (using_virtual_dir && finfo.file_length == UPNP_USING_CHUNKED) {
 		/* Chunked encoding is only supported by HTTP 1.1 clients */
--- a/upnp/src/genlib/net/http/httpreadwrite.c
+++ b/upnp/src/genlib/net/http/httpreadwrite.c
@@ -1668,8 +1668,7 @@
 				}
 				extras++;
 			}
-		}
-		if (c == 's') {
+		} else if (c == 's') {
 			/* C string */
 			s = (char *)va_arg(argp, char *);
 			assert(s);

Attachment: signature.asc
Description: PGP signature

Reply via email to