From 96aa863c78e9b746c3c9b008d777a30aca076248 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Sat, 18 Jul 2020 02:23:46 +0200 Subject: [PATCH 1/2] Fix check for reserved IP addresses in miniupnpc Check for 0.0.0.0, 192.168., 10. and 172. is not enough. Nowadays routers behind NAT are getting IP address from shared CG-NAT space 100.64.0.0/10. This patch adjust miniupnpc to check for all reserved IPv4 addresses. --- miniupnpc/miniupnpc.c | 61 ++++++++++++++++++++++++++++++++----------- 1 file changed, 46 insertions(+), 15 deletions(-) diff --git a/miniupnpc/miniupnpc.c b/miniupnpc/miniupnpc.c index 95ab6cf..cec8c4c 100644 --- a/miniupnpc/miniupnpc.c +++ b/miniupnpc/miniupnpc.c @@ -17,6 +17,11 @@ #include #define snprintf _snprintf #define strdup _strdup +#if !defined(_MSC_VER) +#include +#else /* !defined(_MSC_VER) */ +typedef unsigned long uint32_t; +#endif /* !defined(_MSC_VER) */ #ifndef strncasecmp #if defined(_MSC_VER) && (_MSC_VER >= 1400) #define strncasecmp _memicmp @@ -73,21 +78,49 @@ #define SERVICEPREFIX "u" #define SERVICEPREFIX2 'u' -/* check if an ip address is a private (LAN) address - * see https://tools.ietf.org/html/rfc1918 */ -static int is_rfc1918addr(const char * addr) +/* List of IP address blocks which are private / reserved and therefore not suitable for public external IP addresses */ +#define IP(a, b, c, d) (((a) << 24) + ((b) << 16) + ((c) << 8) + (d)) +#define MSK(m) (32-(m)) +static const struct { uint32_t address; uint32_t rmask; } reserved[] = { + { IP( 0, 0, 0, 0), MSK( 8) }, /* RFC1122 "This host on this network" */ + { IP( 10, 0, 0, 0), MSK( 8) }, /* RFC1918 Private-Use */ + { IP(100, 64, 0, 0), MSK(10) }, /* RFC6598 Shared Address Space */ + { IP(127, 0, 0, 0), MSK( 8) }, /* RFC1122 Loopback */ + { IP(169, 254, 0, 0), MSK(16) }, /* RFC3927 Link-Local */ + { IP(172, 16, 0, 0), MSK(12) }, /* RFC1918 Private-Use */ + { IP(192, 0, 0, 0), MSK(24) }, /* RFC6890 IETF Protocol Assignments */ + { IP(192, 0, 2, 0), MSK(24) }, /* RFC5737 Documentation (TEST-NET-1) */ + { IP(192, 31, 196, 0), MSK(24) }, /* RFC7535 AS112-v4 */ + { IP(192, 52, 193, 0), MSK(24) }, /* RFC7450 AMT */ + { IP(192, 88, 99, 0), MSK(24) }, /* RFC7526 6to4 Relay Anycast */ + { IP(192, 168, 0, 0), MSK(16) }, /* RFC1918 Private-Use */ + { IP(192, 175, 48, 0), MSK(24) }, /* RFC7534 Direct Delegation AS112 Service */ + { IP(198, 18, 0, 0), MSK(15) }, /* RFC2544 Benchmarking */ + { IP(198, 51, 100, 0), MSK(24) }, /* RFC5737 Documentation (TEST-NET-2) */ + { IP(203, 0, 113, 0), MSK(24) }, /* RFC5737 Documentation (TEST-NET-3) */ + { IP(224, 0, 0, 0), MSK( 4) }, /* RFC1112 Multicast */ + { IP(240, 0, 0, 0), MSK( 4) }, /* RFC1112 Reserved for Future Use + RFC919 Limited Broadcast */ +}; +#undef IP +#undef MSK + +static int addr_is_reserved(const char * addr_str) { - /* 192.168.0.0 - 192.168.255.255 (192.168/16 prefix) */ - if(COMPARE(addr, "192.168.")) + unsigned long addr_n; + uint32_t address; + size_t i; + + addr_n = inet_addr(addr_str); + if (addr_n == INADDR_NONE) return 1; - /* 10.0.0.0 - 10.255.255.255 (10/8 prefix) */ - if(COMPARE(addr, "10.")) - return 1; - /* 172.16.0.0 - 172.31.255.255 (172.16/12 prefix) */ - if(COMPARE(addr, "172.")) { - if((atoi(addr + 4) | 0x0f) == 0x1f) + + address = ntohl(addr_n); + + for (i = 0; i < sizeof(reserved)/sizeof(reserved[0]); ++i) { + if ((address >> reserved[i].rmask) == (reserved[i].address >> reserved[i].rmask)) return 1; } + return 0; } @@ -643,8 +676,7 @@ UPNP_GetValidIGD(struct UPNPDev * devlist, /* checks that status is connected AND there is a external IP address assigned */ if(is_connected && (UPNP_GetExternalIPAddress(urls->controlURL, data->first.servicetype, extIpAddr) == 0)) { - if(!is_rfc1918addr(extIpAddr) && (extIpAddr[0] != '\0') - && (0 != strcmp(extIpAddr, "0.0.0.0"))) + if(!addr_is_reserved(extIpAddr)) goto free_and_return; } FreeUPNPUrls(urls); @@ -665,8 +697,7 @@ UPNP_GetValidIGD(struct UPNPDev * devlist, #endif if(is_connected && (UPNP_GetExternalIPAddress(urls->controlURL, data->first.servicetype, extIpAddr) == 0)) { - if(!is_rfc1918addr(extIpAddr) && (extIpAddr[0] != '\0') - && (0 != strcmp(extIpAddr, "0.0.0.0"))) + if(!addr_is_reserved(extIpAddr)) goto free_and_return; } FreeUPNPUrls(urls); From 0c556655ea625c8236cc0dbfad52218b3666be06 Mon Sep 17 00:00:00 2001 From: Thomas Bernard Date: Thu, 24 Sep 2020 00:08:55 +0200 Subject: [PATCH 2/2] Move addr_is_reserved() to a specific source file and test it --- miniupnpc/.gitignore | 2 + miniupnpc/CMakeLists.txt | 11 ++++- miniupnpc/Makefile | 22 +++++++--- miniupnpc/Makefile.mingw | 5 ++- miniupnpc/addr_is_reserved.c | 71 ++++++++++++++++++++++++++++++++ miniupnpc/addr_is_reserved.h | 14 +++++++ miniupnpc/miniupnpc.c | 51 ++--------------------- miniupnpc/testaddr_is_reserved.c | 46 +++++++++++++++++++++ 8 files changed, 164 insertions(+), 58 deletions(-) create mode 100644 miniupnpc/addr_is_reserved.c create mode 100644 miniupnpc/addr_is_reserved.h create mode 100644 miniupnpc/testaddr_is_reserved.c diff --git a/miniupnpc/.gitignore b/miniupnpc/.gitignore index ba06bcc..ed30d5e 100644 --- a/miniupnpc/.gitignore +++ b/miniupnpc/.gitignore @@ -35,3 +35,5 @@ dist/ miniupnpc.egg-info/ init miniupnpc.pc +testaddr_is_reserved +validateaddr_is_reserved diff --git a/miniupnpc/CMakeLists.txt b/miniupnpc/CMakeLists.txt index 1f0d779..1c57173 100644 --- a/miniupnpc/CMakeLists.txt +++ b/miniupnpc/CMakeLists.txt @@ -68,6 +68,7 @@ set (MINIUPNPC_SOURCES portlistingparse.c receivedata.c listdevices.c + addr_is_reserved.c connecthostport.h igd_desc_parse.h minisoap.h @@ -82,6 +83,7 @@ set (MINIUPNPC_SOURCES upnpdev.h upnperrors.h upnpreplyparse.h + addr_is_reserved.h ${CMAKE_CURRENT_BINARY_DIR}/miniupnpcstrings.h ) @@ -179,17 +181,22 @@ if (UPNPC_BUILD_TESTS) add_executable (testigddescparse testigddescparse.c igd_desc_parse.c minixml.c miniupnpc.c miniwget.c minissdpc.c upnpcommands.c upnpreplyparse.c minisoap.c connecthostport.c - portlistingparse.c receivedata.c + portlistingparse.c receivedata.c addr_is_reserved.c ) target_link_libraries (testigddescparse PRIVATE miniupnpc-tests) add_executable (testminiwget testminiwget.c miniwget.c miniupnpc.c minisoap.c upnpcommands.c minissdpc.c upnpreplyparse.c minixml.c igd_desc_parse.c connecthostport.c - portlistingparse.c receivedata.c + portlistingparse.c receivedata.c addr_is_reserved.c ) target_link_libraries (testminiwget PRIVATE miniupnpc-tests) + add_executable (testaddr_is_reserved testaddr_is_reserved.c + addr_is_reserved.c + ) + target_link_libraries (testaddr_is_reserved PRIVATE miniupnpc-tests) + # set (UPNPC_INSTALL_TARGETS ${UPNPC_INSTALL_TARGETS} testminixml minixmlvalid testupnpreplyparse testigddescparse testminiwget) endif () diff --git a/miniupnpc/Makefile b/miniupnpc/Makefile index 0102f3b..794bbe2 100644 --- a/miniupnpc/Makefile +++ b/miniupnpc/Makefile @@ -1,9 +1,9 @@ # $Id: Makefile,v 1.134 2016/10/07 09:04:36 nanard Exp $ # MiniUPnP Project # http://miniupnp.free.fr/ -# http://miniupnp.tuxfamily.org/ +# https://miniupnp.tuxfamily.org/ # https://github.com/miniupnp/miniupnp -# (c) 2005-2018 Thomas Bernard +# (c) 2005-2020 Thomas Bernard # to install use : # $ make DESTDIR=/tmp/dummylocation install # or @@ -77,12 +77,13 @@ SRCS = igd_desc_parse.c miniupnpc.c minixml.c minisoap.c miniwget.c \ upnperrors.c testigddescparse.c testminiwget.c \ connecthostport.c portlistingparse.c receivedata.c \ upnpdev.c testportlistingparse.c miniupnpcmodule.c \ - minihttptestserver.c \ + minihttptestserver.c addr_is_reserved.c testaddr_is_reserved.c \ listdevices.c LIBOBJS = miniwget.o minixml.o igd_desc_parse.o minisoap.o \ miniupnpc.o upnpreplyparse.o upnpcommands.o upnperrors.o \ - connecthostport.o portlistingparse.o receivedata.o upnpdev.o + connecthostport.o portlistingparse.o receivedata.o upnpdev.o \ + addr_is_reserved.o ifeq (, $(findstring amiga, $(OS))) ifeq (, $(findstring mingw, $(OS))$(findstring cygwin, $(OS))$(findstring msys, $(OS))) @@ -128,10 +129,12 @@ TESTUPNPREPLYPARSE = testupnpreplyparse.o minixml.o upnpreplyparse.o TESTPORTLISTINGPARSE = testportlistingparse.o minixml.o portlistingparse.o +TESTADDR_IS_RESERVED = testaddr_is_reserved.o addr_is_reserved.o + TESTIGDDESCPARSE = testigddescparse.o igd_desc_parse.o minixml.o \ miniupnpc.o miniwget.o upnpcommands.o upnpreplyparse.o \ minisoap.o connecthostport.o receivedata.o \ - portlistingparse.o + portlistingparse.o addr_is_reserved.o ifeq (, $(findstring amiga, $(OS))) EXECUTABLES := $(EXECUTABLES) upnpc-shared @@ -167,7 +170,7 @@ all: $(LIBRARY) $(EXECUTABLES) test: check check: validateminixml validateminiwget validateupnpreplyparse \ - validateportlistingparse validateigddescparse + validateportlistingparse validateigddescparse validateaddr_is_reserved everything: all $(EXECUTABLES_ADDTESTS) @@ -211,6 +214,11 @@ validateigddescparse: testigddescparse ./testigddescparse testdesc/linksys_WAG200G_desc.xml testdesc/linksys_WAG200G_desc.values touch $@ +validateaddr_is_reserved: testaddr_is_reserved + @echo "addr_is_reserved() validation test" + ./testaddr_is_reserved + touch $@ + clean: $(RM) $(LIBRARY) $(SHAREDLIBRARY) $(EXECUTABLES) $(OBJS) miniupnpcstrings.h $(RM) $(EXECUTABLES_ADDTESTS) @@ -321,6 +329,8 @@ testigddescparse: $(TESTIGDDESCPARSE) testportlistingparse: $(TESTPORTLISTINGPARSE) +testaddr_is_reserved: $(TESTADDR_IS_RESERVED) + miniupnpcstrings.h: miniupnpcstrings.h.in updateminiupnpcstrings.sh VERSION $(SH) updateminiupnpcstrings.sh diff --git a/miniupnpc/Makefile.mingw b/miniupnpc/Makefile.mingw index f59806a..9cd457e 100644 --- a/miniupnpc/Makefile.mingw +++ b/miniupnpc/Makefile.mingw @@ -26,7 +26,7 @@ OBJS=miniwget.o minixml.o igd_desc_parse.o minisoap.o \ minissdpc.o \ miniupnpc.o upnpreplyparse.o upnpcommands.o upnperrors.o \ connecthostport.o portlistingparse.o receivedata.o \ - upnpdev.o + upnpdev.o addr_is_reserved.o OBJSDLL=$(addprefix dll-, $(OBJS)) all: upnpc-static.exe upnpc-shared.exe testminixml.exe libminiupnpc.a \ @@ -95,7 +95,8 @@ miniwget.o: miniwget.c miniwget.h miniupnpcstrings.h connecthostport.h minisoap.o: minisoap.c minisoap.h miniupnpcstrings.h -miniupnpc.o: miniupnpc.c miniupnpc.h minisoap.h miniwget.h minixml.h +miniupnpc.o: miniupnpc.c miniupnpc.h minisoap.h miniwget.h minixml.h \ + addr_is_reserved.h igd_desc_parse.o: igd_desc_parse.c igd_desc_parse.h diff --git a/miniupnpc/addr_is_reserved.c b/miniupnpc/addr_is_reserved.c new file mode 100644 index 0000000..7adee9a --- /dev/null +++ b/miniupnpc/addr_is_reserved.c @@ -0,0 +1,71 @@ +/* $Id: $ */ +/* vim: tabstop=4 shiftwidth=4 noexpandtab + * Project : miniupnp + * Web : http://miniupnp.free.fr/ or https://miniupnp.tuxfamily.org/ + * Author : Thomas BERNARD + * copyright (c) 2005-2020 Thomas Bernard + * This software is subjet to the conditions detailed in the + * provided LICENSE file. */ +#ifdef _WIN32 +/* Win32 Specific includes and defines */ +#include +#include +#if !defined(_MSC_VER) +#include +#else /* !defined(_MSC_VER) */ +typedef unsigned long uint32_t; +#endif /* !defined(_MSC_VER) */ +#else /* _WIN32 */ +#include +#include +#include +#endif /* _WIN32 */ + +/* List of IP address blocks which are private / reserved and therefore not suitable for public external IP addresses */ +#define IP(a, b, c, d) (((a) << 24) + ((b) << 16) + ((c) << 8) + (d)) +#define MSK(m) (32-(m)) +static const struct { uint32_t address; uint32_t rmask; } reserved[] = { + { IP( 0, 0, 0, 0), MSK( 8) }, /* RFC1122 "This host on this network" */ + { IP( 10, 0, 0, 0), MSK( 8) }, /* RFC1918 Private-Use */ + { IP(100, 64, 0, 0), MSK(10) }, /* RFC6598 Shared Address Space */ + { IP(127, 0, 0, 0), MSK( 8) }, /* RFC1122 Loopback */ + { IP(169, 254, 0, 0), MSK(16) }, /* RFC3927 Link-Local */ + { IP(172, 16, 0, 0), MSK(12) }, /* RFC1918 Private-Use */ + { IP(192, 0, 0, 0), MSK(24) }, /* RFC6890 IETF Protocol Assignments */ + { IP(192, 0, 2, 0), MSK(24) }, /* RFC5737 Documentation (TEST-NET-1) */ + { IP(192, 31, 196, 0), MSK(24) }, /* RFC7535 AS112-v4 */ + { IP(192, 52, 193, 0), MSK(24) }, /* RFC7450 AMT */ + { IP(192, 88, 99, 0), MSK(24) }, /* RFC7526 6to4 Relay Anycast */ + { IP(192, 168, 0, 0), MSK(16) }, /* RFC1918 Private-Use */ + { IP(192, 175, 48, 0), MSK(24) }, /* RFC7534 Direct Delegation AS112 Service */ + { IP(198, 18, 0, 0), MSK(15) }, /* RFC2544 Benchmarking */ + { IP(198, 51, 100, 0), MSK(24) }, /* RFC5737 Documentation (TEST-NET-2) */ + { IP(203, 0, 113, 0), MSK(24) }, /* RFC5737 Documentation (TEST-NET-3) */ + { IP(224, 0, 0, 0), MSK( 4) }, /* RFC1112 Multicast */ + { IP(240, 0, 0, 0), MSK( 4) }, /* RFC1112 Reserved for Future Use + RFC919 Limited Broadcast */ +}; +#undef IP +#undef MSK + +/** + * @return 1 or 0 + */ +int addr_is_reserved(const char * addr_str) +{ + unsigned long addr_n; + uint32_t address; + size_t i; + + addr_n = inet_addr(addr_str); + if (addr_n == INADDR_NONE) + return 1; + + address = ntohl(addr_n); + + for (i = 0; i < sizeof(reserved)/sizeof(reserved[0]); ++i) { + if ((address >> reserved[i].rmask) == (reserved[i].address >> reserved[i].rmask)) + return 1; + } + + return 0; +} diff --git a/miniupnpc/addr_is_reserved.h b/miniupnpc/addr_is_reserved.h new file mode 100644 index 0000000..f8b5d66 --- /dev/null +++ b/miniupnpc/addr_is_reserved.h @@ -0,0 +1,14 @@ +/* $Id: $ */ +/* vim: tabstop=4 shiftwidth=4 noexpandtab + * Project: miniupnp + * http://miniupnp.free.fr/ or https://miniupnp.tuxfamily.org/ + * Author: Thomas Bernard + * Copyright (c) 2005-2020 Thomas Bernard + * This software is subjects to the conditions detailed + * in the LICENCE file provided within this distribution */ +#ifndef ADDR_IS_RESERVED_H_INCLUDED +#define ADDR_IS_RESERVED_H_INCLUDED + +int addr_is_reserved(const char * addr_str); + +#endif /* ADDR_IS_RESERVED_H_INCLUDED */ diff --git a/miniupnpc/miniupnpc.c b/miniupnpc/miniupnpc.c index cec8c4c..c702a85 100644 --- a/miniupnpc/miniupnpc.c +++ b/miniupnpc/miniupnpc.c @@ -1,9 +1,9 @@ /* $Id: miniupnpc.c,v 1.154 2019/04/23 12:12:13 nanard Exp $ */ /* vim: tabstop=4 shiftwidth=4 noexpandtab * Project : miniupnp - * Web : http://miniupnp.free.fr/ + * Web : http://miniupnp.free.fr/ or https://miniupnp.tuxfamily.org/ * Author : Thomas BERNARD - * copyright (c) 2005-2019 Thomas Bernard + * copyright (c) 2005-2020 Thomas Bernard * This software is subjet to the conditions detailed in the * provided LICENSE file. */ #include @@ -66,6 +66,7 @@ typedef unsigned long uint32_t; #include "minixml.h" #include "upnpcommands.h" #include "connecthostport.h" +#include "addr_is_reserved.h" /* compare the beginning of a string with a constant string */ #define COMPARE(str, cstr) (0==strncmp(str, cstr, sizeof(cstr) - 1)) @@ -78,52 +79,6 @@ typedef unsigned long uint32_t; #define SERVICEPREFIX "u" #define SERVICEPREFIX2 'u' -/* List of IP address blocks which are private / reserved and therefore not suitable for public external IP addresses */ -#define IP(a, b, c, d) (((a) << 24) + ((b) << 16) + ((c) << 8) + (d)) -#define MSK(m) (32-(m)) -static const struct { uint32_t address; uint32_t rmask; } reserved[] = { - { IP( 0, 0, 0, 0), MSK( 8) }, /* RFC1122 "This host on this network" */ - { IP( 10, 0, 0, 0), MSK( 8) }, /* RFC1918 Private-Use */ - { IP(100, 64, 0, 0), MSK(10) }, /* RFC6598 Shared Address Space */ - { IP(127, 0, 0, 0), MSK( 8) }, /* RFC1122 Loopback */ - { IP(169, 254, 0, 0), MSK(16) }, /* RFC3927 Link-Local */ - { IP(172, 16, 0, 0), MSK(12) }, /* RFC1918 Private-Use */ - { IP(192, 0, 0, 0), MSK(24) }, /* RFC6890 IETF Protocol Assignments */ - { IP(192, 0, 2, 0), MSK(24) }, /* RFC5737 Documentation (TEST-NET-1) */ - { IP(192, 31, 196, 0), MSK(24) }, /* RFC7535 AS112-v4 */ - { IP(192, 52, 193, 0), MSK(24) }, /* RFC7450 AMT */ - { IP(192, 88, 99, 0), MSK(24) }, /* RFC7526 6to4 Relay Anycast */ - { IP(192, 168, 0, 0), MSK(16) }, /* RFC1918 Private-Use */ - { IP(192, 175, 48, 0), MSK(24) }, /* RFC7534 Direct Delegation AS112 Service */ - { IP(198, 18, 0, 0), MSK(15) }, /* RFC2544 Benchmarking */ - { IP(198, 51, 100, 0), MSK(24) }, /* RFC5737 Documentation (TEST-NET-2) */ - { IP(203, 0, 113, 0), MSK(24) }, /* RFC5737 Documentation (TEST-NET-3) */ - { IP(224, 0, 0, 0), MSK( 4) }, /* RFC1112 Multicast */ - { IP(240, 0, 0, 0), MSK( 4) }, /* RFC1112 Reserved for Future Use + RFC919 Limited Broadcast */ -}; -#undef IP -#undef MSK - -static int addr_is_reserved(const char * addr_str) -{ - unsigned long addr_n; - uint32_t address; - size_t i; - - addr_n = inet_addr(addr_str); - if (addr_n == INADDR_NONE) - return 1; - - address = ntohl(addr_n); - - for (i = 0; i < sizeof(reserved)/sizeof(reserved[0]); ++i) { - if ((address >> reserved[i].rmask) == (reserved[i].address >> reserved[i].rmask)) - return 1; - } - - return 0; -} - /* root description parsing */ MINIUPNP_LIBSPEC void parserootdesc(const char * buffer, int bufsize, struct IGDdatas * data) { diff --git a/miniupnpc/testaddr_is_reserved.c b/miniupnpc/testaddr_is_reserved.c new file mode 100644 index 0000000..e30d947 --- /dev/null +++ b/miniupnpc/testaddr_is_reserved.c @@ -0,0 +1,46 @@ +/* $Id: $ */ +/* vim: tabstop=4 shiftwidth=4 noexpandtab + * Project : miniupnp + * Web : http://miniupnp.free.fr/ or https://miniupnp.tuxfamily.org/ + * Author : Thomas BERNARD + * copyright (c) 2005-2020 Thomas Bernard + * This software is subjet to the conditions detailed in the + * provided LICENSE file. */ +#include +#include "addr_is_reserved.h" + +static const struct { + const char * str; + int expected_result; +} tests[] = { +{ "0.0.0.0", 1 }, +{ "8.8.8.8", 0 }, +{ "192.168.1.1", 1 }, +{ "10.250.42.12", 1 }, +{ "11.250.42.12", 0 }, +{ "172.31.1.1", 1 }, +{ "172.32.1.1", 0 }, +{ "169.254.42.42", 1 }, +{ "192.0.0.11", 1 }, +{ "198.0.0.11", 0 }, +{ "198.18.0.11", 1 }, +{ "100.64.1.1", 1 }, +{ "100.127.1.1", 1 }, +{ "100.128.1.1", 0 }, +{ NULL, 0 } +}; + +int main(int argc, char * * argv) { + int i, result; + (void)argc; (void)argv; + + for (i = 0; tests[i].str != NULL; i++) { + result = addr_is_reserved(tests[i].str); + printf("testing %s %d %d\n", tests[i].str, tests[i].expected_result, result); + if (result != tests[i].expected_result) { + fprintf(stderr, "*** FAILURE ***\n"); + return 1; /* Failure */ + } + } + return 0; /* success */ +}