From: Francis Dupont <fdupont@isc.org>
Date: Thu, 12 Jan 2017 12:12:51 +0000 (+0100)
Subject: [5097] Added a check and unit tests against prefix length truncation
X-Git-Tag: trac5119_base~2^2~3
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1af539811e1f72b05e19d047796ae42d96eac8d9;p=thirdparty%2Fkea.git

[5097] Added a check and unit tests against prefix length truncation
---

diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc
index c59efc6d09..9ba58a4b7b 100644
--- a/src/bin/dhcp4/tests/config_parser_unittest.cc
+++ b/src/bin/dhcp4/tests/config_parser_unittest.cc
@@ -1524,6 +1524,15 @@ TEST_F(Dhcp4ParserTest, badPools) {
         "    \"subnet\": \"192.0.2.0/24\" } ],"
         "\"valid-lifetime\": 4000 }";
 
+    // out of range prefix length (new check)
+    string config_bogus7 = "{ " + genIfaceConfig() + "," +
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"subnet4\": [ { "
+        "    \"pools\": [ { \"pool\": \"192.0.2.128/1052\" } ],"
+        "    \"subnet\": \"192.0.2.0/24\" } ],"
+        "\"valid-lifetime\": 4000 }";
+
     ConstElementPtr json1;
     ASSERT_NO_THROW(json1 = parseDHCP4(config_bogus1));
     ConstElementPtr json2;
@@ -1536,6 +1545,8 @@ TEST_F(Dhcp4ParserTest, badPools) {
     ASSERT_NO_THROW(json5 = parseDHCP4(config_bogus5));
     ConstElementPtr json6;
     ASSERT_NO_THROW(json6 = parseDHCP4(config_bogus6));
+    ConstElementPtr json7;
+    ASSERT_NO_THROW(json7 = parseDHCP4(config_bogus7));
 
     ConstElementPtr status;
     EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json1));
@@ -1573,6 +1584,12 @@ TEST_F(Dhcp4ParserTest, badPools) {
     EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json6));
     checkResult(status, 1);
     EXPECT_TRUE(errorContainsPosition(status, "<string>"));
+
+    CfgMgr::instance().clear();
+
+    EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json7));
+    checkResult(status, 1);
+    EXPECT_TRUE(errorContainsPosition(status, "<string>"));
 }
 
 // The goal of this test is to check whether an option definition
diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc
index 7f121d8ff8..382fc07d8b 100644
--- a/src/bin/dhcp6/tests/config_parser_unittest.cc
+++ b/src/bin/dhcp6/tests/config_parser_unittest.cc
@@ -1572,7 +1572,17 @@ TEST_F(Dhcp6ParserTest, badPools) {
         "\"rebind-timer\": 2000, "
         "\"renew-timer\": 1000, "
         "\"subnet6\": [ { "
-        "    \"pools\": [ { \"pool\": \"2001:db8::ff:ffff - 2001:db8::\" } ],"
+        "    \"pools\": [ { \"pool\": \"2001:db8:1::ffff - 2001:db8:1::\" } ],"
+        "    \"subnet\": \"2001:db8:1::/64\" } ],"
+        "\"valid-lifetime\": 4000 }";
+
+    // out of range prefix length (new check)
+    string config_bogus7 = "{ " + genIfaceConfig() + ","
+        "\"preferred-lifetime\": 3000,"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"subnet6\": [ { "
+        "    \"pools\": [ { \"pool\": \"2001:db8:1::/1104\" } ],"
         "    \"subnet\": \"2001:db8:1::/64\" } ],"
         "\"valid-lifetime\": 4000 }";
 
@@ -1588,6 +1598,8 @@ TEST_F(Dhcp6ParserTest, badPools) {
     ASSERT_NO_THROW(json5 = parseDHCP6(config_bogus5));
     ConstElementPtr json6;
     ASSERT_NO_THROW(json6 = parseDHCP6(config_bogus6));
+    ConstElementPtr json7;
+    ASSERT_NO_THROW(json7 = parseDHCP6(config_bogus7));
 
     ConstElementPtr status;
     EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json1));
@@ -1625,6 +1637,12 @@ TEST_F(Dhcp6ParserTest, badPools) {
     EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json6));
     checkResult(status, 1);
     EXPECT_TRUE(errorContainsPosition(status, "<string>"));
+
+    CfgMgr::instance().clear();
+
+    EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json7));
+    checkResult(status, 1);
+    EXPECT_TRUE(errorContainsPosition(status, "<string>"));
 }
 
 // Goal of this test is to verify the basic parsing of a prefix delegation
diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc
index 1e85b32548..8c2e09c034 100644
--- a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc
+++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc
@@ -900,7 +900,12 @@ PoolParser::parse(ConstElementPtr pool_structure,
 
             // No checks for values over 128. Range correctness will
             // be checked in Pool4 constructor.
-            len = boost::lexical_cast<int>(prefix_len);
+            int val_len = boost::lexical_cast<int>(prefix_len);
+            if ((val_len < std::numeric_limits<uint8_t>::min()) ||
+                (val_len > std::numeric_limits<uint8_t>::max())) {
+                isc_throw(OutOfRange, "");
+            }
+            len = static_cast<uint8_t>(val_len);
         } catch (...)  {
             isc_throw(DhcpConfigError, "Failed to parse pool "
                       "definition: " << txt << " ("