Package: minissdpd
Version: 1.2.20130907-3
Justification: renders package unusable
Severity: grave
Tags: security patch

Dear Maintainer,

The following bug report provides the technical description and bug fixes
and has been extracted from the detailed security advisory
at http://speirofr.appspot.com/files/advisory/SPADV-2016-02.md

Best,

Vulnerability details
=====================

The minissdpd daemon (version: 1.2.20130907-3) contains a
improper validation of array index vulnerability (CWE-129)
when processing requests sent to the Unix socket at /var/run/minissdpd.sock
the Unix socket can be accessed by an unprivileged user to send invalid
request causes an out-of-bounds memory access that crashes the minissdpd
daemon.

Technical Details
=================

The vulnerability can be triggered by a local unprivileged user performs
the following request:

$ echo -en '\x04\x01\x60\x8f\xff\xff\xff\x7f\x0a' | nc.openbsd -U
/var/run/minissdpd.sock

The request is processed by the processRequest() function at minissdpd.c
which identifies the request of type=4, and performs the parsing of the
"new service" request, the decoding of the length of the usn field
performed at
line 663, sets l = 0xffffffff, with p = buf+4, and n = 9, the negative
length
l=-1 passes the check at line 664 with (buf+4-1) < (buf + 9), continuing
with
the allocation of the usn field at line 673, that initialises newserv->usn =
malloc(0), where in the case of Linux malloc(3) the allocator returns a
pointer
that can be later passed to free().  The line 668 attempts to copy
0xffffffff
bytes from the message pointer p to newserv->usn, that causes the daemon to
perform an out-of-bound memory access writing outside the allocated buffer.

~~~
663         DECODELENGTH_CHECKLIMIT(l, p, buf + n);
664         if(p+l > buf+n) {
665             syslog(LOG_WARNING, "bad request (length encoding)");
666             goto error;
667         }
...
673         newserv->usn = malloc(l + 1);
674         if(!newserv->usn) {
675             syslog(LOG_ERR, "cannot allocate memory");
676             goto error;
677         }
668         memcpy(newserv->usn, p, l);
~~~

The problem is the incorrect validation on the length returned by the
DECODELENGTH_CHECKLIMIT macro at line 664, that does not consider negative
length values. The fix of the length has already been applied to the
upstream
minissdpd repository see [2], the bug happens at multiple locations after
the
DECODELENGTH_CHECKLIMIT macro that also need to be fixed:

~~~
          DECODELENGTH_CHECKLIMIT(l, p, buf + n);
-         if(p+l > buf+n) {
+         if(l > (unsigned)(buf+n-p)) {
             syslog(LOG_ERR, "cannot allocate memory");
             goto error;
          }
~~~

After performing the corrections of the length check the minissdpd daemon
properly detects the invalid negative values and performs error handling.
However, the error handling code at line 753 attempts to free the undefined
memory contents that newserv = malloc() allocated at line 642.

~~~
753     if(newserv) {
754         free(newserv->st);
755         free(newserv->usn);
756         free(newserv->server);
757         free(newserv->location);
758         free(newserv);
759         newserv = NULL;
760     }
~~~

The issue is corrected by applying initialising the contents of the newserv
to zero [3].
That causes free() to correctly operate when freeing the uninitialised
struct fields.

~~~
642         newserv = malloc(sizeof(struct service));
643         if(!newserv) {
644             syslog(LOG_ERR, "cannot allocate memory");
645             goto error;
646         }
+               memset(newserv, 0, sizeof(struct service));
~~~

Solution
========

Apply the proposed fixes, contained in the patch below.

~~~
>From 2f6746a0c00872b977cc81452d77463aa39609e7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Salva=20Peir=C3=B3?= <speir...@gmail.com>
Date: Fri, 4 Mar 2016 12:38:18 +0100
Subject: [PATCH] Fix minissdpd.c handling of request with negative length

---
 minissdpd.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/minissdpd.c b/minissdpd.c
index 520a6c5..1cd079e 100644
--- a/minissdpd.c
+++ b/minissdpd.c
@@ -555,7 +555,7 @@ void processRequest(struct reqelem * req)
    type = buf[0];
    p = buf + 1;
    DECODELENGTH_CHECKLIMIT(l, p, buf + n);
-       if(p+l > buf+n) {
+       if(l > (unsigned)(buf+n-p)) {
        syslog(LOG_WARNING, "bad request (length encoding)");
        goto error;
    }
@@ -644,6 +644,7 @@ void processRequest(struct reqelem * req)
            syslog(LOG_ERR, "cannot allocate memory");
            goto error;
        }
+               memset(newserv, 0, sizeof(struct service));
        if(containsForbiddenChars(p, l)) {
            syslog(LOG_ERR, "bad request (st contains forbidden chars)");
            goto error;
@@ -661,7 +662,7 @@ void processRequest(struct reqelem * req)
            goto error;
        }
        DECODELENGTH_CHECKLIMIT(l, p, buf + n);
-               if(p+l > buf+n) {
+               if(l > (unsigned)(buf+n-p)) {
            syslog(LOG_WARNING, "bad request (length encoding)");
            goto error;
        }
@@ -679,7 +680,7 @@ void processRequest(struct reqelem * req)
        newserv->usn[l] = '\0';
        p += l;
        DECODELENGTH_CHECKLIMIT(l, p, buf + n);
-               if(p+l > buf+n) {
+               if(l > (unsigned)(buf+n-p)) {
            syslog(LOG_WARNING, "bad request (length encoding)");
            goto error;
        }
@@ -697,7 +698,7 @@ void processRequest(struct reqelem * req)
        newserv->server[l] = '\0';
        p += l;
        DECODELENGTH_CHECKLIMIT(l, p, buf + n);
-               if(p+l > buf+n) {
+               if(l > (unsigned)(buf+n-p)) {
            syslog(LOG_WARNING, "bad request (length encoding)");
            goto error;
        }
--
2.1.4
~~~

Affected versions
=================

Debian: https://packages.debian.org/jessie/minissdpd
minissdpd version 1.2.20130907-3

Ubuntu: https://launchpad.net/ubuntu/+source/minissdpd
minissdpd version 1.2.20130907-3


History
=======

  2016/03/04 - Vendor notified


Credits
=======

  Vulnerability found and advisory written by Salva Peiró.


References
==========

 [1] https://speirofr.appspot.com/files/advisory/SPADV-2016-02.md
 [2]
https://github.com/miniupnp/miniupnp/commit/b238cade9a173c6f751a34acf8ccff838a62aa47
 [3]
https://github.com/miniupnp/miniupnp/commit/140ee8d2204b383279f854802b27bdb41c1d5d1a




-- System Information:
Debian Release: 8.2
  APT prefers stable
  APT policy: (500, 'stable')
Architecture: i386 (i686)

Kernel: Linux 3.16.0-4-686-pae (SMP w/1 CPU core)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)

Versions of packages minissdpd depends on:
ii  libc6  2.19-18+deb8u1

minissdpd recommends no packages.

minissdpd suggests no packages.

-- no debconf information

Reply via email to