Vulnerability Research
February 5, 2019
By Ben Barnea

Security Issues Discovered in MiniUPnP

For the past several months, VDOO security research teams have been undertaking broad-scale research of leading safety and security IoT products. Open-source projects are deployed in many of those devices, therefore our research focuses on some of the most common projects, to provide the highest security level for those devices as well.

As part of this research, VDOO discovered several vulnerabilities in the open-source MiniUPnP library.

An information leak vulnerability (VD-1010) was automatically discovered using VDOO Vision™ platform. Then, while manually researching the library’s source code, a Use-After-Free (VD-1012) and NULL Dereference (VD-1011) vulnerabilities were also found. VD-1010, is not a critical one by itself, but it could have been abused as part of an exploit chain if more vulnerabilities were discovered. Information leak vulnerabilities are generally used to bypass the ASLR mechanism and gain access to sensitive user data and this is true in this case as well. VD-1011 allows an attacker to crash the MiniUPnP process which will lead to a denial of service of the UPnP functionality. Abusing VD-1012 to remotely execute code is not likely, as the specific conditions prevent reliable exploitation.

VDOO has responsibly disclosed the vulnerabilities to the project’s maintainer on December 18, 2018, who responded very promptly and uploaded their fixes on the same day. Maintainers who take the security of their users as seriously as in this case should be commended and appreciated and we would like to thank Thomas Bernard for this.

We recently stumbled upon a tweet from January 25, 2019 by a security researcher indicating prior knowledge of the information leak vulnerability before VDOO’s disclosure. The researcher also mentioned working on “finding a second vulnerability to accompany it". The maintainer’s fixes, resulting from VDOO’s disclosure, prevented any such exploitation even if such additional vulnerability was to be found. This is another case emphasizing the importance of responsibly disclosing vulnerabilities at the earliest possible stage as VDOO encourages.

In this article, we will discuss the technical information of the vulnerabilities found.

Background

UPnP (Universal Plug and Play) is a protocol that allows different network devices to discover and communicate with each other without the need for manual configuration. The discovery part is done by the SSDP protocol.

The MiniUPnP project includes implementations for UPnP client, UPnP daemon, and SSDP discovery daemon, for Internet Gateway Devices. This improves the connectivity for devices sitting behind a NAT router by providing functionality such as automatic port forwarding.

This library is very popular and is used by many IoT devices, such as TP-Link routers, Google WiFi, Samsung SmartThings WiFi.

Technical Deep-Dive

In this part of the article, all code references will be to the files in the latest vulnerable version available online that can be found here.

VD-1010 - Information Leak Vulnerability

A user can subscribe to UPnP events, supplying a ‘callback’ address. When an event occurs, the server sends an HTTP request to the user’s callback address.

An information leakage can be triggered by subscribing to an event using an especially long callback address. The callback address is passed to snprintf function to be used as part of the packet that is sent back to the user.

If the length of the formatted string created by snprintf is longer than the maximum-size parameter supplied, snprintf trims it to the maximum-size parameter’s size, but its return value is the number of bytes that would have been written if the buffer was large enough.

The code relies on the snprintf’s return value (obj->tosend) for calculating the number of bytes to send back to the user. Since the code fails to verify that the return value is shorter than the maximum-size parameter, the buffer that is sent back to the user can exceed the buffer’s length – leaking memory from the process’ heap. This vulnerability was fixed by this commit.

obj->buffersize = 1024;
obj->buffer = malloc(obj->buffersize);
if(!obj->buffer) {
                syslog(LOG_ERR, "%s: malloc returned NULL", "upnp_event_prepare");
                if(xml) {
                                free(xml);
                }
                obj->state = EError;
                return;
}
obj->tosend = snprintf(obj->buffer, obj->buffersize, notifymsg,
                       obj->path, obj->addrstr, obj->portstr, l+2,
                       obj->sub->uuid, obj->sub->seq,
                       l, xml);
if(xml) {
                free(xml);
                xml = NULL;
}
obj->state = ESending;

Figure 1 - line 455 of miniupnpd/upnpevents.c – In the code above, a buffer of 1024 bytes is allocated for the destination buffer. The obj->path (the user’s callback), one of the parameters passed to snprintf, is user controllable.

Our research team is working closely on generalizing the most common vulnerabilities that we and others find in common IoT devices. As an example, this vulnerability in miniupnpd is similar to one of the vulnerabilities we discovered in Axis cameras a few months ago, and we have used that knowledge to automatically detect even more cases such as this.

VD-1011 - Null Dereference Vulnerabilities

Two null dereferences were found due to insufficient validation of the value returned by the GetValueFromNameValueList function.

The server expects to receive specific fields from the user in XML format. Therefore, if the user fails to supply a requested key, the GetValueFromNameValueList function will return NULL.

ParseNameValue(h->req_buf + h->req_contentoff, h->req_contentlen, &data);
                int_ip = GetValueFromNameValueList(&data, "InternalClient");
                int_port = GetValueFromNameValueList(&data, "InternalPort");
                rem_host = GetValueFromNameValueList(&data, "RemoteHost");
                rem_port = GetValueFromNameValueList(&data, "RemotePort");
                protocol = GetValueFromNameValueList(&data, "Protocol");
 
                rport = (unsigned short)atoi(rem_port);
                iport = (unsigned short)atoi(int_port);

Figure 2 - line 1853 of miniupnpd/upnpsoap.c

As can be seen in the code above, atoi is called on rem_port and int_port. In case their values were not provided in the XML, their values will be NULL, causing the process to crash. (fixed by these two commits: 1 and 2)

The same behavior happens in AddPortMapping. (fixed by this commit)

Another NULL dereference happens in a code that parses a PCP request. (fixed by this commit)

VD-1012 - Use After Free Vulnerability

A use after free condition can occur if a user can exhaust the memory of the process. In that case, if the minissdp daemon tries to update the device’s information, it will fail allocating new memory for the device struct. Then, it will free the original struct without removing it from the devices list it maintains.

static int
updateDevice(const struct header * headers, time_t t)
{
    struct device ** pp = &devlist;
    struct device * p = *pp; /* = devlist; */
    while(p)
    {
        if(  p->headers[HEADER_NT].l == headers[HEADER_NT].l
          && (0==memcmp(p->headers[HEADER_NT].p, headers[HEADER_NT].p, headers[HEADER_NT].l))
          && p->headers[HEADER_USN].l == headers[HEADER_USN].l
          && (0==memcmp(p->headers[HEADER_USN].p, headers[HEADER_USN].p, headers[HEADER_USN].l)) )
        {
            /*printf("found! %d\n", (int)(t - p->t));*/
            syslog(LOG_DEBUG, "device updated : %.*s", headers[HEADER_USN].l, headers[HEADER_USN].p);
            p->t = t;
            /* update Location ! */
            if(headers[HEADER_LOCATION].l > p->headers[HEADER_LOCATION].l)
            {
                struct device * tmp;
                tmp = realloc(p, sizeof(struct device)
                    + headers[0].l+headers[1].l+headers[2].l);
                if(!tmp) /* allocation error */
                {
                    syslog(LOG_ERR, "updateDevice() : memory allocation error");
                    free(p);
                    return 0;
                }
                p = tmp;
                *pp = p;
            }
            memcpy(p->data + p->headers[0].l + p->headers[1].l,
                   headers[2].p, headers[2].l);
            /* TODO : check p->headers[HEADER_LOCATION].l */
            return 0;
        }
        pp = &p->next;
        p = *pp;               /* p = p->next; */
    }

Figure 3 - line 318 of minissdpd/minissdpd.c - In the code above, if the realloc fails, then p (the device struct pointer) is freed without updating the devlist.

When the list will be traversed, a crash will happen.

A fix for this issue can be found here.

About This Research

The research goal is to contribute knowledge and tools to mitigate risks, as well as encourage the devices' vendors to implement the right security for their products. In most cases, the research was carried out together with them, for the sake of efficiency and transparency.

The research findings are implemented in all our automated IoT security solutions for the widest risk mitigation coverage.

We believe that an appropriate implementation of the security essentials will dramatically decrease the chances of exploiting vulnerabilities on the device.

About VDOO

VDOO is a technology-driven company that strives to change the reality of unprotected connected devices by building a products line that supports device manufacturers in embedding security into their connected devices during the development stage. In addition to developing products and services, significant efforts are invested in a wide scope research of connected devices and its supply chain.

Credit

Ben Barnea, Security Researcher, VDOO

Share this post

You may also like