Skip to content

Commit 1ea57ad

Browse files
vasildvasild
committed
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
1 parent cb1ee15 commit 1ea57ad

File tree

3 files changed

+49
-55
lines changed

3 files changed

+49
-55
lines changed

‎doc/release-notes.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,10 @@ Updated settings
135135
in future releases. Refer to the help of the affected settings `-whitebind`
136136
and `-whitelist` for more details. (#19191)
137137

138+
- Netmasks that contain 1-bits after 0-bits (the 1-bits are not contiguous on
139+
the left side, e.g. 255.0.255.255) are no longer accepted. They are invalid
140+
according to RFC 4632.
141+
138142
Changes to Wallet or GUI related settings can be found in the GUI or Wallet section below.
139143

140144
Tools and Utilities

‎src/netaddress.cpp‎

Lines changed: 40 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -789,9 +789,41 @@ CSubNet::CSubNet(const CNetAddr &addr, int32_t mask)
789789
network.ip[x] &= netmask[x];
790790
}
791791

792+
/**
793+
* @returns The number of 1-bits in the prefix of the specified subnet mask. If
794+
* the specified subnet mask is not a valid one, -1.
795+
*/
796+
static inline int NetmaskBits(uint8_t x)
797+
{
798+
switch(x) {
799+
case 0x00: return 0;
800+
case 0x80: return 1;
801+
case 0xc0: return 2;
802+
case 0xe0: return 3;
803+
case 0xf0: return 4;
804+
case 0xf8: return 5;
805+
case 0xfc: return 6;
806+
case 0xfe: return 7;
807+
case 0xff: return 8;
808+
default: return -1;
809+
}
810+
}
811+
792812
CSubNet::CSubNet(const CNetAddr &addr, const CNetAddr &mask)
793813
{
794814
valid = true;
815+
// Check if `mask` contains 1-bits after 0-bits (which is an invalid netmask).
816+
bool zeros_found = false;
817+
for (size_t i = mask.IsIPv4() ? 12 : 0; i < sizeof(mask.ip); ++i) {
818+
const int num_bits = NetmaskBits(mask.ip[i]);
819+
if (num_bits == -1 || (zeros_found && num_bits != 0)) {
820+
valid = false;
821+
return;
822+
}
823+
if (num_bits < 8) {
824+
zeros_found = true;
825+
}
826+
}
795827
network = addr;
796828
// Default to /32 (IPv4) or /128 (IPv6), i.e. match single address
797829
memset(netmask, 255, sizeof(netmask));
@@ -828,62 +860,18 @@ bool CSubNet::Match(const CNetAddr &addr) const
828860
return true;
829861
}
830862

831-
/**
832-
* @returns The number of 1-bits in the prefix of the specified subnet mask. If
833-
* the specified subnet mask is not a valid one, -1.
834-
*/
835-
static inline int NetmaskBits(uint8_t x)
836-
{
837-
switch(x) {
838-
case 0x00: return 0;
839-
case 0x80: return 1;
840-
case 0xc0: return 2;
841-
case 0xe0: return 3;
842-
case 0xf0: return 4;
843-
case 0xf8: return 5;
844-
case 0xfc: return 6;
845-
case 0xfe: return 7;
846-
case 0xff: return 8;
847-
default: return -1;
848-
}
849-
}
850-
851863
std::string CSubNet::ToString() const
852864
{
853-
/* Parse binary 1{n}0{N-n} to see if mask can be represented as /n */
854-
int cidr = 0;
855-
bool valid_cidr = true;
856-
int n = network.IsIPv4() ? 12 : 0;
857-
for (; n < 16 && netmask[n] == 0xff; ++n)
858-
cidr += 8;
859-
if (n < 16) {
860-
int bits = NetmaskBits(netmask[n]);
861-
if (bits < 0)
862-
valid_cidr = false;
863-
else
864-
cidr += bits;
865-
++n;
866-
}
867-
for (; n < 16 && valid_cidr; ++n)
868-
if (netmask[n] != 0x00)
869-
valid_cidr = false;
870-
871-
/* Format output */
872-
std::string strNetmask;
873-
if (valid_cidr) {
874-
strNetmask = strprintf("%u", cidr);
875-
} else {
876-
if (network.IsIPv4())
877-
strNetmask = strprintf("%u.%u.%u.%u", netmask[12], netmask[13], netmask[14], netmask[15]);
878-
else
879-
strNetmask = strprintf("%x:%x:%x:%x:%x:%x:%x:%x",
880-
netmask[0] << 8 | netmask[1], netmask[2] << 8 | netmask[3],
881-
netmask[4] << 8 | netmask[5], netmask[6] << 8 | netmask[7],
882-
netmask[8] << 8 | netmask[9], netmask[10] << 8 | netmask[11],
883-
netmask[12] << 8 | netmask[13], netmask[14] << 8 | netmask[15]);
865+
uint8_t cidr = 0;
866+
867+
for (size_t i = network.IsIPv4() ? 12 : 0; i < sizeof(netmask); ++i) {
868+
if (netmask[i] == 0x00) {
869+
break;
870+
}
871+
cidr += NetmaskBits(netmask[i]);
884872
}
885873

886-
return network.ToString() + "/" + strNetmask;
874+
return network.ToString() + strprintf("/%u", cidr);
887875
}
888876

889877
bool CSubNet::IsValid() const

‎src/test/netbase_tests.cpp‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -290,11 +290,13 @@ BOOST_AUTO_TEST_CASE(subnet_test)
290290
BOOST_CHECK_EQUAL(subnet.ToString(), "1::/16");
291291
subnet = ResolveSubNet("1:2:3:4:5:6:7:8/0000:0000:0000:0000:0000:0000:0000:0000");
292292
BOOST_CHECK_EQUAL(subnet.ToString(), "::/0");
293+
// Invalid netmasks (with 1-bits after 0-bits)
293294
subnet = ResolveSubNet("1.2.3.4/255.255.232.0");
294-
BOOST_CHECK_EQUAL(subnet.ToString(), "1.2.0.0/255.255.232.0");
295+
BOOST_CHECK(!subnet.IsValid());
296+
subnet = ResolveSubNet("1.2.3.4/255.0.255.255");
297+
BOOST_CHECK(!subnet.IsValid());
295298
subnet = ResolveSubNet("1:2:3:4:5:6:7:8/ffff:ffff:ffff:fffe:ffff:ffff:ffff:ff0f");
296-
BOOST_CHECK_EQUAL(subnet.ToString(), "1:2:3:4:5:6:7:8/ffff:ffff:ffff:fffe:ffff:ffff:ffff:ff0f");
297-
299+
BOOST_CHECK(!subnet.IsValid());
298300
}
299301

300302
BOOST_AUTO_TEST_CASE(netbase_getgroup)

0 commit comments

Comments
 (0)