Advisory: The minissdpd daemon (1.2.20130907-3) is affected by an improper validation of array index weakness (CWE-129) Advisory ID: SPADV2016-02 Revision: 0.3 Release Date: 2016/03/04 Last Modified: 2016/03/21 Date Reported: 2016/03/04 Author: Salva Peiró (speirofr at gmail.com) Affected Software: minissdpd version 1.2.20130907-3 Remotely Exploitable: No Locally Exploitable: Yes Vendor URL: http://miniupnp.tuxfamily.org/minissdpd.html Vendor Status: Vendor has been notified CVE-ID: CVE-2016-3178 CVE-2016-3179 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 that 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 check has already been applied to the upstream minissdpd repository see [2], the check 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?= 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/01 - Author notified of the security issue 2016/03/01 - Author fixed the security issue (see [2]) 2016/03/04 - Vendor notified 2016/03/07 - Advisory notified to oss-security 2016/03/16 - CVEs assigned CVE-2016-3178 CVE-2016-3179 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 Changes ======= Revision 0.1 - Initial draft release to the vendor https://bugs.debian.org/cgi-bin/pkgreport.cgi?pkg=minissdpd;dist=unstable Revision 0.2 - Security advisory notified to oss-security http://seclists.org/oss-sec/2016/q1/551 Revision 0.3 - Update security advisory CVE: CVE-2016-3178 CVE-2016-3179 http://www.openwall.com/lists/oss-security/2016/03/16/13 Disclaimer ========== The information within this advisory may change without notice. Use of this information constitutes acceptance for use in an AS IS condition. There are no warranties, implied or express, with regard to this information. In no event shall the author be liable for any direct or indirect damages whatsoever arising out of or in connection with the use or spread of this information. Any use of this information is at the user's own risk. Copyright 2016 Salva Peiró. All rights reserved.