From c6a8879c8717aec43774913cc355054f383b4c97 Mon Sep 17 00:00:00 2001 From: Daniel Becker Date: Fri, 28 Feb 2014 00:00:26 -0800 Subject: [PATCH] miniupnpd/natpmp.c: avoid hang when all external ports in use The NAT-PMP code attempts to find a different eport if the requested one is already in use. If all eports are in use, that would previously cause the code to iterate through the range of eports forever. To avoid this case, we keep track of the first eport we attempted to use and abort the loop once we've cycled through all possible values exactly once (which takes us back to the initial eport). --- miniupnpd/natpmp.c | 94 +++++++++++++++++++++++++--------------------- 1 file changed, 52 insertions(+), 42 deletions(-) diff --git a/miniupnpd/natpmp.c b/miniupnpd/natpmp.c index 860c41b..6764732 100644 --- a/miniupnpd/natpmp.c +++ b/miniupnpd/natpmp.c @@ -205,6 +205,7 @@ void ProcessIncomingNATPMPPacket(int s, unsigned char *msg_buff, int len, int proto; char iaddr_old[16]; unsigned short iport_old; + unsigned short eport_first; unsigned int timestamp; iport = ntohs(*((uint16_t *)(req+4))); @@ -266,56 +267,65 @@ void ProcessIncomingNATPMPPacket(int s, unsigned char *msg_buff, int len, } else if(iport==0 || !check_upnp_rule_against_permissions(upnppermlist, num_upnpperm, eport, senderaddr->sin_addr, iport)) { resp[3] = 2; /* Not Authorized/Refused */ - } else do { - r = get_redirect_rule(ext_if_name, eport, proto, - iaddr_old, sizeof(iaddr_old), - &iport_old, 0, 0, 0, 0, - ×tamp, 0, 0); - if(r==0) { - if(strcmp(senderaddrstr, iaddr_old)==0 - && iport==iport_old) { - /* redirection allready existing */ - syslog(LOG_INFO, "port %hu %s already redirected to %s:%hu, replacing", - eport, (proto==IPPROTO_TCP)?"tcp":"udp", iaddr_old, iport_old); - /* remove and then add again */ - if(_upnp_delete_redir(eport, proto) < 0) { - syslog(LOG_ERR, "failed to remove port mapping"); - break; + } else { + eport_first = eport; + do { + r = get_redirect_rule(ext_if_name, eport, proto, + iaddr_old, sizeof(iaddr_old), + &iport_old, 0, 0, 0, 0, + ×tamp, 0, 0); + if(r==0) { + if(strcmp(senderaddrstr, iaddr_old)==0 + && iport==iport_old) { + /* redirection allready existing */ + syslog(LOG_INFO, "port %hu %s already redirected to %s:%hu, replacing", + eport, (proto==IPPROTO_TCP)?"tcp":"udp", iaddr_old, iport_old); + /* remove and then add again */ + if(_upnp_delete_redir(eport, proto) < 0) { + syslog(LOG_ERR, "failed to remove port mapping"); + break; + } + } else { + eport++; + if(eport == eport_first) { /* no external port available */ + syslog(LOG_ERR, "Failed to find available eport for NAT-PMP %hu %s->%s:%hu", + eport, (proto==IPPROTO_TCP)?"tcp":"udp", senderaddrstr, iport); + resp[3] = 3; /* Failure */ + break; + } + continue; } - } else { - eport++; - continue; } - } - { /* do the redirection */ - char desc[64]; + { /* do the redirection */ + char desc[64]; #if 0 - timestamp = (unsigned)(time(NULL) - startup_time) - + lifetime; - snprintf(desc, sizeof(desc), "NAT-PMP %u", timestamp); + timestamp = (unsigned)(time(NULL) - startup_time) + + lifetime; + snprintf(desc, sizeof(desc), "NAT-PMP %u", timestamp); #else - timestamp = time(NULL) + lifetime; - snprintf(desc, sizeof(desc), "NAT-PMP %hu %s", - eport, (proto==IPPROTO_TCP)?"tcp":"udp"); + timestamp = time(NULL) + lifetime; + snprintf(desc, sizeof(desc), "NAT-PMP %hu %s", + eport, (proto==IPPROTO_TCP)?"tcp":"udp"); #endif - /* TODO : check return code */ - if(upnp_redirect_internal(NULL, eport, senderaddrstr, - iport, proto, desc, - timestamp) < 0) { - syslog(LOG_ERR, "Failed to add NAT-PMP %hu %s->%s:%hu '%s'", - eport, (proto==IPPROTO_TCP)?"tcp":"udp", senderaddrstr, iport, desc); - resp[3] = 3; /* Failure */ + /* TODO : check return code */ + if(upnp_redirect_internal(NULL, eport, senderaddrstr, + iport, proto, desc, + timestamp) < 0) { + syslog(LOG_ERR, "Failed to add NAT-PMP %hu %s->%s:%hu '%s'", + eport, (proto==IPPROTO_TCP)?"tcp":"udp", senderaddrstr, iport, desc); + resp[3] = 3; /* Failure */ #if 0 - } else if( !nextnatpmptoclean_eport - || timestamp < nextnatpmptoclean_timestamp) { - nextnatpmptoclean_timestamp = timestamp; - nextnatpmptoclean_eport = eport; - nextnatpmptoclean_proto = proto; + } else if( !nextnatpmptoclean_eport + || timestamp < nextnatpmptoclean_timestamp) { + nextnatpmptoclean_timestamp = timestamp; + nextnatpmptoclean_eport = eport; + nextnatpmptoclean_proto = proto; #endif + } + break; } - break; - } - } while(r==0); + } while(r==0); + } *((uint16_t *)(resp+8)) = htons(iport); /* private port */ *((uint16_t *)(resp+10)) = htons(eport); /* public port */ *((uint32_t *)(resp+12)) = htonl(lifetime); /* Port Mapping lifetime */