From 8839828ac9fe8409410e8df4d946dfe5afa0b85e Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Mon, 24 Aug 2020 21:03:31 +0200 Subject: [PATCH] net: don't accept non-left-contiguous netmasks A netmask that contains 1-bits after 0-bits (the 1-bits are not contiguous on the left side) is invalid [1] [2]. The code before this PR used to parse and accept such non-left-contiguous netmasks. However, a coming change that will alter `CNetAddr::ip` to have flexible size would make juggling with such netmasks more difficult, thus drop support for those. [1] https://en.wikipedia.org/wiki/Classless_Inter-Domain_Routing#Subnet_masks [2] https://tools.ietf.org/html/rfc4632#section-5.1 --- doc/release-notes.md | 7 +++ src/netaddress.cpp | 88 +++++++++++++++++--------------------- src/test/netbase_tests.cpp | 7 ++- 3 files changed, 52 insertions(+), 50 deletions(-) diff --git a/doc/release-notes.md b/doc/release-notes.md index e242b03df8de8..20f9357d0b223 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -533,6 +533,13 @@ The `autocombine` RPC command has been replaced with specific set/get commands ( } ``` +Updated settings +---------------- + +- Netmasks that contain 1-bits after 0-bits (the 1-bits are not contiguous on + the left side, e.g. 255.0.255.255) are no longer accepted. They are invalid + according to RFC 4632. + Build system changes -------------------- diff --git a/src/netaddress.cpp b/src/netaddress.cpp index 226af30dbd6fe..7c0628cafdcb8 100644 --- a/src/netaddress.cpp +++ b/src/netaddress.cpp @@ -724,9 +724,41 @@ CSubNet::CSubNet(const CNetAddr &addr, int32_t mask) network.ip[x] &= netmask[x]; } +/** + * @returns The number of 1-bits in the prefix of the specified subnet mask. If + * the specified subnet mask is not a valid one, -1. + */ +static inline int NetmaskBits(uint8_t x) +{ + switch(x) { + case 0x00: return 0; + case 0x80: return 1; + case 0xc0: return 2; + case 0xe0: return 3; + case 0xf0: return 4; + case 0xf8: return 5; + case 0xfc: return 6; + case 0xfe: return 7; + case 0xff: return 8; + default: return -1; + } +} + CSubNet::CSubNet(const CNetAddr &addr, const CNetAddr &mask) { valid = true; + // Check if `mask` contains 1-bits after 0-bits (which is an invalid netmask). + bool zeros_found = false; + for (size_t i = mask.IsIPv4() ? 12 : 0; i < sizeof(mask.ip); ++i) { + const int num_bits = NetmaskBits(mask.ip[i]); + if (num_bits == -1 || (zeros_found && num_bits != 0)) { + valid = false; + return; + } + if (num_bits < 8) { + zeros_found = true; + } + } network = addr; // Default to /32 (IPv4) or /128 (IPv6), i.e. match single address memset(netmask, 255, sizeof(netmask)); @@ -759,58 +791,18 @@ bool CSubNet::Match(const CNetAddr &addr) const return true; } -static inline int NetmaskBits(uint8_t x) -{ - switch(x) { - case 0x00: return 0; break; - case 0x80: return 1; break; - case 0xc0: return 2; break; - case 0xe0: return 3; break; - case 0xf0: return 4; break; - case 0xf8: return 5; break; - case 0xfc: return 6; break; - case 0xfe: return 7; break; - case 0xff: return 8; break; - default: return -1; break; - } -} - std::string CSubNet::ToString() const { - /* Parse binary 1{n}0{N-n} to see if mask can be represented as /n */ - int cidr = 0; - bool valid_cidr = true; - int n = network.IsIPv4() ? 12 : 0; - for (; n < 16 && netmask[n] == 0xff; ++n) - cidr += 8; - if (n < 16) { - int bits = NetmaskBits(netmask[n]); - if (bits < 0) - valid_cidr = false; - else - cidr += bits; - ++n; - } - for (; n < 16 && valid_cidr; ++n) - if (netmask[n] != 0x00) - valid_cidr = false; - - /* Format output */ - std::string strNetmask; - if (valid_cidr) { - strNetmask = strprintf("%u", cidr); - } else { - if (network.IsIPv4()) - strNetmask = strprintf("%u.%u.%u.%u", netmask[12], netmask[13], netmask[14], netmask[15]); - else - strNetmask = strprintf("%x:%x:%x:%x:%x:%x:%x:%x", - netmask[0] << 8 | netmask[1], netmask[2] << 8 | netmask[3], - netmask[4] << 8 | netmask[5], netmask[6] << 8 | netmask[7], - netmask[8] << 8 | netmask[9], netmask[10] << 8 | netmask[11], - netmask[12] << 8 | netmask[13], netmask[14] << 8 | netmask[15]); + uint8_t cidr = 0; + + for (size_t i = network.IsIPv4() ? 12 : 0; i < sizeof(netmask); ++i) { + if (netmask[i] == 0x00) { + break; + } + cidr += NetmaskBits(netmask[i]); } - return network.ToString() + "/" + strNetmask; + return network.ToString() + strprintf("/%u", cidr); } bool CSubNet::IsValid() const diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp index eb03d24316b0f..9c12765fc151c 100644 --- a/src/test/netbase_tests.cpp +++ b/src/test/netbase_tests.cpp @@ -287,10 +287,13 @@ BOOST_AUTO_TEST_CASE(subnet_test) BOOST_CHECK_EQUAL(subnet.ToString(), "1::/16"); subnet = ResolveSubNet("1:2:3:4:5:6:7:8/0000:0000:0000:0000:0000:0000:0000:0000"); BOOST_CHECK_EQUAL(subnet.ToString(), "::/0"); + // Invalid netmasks (with 1-bits after 0-bits) subnet = ResolveSubNet("1.2.3.4/255.255.232.0"); - BOOST_CHECK_EQUAL(subnet.ToString(), "1.2.0.0/255.255.232.0"); + BOOST_CHECK(!subnet.IsValid()); + subnet = ResolveSubNet("1.2.3.4/255.0.255.255"); + BOOST_CHECK(!subnet.IsValid()); subnet = ResolveSubNet("1:2:3:4:5:6:7:8/ffff:ffff:ffff:fffe:ffff:ffff:ffff:ff0f"); - BOOST_CHECK_EQUAL(subnet.ToString(), "1:2:3:4:5:6:7:8/ffff:ffff:ffff:fffe:ffff:ffff:ffff:ff0f"); + BOOST_CHECK(!subnet.IsValid()); } BOOST_AUTO_TEST_CASE(validate_test)