CVE-2019-12107: Unchecked Return Value (CWE-252)
value: The upnp_event_prepare function in upnpevents.c in MiniUPnP MiniUPnPd through 2.1 allows a remote attacker to leak information from the heap due to improper validation of an snprintf return value.
CWE description: The product does not check the return value from a method or function, which can prevent it from detecting unexpected states and conditions.
CVSS v3 base score: 7.5
Published: 2019-05-15T23:29Z
Repository: https://github.com/miniupnp/miniupnp
Language: C
Files changed: miniupnpd/upnpevents.c
Commit date: 2018-12-18 22:37:14 +0100
Commit message: upnp_event_prepare(): check the return value of snprintf()

Diff: diff --git a/miniupnpd/upnpevents.c b/miniupnpd/upnpevents.c
index d96bccbe..3bc402f3 100644
--- a/miniupnpd/upnpevents.c
+++ b/miniupnpd/upnpevents.c
@@ -441,23 +441,38 @@ static void upnp_event_prepare(struct upnp_event_notify * obj)
 	default:
 		xml = NULL;
 		l = 0;
 	}
 	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);
+	for (;;) {
+		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->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 (obj->tosend < 0) {
+			syslog(LOG_ERR, "%s: snprintf() failed", "upnp_event_prepare");
+			if(xml) {
+				free(xml);
+			}
+			obj->state = EError;
+			return;
+		} else if (obj->tosend < obj->buffersize) {
+			break; /* the buffer was large enough */
+		}
+		/* Try again with a buffer big enough */
+		free(obj->buffer);
+		obj->buffersize = obj->tosend + 1;	/* reserve space for the final 0 */
 	}
-	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;

Vulnerable code: 	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);
Fixed code: 	for (;;) {
		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 (obj->tosend < 0) {
			syslog(LOG_ERR, "%s: snprintf() failed", "upnp_event_prepare");
			if(xml) {
				free(xml);
			}
			obj->state = EError;
			return;
		} else if (obj->tosend < obj->buffersize) {
			break; /* the buffer was large enough */
		}
		/* Try again with a buffer big enough */
		free(obj->buffer);
		obj->buffersize = obj->tosend + 1;	/* reserve space for the final 0 */