From db7e1ef162f35b2858740b51ee54d3ebf4df8238 Mon Sep 17 00:00:00 2001 From: Donal Cahill Date: Sun, 13 Dec 2015 15:36:35 +0000 Subject: [PATCH] Fix up CIDR code. --- src/User.cpp | 31 +++++++++++++++++++++++++------ test/UserTest.cpp | 26 ++++++++++++++++++++++---- 2 files changed, 47 insertions(+), 10 deletions(-) diff --git a/src/User.cpp b/src/User.cpp index 300297ea..0b421487 100644 --- a/src/User.cpp +++ b/src/User.cpp @@ -857,10 +857,23 @@ bool CUser::IsHostAllowed(const CString& sHostMask) const { // Try to split the string into an IP and routing prefix VCString vsSplitCIDR; - const int iSplitCIDRCount = sHost.Split("/", vsSplitCIDR, false); - const int iRoutingPrefix = vsSplitCIDR.back().ToInt(); + if (sHost.Split("/", vsSplitCIDR, false) != 2) continue; + const CString sRoutingPrefix = vsSplitCIDR.back(); + const int iRoutingPrefix = sRoutingPrefix.ToInt(); + if (iRoutingPrefix < 0 || iRoutingPrefix > 128) continue; - if (iSplitCIDRCount != 2 || iRoutingPrefix < 0) continue; + // If iRoutingPrefix is 0, it could be due to ToInt() failing, so + // sRoutingPrefix needs to be all zeroes + if (iRoutingPrefix == 0) { + bool bBadPrefix = false; + for (const char c : sRoutingPrefix) { + if (!CString(c).Equals("0")) { + bBadPrefix = true; + break; + } + } + if (bBadPrefix) continue; + } // Convert each IP from a numeric string to an addrinfo addrinfo aiHints; @@ -869,15 +882,15 @@ bool CUser::IsHostAllowed(const CString& sHostMask) const { addrinfo* aiHost; int iIsHostValid = - getaddrinfo(sHostMask.c_str(), NULL, &aiHints, &aiHost); + getaddrinfo(sHostMask.c_str(), nullptr, &aiHints, &aiHost); if (iIsHostValid != 0) continue; aiHints.ai_family = aiHost->ai_family; // Host and range must be in // the same address family addrinfo* aiRange; - int iIsRangeValid = getaddrinfo(vsSplitCIDR.front().c_str(), NULL, - &aiHints, &aiRange); + int iIsRangeValid = getaddrinfo(vsSplitCIDR.front().c_str(), + nullptr, &aiHints, &aiRange); if (iIsRangeValid != 0) { freeaddrinfo(aiHost); continue; @@ -895,6 +908,12 @@ bool CUser::IsHostAllowed(const CString& sHostMask) const { // they match bool bIsHostInRange = false; if (aiHost->ai_family == AF_INET) { + if (iRoutingPrefix > 32) { + freeaddrinfo(aiHost); + freeaddrinfo(aiRange); + continue; + } + const sockaddr_in* saHost = (sockaddr_in*)(aiHost->ai_addr); const sockaddr_in* saRange = (sockaddr_in*)(aiRange->ai_addr); diff --git a/test/UserTest.cpp b/test/UserTest.cpp index e8b197f9..8af79f62 100644 --- a/test/UserTest.cpp +++ b/test/UserTest.cpp @@ -20,6 +20,7 @@ class UserTest : public ::testing::Test { protected: + // A CZNC instance is required to instantiate CUsers UserTest() { CZNC::CreateInstance(); } ~UserTest() { CZNC::DestroyInstance(); } }; @@ -90,12 +91,29 @@ TEST_F(UserTest, IsHostAllowed) { {"2001:db8::/33", "2001:db9:0::", false}, {"2001:db8::/32", "2001:db8:8000::", true}, {"2001:db8::/33", "2001:db8:8000::", false}, + + {"0.0.0.0/0", "::1", false}, + {"0.0.0.0/0", "127.0.0.1", true}, + {"::0/0", "127.0.0.1", false}, + {"::0/0", "::1", true}, + {"0.0.0.0", "127.0.0.1", false}, + {"0.0.0.0", "::1", false}, + {"::0", "::1", false}, + {"::0", "127.0.0.1", false}, + + {"127.0.0.2/abc", "127.0.0.1", false}, + {"::2/abc", "::1", false}, + {"127.0.0.1/33", "127.0.0.1", false}, + {"::1/129", "::1", false}, + + {"::2/00000000000", "::1", true}, + {"::2/0a", "::1", false}, }; for (const hostTest& h : aHostTests) { - CUser pTestUser("user"); - pTestUser.AddAllowedHost(h.sTestHost); - EXPECT_EQ(h.bExpectedResult, pTestUser.IsHostAllowed(h.sIP)) - << "Allow-host is " << h.sTestHost; + CUser user("user"); + user.AddAllowedHost(h.sTestHost); + EXPECT_EQ(h.bExpectedResult, user.IsHostAllowed(h.sIP)) + << "Allow-host is " << h.sTestHost; } }