Processed: Re: Bug#883118: Crash in libupnp -- NULL-ptr dereference
Processing control commands: > tag -1 += upstream fixed-upstream Bug #883118 [libupnp] libupnp: SEGV in upnp/src/genlib/net/http/httpreadwrite.c:1662 Ignoring request to alter tags of bug #883118 to the same tags previously set -- 883118: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=883118 Debian Bug Tracking System Contact ow...@bugs.debian.org with problems
Processed: Re: Bug#883118: Crash in libupnp -- NULL-ptr dereference
Processing control commands: > tag -1 += upstream fixed-upstream Bug #883118 [libupnp] libupnp: SEGV in upnp/src/genlib/net/http/httpreadwrite.c:1662 Added tag(s) fixed-upstream and upstream; removed tag(s) confirmed. -- 883118: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=883118 Debian Bug Tracking System Contact ow...@bugs.debian.org with problems
Bug#883118: Crash in libupnp -- NULL-ptr dereference
Control: tag -1 += upstream fixed-upstream
Hello,
On Sun, Dec 10, 2017 at 05:47:43PM +0100, Tobias Frost wrote:
> 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.
thanks, that reproduction recipe was very helpful.
> 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)
This is already fixed upstream, see
https://github.com/mrjimenez/pupnp/commit/70e3d626378e12ea50d76dfda50311c8bb4a2a78
> --- 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);
I saw this inconsistency but didn't notice that this runs in an
assert(0). It seems nobody runs debug builds of libupnp :-\.
I'll prepare a patch for upstream.
Best regards
Uwe
signature.asc
Description: PGP signature
Bug#883118: Crash in libupnp -- NULL-ptr dereference
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);
signature.asc
Description: PGP signature
Processed: Bug#883118: Crash in libupnp -- NULL-ptr dereference
Processing control commands: > reassign -1 libupnp 1:1.6.24 Bug #883118 [libupnp] libupnp: SEGV in upnp/src/genlib/net/http/httpreadwrite.c:1662 Ignoring request to reassign bug #883118 to the same package Bug #883118 [libupnp] libupnp: SEGV in upnp/src/genlib/net/http/httpreadwrite.c:1662 There is no source info for the package 'libupnp' at version '1:1.6.24' with architecture '' Unable to make a source version for version '1:1.6.24' Ignoring request to alter found versions of bug #883118 to the same values previously set > retitle -1 libupnp: SEGV in upnp/src/genlib/net/http/httpreadwrite.c:1662 Bug #883118 [libupnp] libupnp: SEGV in upnp/src/genlib/net/http/httpreadwrite.c:1662 Ignoring request to change the title of bug#883118 to the same title > affects -1 gmediarender Bug #883118 [libupnp] libupnp: SEGV in upnp/src/genlib/net/http/httpreadwrite.c:1662 Ignoring request to set affects of bug 883118 to the same value previously set -- 883118: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=883118 Debian Bug Tracking System Contact ow...@bugs.debian.org with problems
Processed: Bug#883118: Crash in libupnp -- NULL-ptr dereference
Processing control commands: > reassign -1 libupnp 1:1.6.24 Bug #883118 [gmediarender] gmediarender crashes when gets an upnp search Bug reassigned from package 'gmediarender' to 'libupnp'. No longer marked as found in versions gmrender-resurrect/0.0.7~git20160329+repack-1. Ignoring request to alter fixed versions of bug #883118 to the same values previously set Bug #883118 [libupnp] gmediarender crashes when gets an upnp search There is no source info for the package 'libupnp' at version '1:1.6.24' with architecture '' Unable to make a source version for version '1:1.6.24' Marked as found in versions 1:1.6.24. > retitle -1 libupnp: SEGV in upnp/src/genlib/net/http/httpreadwrite.c:1662 Bug #883118 [libupnp] gmediarender crashes when gets an upnp search Changed Bug title to 'libupnp: SEGV in upnp/src/genlib/net/http/httpreadwrite.c:1662' from 'gmediarender crashes when gets an upnp search'. > affects -1 gmediarender Bug #883118 [libupnp] libupnp: SEGV in upnp/src/genlib/net/http/httpreadwrite.c:1662 Added indication that 883118 affects gmediarender -- 883118: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=883118 Debian Bug Tracking System Contact ow...@bugs.debian.org with problems
