From: Razvan Becheriu Date: Tue, 14 Jul 2026 17:30:20 +0000 (+0300) Subject: [#2260] check page size limits X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=07712baefe29342748c58b4c8163b088ec204375;p=thirdparty%2Fkea.git [#2260] check page size limits --- diff --git a/src/bin/keactrl/tests/keactrl_tests.sh.in b/src/bin/keactrl/tests/keactrl_tests.sh.in index dac42232e6..83315ef329 100755 --- a/src/bin/keactrl/tests/keactrl_tests.sh.in +++ b/src/bin/keactrl/tests/keactrl_tests.sh.in @@ -645,7 +645,7 @@ Expected wait_for_message return %d, returned %d." # servers are not started. start_v6_server_test() { # Create configuration file for keactrl. This configuration enables - # DHCPv6 server but disables other servers.. + # DHCPv6 server but disables other servers. keactrl_config="${CFG_FILES}\ndhcp4=no\ndhcp6=yes\n\ dhcp_ddns=no\nnetconf=no\n\ kea_verbose=no\n${keactrl_fixed_config}" diff --git a/src/hooks/dhcp/lease_cmds/lease_cmds.cc b/src/hooks/dhcp/lease_cmds/lease_cmds.cc index 150edd12c9..57cb211c7e 100644 --- a/src/hooks/dhcp/lease_cmds/lease_cmds.cc +++ b/src/hooks/dhcp/lease_cmds/lease_cmds.cc @@ -1071,8 +1071,17 @@ LeaseCmdsImpl::leaseGetPageHandler(CalloutHandle& handle) { isc_throw(BadValue, "'limit' parameter must be a number"); } + auto tmp128 = page_limit->intValue(); + if ((tmp128 < uint32_min) || (tmp128 > uint32_max)) { + isc_throw(BadValue, "'limit' parameter is not a 32 bit unsigned integer."); + } + + if (tmp128 == 0) { + isc_throw(BadValue, "'limit' parameter must not be 0."); + } + // Retrieve the desired page size. - size_t page_limit_value = static_cast(page_limit->intValue()); + size_t page_limit_value = static_cast(tmp128); ElementPtr leases_json = Element::createList(); diff --git a/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds4_unittest.cc b/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds4_unittest.cc index 14573233e1..4674b90643 100644 --- a/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds4_unittest.cc +++ b/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds4_unittest.cc @@ -273,6 +273,9 @@ public: /// @brief Verifies that the limit of 0 is rejected. void testLease4GetPagedLimitIsZero(); + /// @brief Verifies that the limit of negative value is rejected. + void testLease4GetPagedLimitIsNegative(); + /// @brief Check that lease4-get-by-hw-address can handle a situation when /// the query is broken (required parameter is missing). void testLease4GetByHwAddressParams(); @@ -1699,7 +1702,7 @@ void Lease4CmdsTest::testLease4GetBySubnetIdInvalidArguments() { exp_rsp = "listed subnet identifiers must be numbers"; testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp); - // Subnets list must contain positive numbers.. + // Subnets list must contain positive numbers. cmd = "{\n" " \"command\": \"lease4-get-all\",\n" @@ -1710,7 +1713,7 @@ void Lease4CmdsTest::testLease4GetBySubnetIdInvalidArguments() { exp_rsp = "out of range subnet identifier -1"; testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp); - // Subnets list must contain 32 bit unsigned integers.. + // Subnets list must contain 32 bit unsigned integers. cmd = "{\n" " \"command\": \"lease4-get-all\",\n" @@ -1920,7 +1923,25 @@ void Lease4CmdsTest::testLease4GetPagedLimitIsZero() { " }" "}"; - string exp_rsp = "page size of retrieved leases must not be 0"; + string exp_rsp = "'limit' parameter must not be 0."; + testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp); +} + +void Lease4CmdsTest::testLease4GetPagedLimitIsNegative() { + // Initialize lease manager (false = v6, true = add leases) + initLeaseMgr(false, true); + + // Query for a page of leases. + string cmd = + "{\n" + " \"command\": \"lease4-get-page\",\n" + " \"arguments\": {" + " \"from\": \"start\"," + " \"limit\": -1" + " }" + "}"; + + string exp_rsp = "'limit' parameter is not a 32 bit unsigned integer."; testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp); } @@ -2295,7 +2316,7 @@ void Lease4CmdsTest::testLease4GetByStateBadArgs() { "'subnet-id' parameter must be a 32 bit unsigned integer" }, { - "Subnet id must be fit into 32 bits", + "Subnet id must fit into 32 bits", "\"subnet-id\": 4294967297", "'subnet-id' parameter must be a 32 bit unsigned integer" } @@ -4396,6 +4417,15 @@ TEST_F(Lease4CmdsTest, lease4GetPagedLimitIsZeroMultiThreading) { testLease4GetPagedLimitIsZero(); } +TEST_F(Lease4CmdsTest, lease4GetPagedLimitIsNegative) { + testLease4GetPagedLimitIsNegative(); +} + +TEST_F(Lease4CmdsTest, lease4GetPagedLimitIsNegativeMultiThreading) { + MultiThreadingTest mt(true); + testLease4GetPagedLimitIsNegative(); +} + TEST_F(Lease4CmdsTest, lease4GetByHwAddressParams) { testLease4GetByHwAddressParams(); } diff --git a/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds6_unittest.cc b/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds6_unittest.cc index f7f57c6f18..f3091d928f 100644 --- a/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds6_unittest.cc +++ b/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds6_unittest.cc @@ -302,6 +302,9 @@ public: /// @brief Verifies that the limit of 0 is rejected. void testLease6GetPagedLimitIsZero(); + /// @brief Verifies that the limit of negative value is rejected. + void testLease6GetPagedLimitIsNegative(); + /// @brief Check that lease6-get-by-hw-address can handle a situation when /// the query is broken (required parameter is missing). void testLease6GetByHwAddressParams(); @@ -1975,7 +1978,7 @@ void Lease6CmdsTest::testLease6GetBySubnetIdInvalidArguments() { exp_rsp = "listed subnet identifiers must be numbers"; testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp); - // Subnets list must contain positive numbers.. + // Subnets list must contain positive numbers. cmd = "{\n" " \"command\": \"lease6-get-all\",\n" @@ -1986,7 +1989,7 @@ void Lease6CmdsTest::testLease6GetBySubnetIdInvalidArguments() { exp_rsp = "out of range subnet identifier -1"; testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp); - // Subnets list must contain 32 bit unsigned integers.. + // Subnets list must contain 32 bit unsigned integers. cmd = "{\n" " \"command\": \"lease6-get-all\",\n" @@ -2197,7 +2200,25 @@ void Lease6CmdsTest::testLease6GetPagedLimitIsZero() { " }" "}"; - string exp_rsp = "page size of retrieved leases must not be 0"; + string exp_rsp = "'limit' parameter must not be 0."; + testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp); +} + +void Lease6CmdsTest::testLease6GetPagedLimitIsNegative() { + // Initialize lease manager (true = v6, true = add leases) + initLeaseMgr(true, true); + + // Query for a page of leases. + string cmd = + "{\n" + " \"command\": \"lease6-get-page\",\n" + " \"arguments\": {" + " \"from\": \"start\"," + " \"limit\": -1" + " }" + "}"; + + string exp_rsp = "'limit' parameter is not a 32 bit unsigned integer."; testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp); } @@ -2577,7 +2598,7 @@ void Lease6CmdsTest::testLease6GetByStateBadArgs() { "'subnet-id' parameter must be a 32 bit unsigned integer" }, { - "Subnet id must be fit into 32 bits", + "Subnet id must fit into 32 bits", "\"subnet-id\": 4294967297", "'subnet-id' parameter must be a 32 bit unsigned integer" } @@ -5470,6 +5491,15 @@ TEST_F(Lease6CmdsTest, lease6GetPagedLimitIsZeroMultiThreading) { testLease6GetPagedLimitIsZero(); } +TEST_F(Lease6CmdsTest, lease6GetPagedLimitIsNegative) { + testLease6GetPagedLimitIsNegative(); +} + +TEST_F(Lease6CmdsTest, lease6GetPagedLimitIsNegativeMultiThreading) { + MultiThreadingTest mt(true); + testLease6GetPagedLimitIsNegative(); +} + TEST_F(Lease6CmdsTest, lease6GetByHwAddressParams) { testLease6GetByHwAddressParams(); } diff --git a/src/lib/dhcpsrv/dhcpsrv_messages.cc b/src/lib/dhcpsrv/dhcpsrv_messages.cc index 8b04b3bbf0..5f15a05fe9 100644 --- a/src/lib/dhcpsrv/dhcpsrv_messages.cc +++ b/src/lib/dhcpsrv/dhcpsrv_messages.cc @@ -231,7 +231,7 @@ const char* values[] = { "DHCPSRV_CFGMGR_USE_ADDRESS", "listening on address %1, on interface %2", "DHCPSRV_CFGMGR_USE_ALLOCATOR", "using the %1 allocator for %2 leases in subnet %3", "DHCPSRV_CFGMGR_USE_UNICAST", "listening on unicast address %1, on interface %2", - "DHCPSRV_CLASS_BAD_NAME", "class name '%1' includes problematic characters: its escaped form is '%2'", + "DHCPSRV_CLASS_BAD_NAME", "class name '%1' includes problematic characters: suggest to use '%2' instead", "DHCPSRV_CLASS_WITH_ADDITIONAL_AND_LIFETIMES", "class: %1 has 'only-in-additional-list' true while specifying one or more lease life time values. Life time values will be ignored.", "DHCPSRV_CLIENT_CLASS_DEPRECATED", "The parameter 'client-class' is deprecated. Use 'client-classes' list parameter instead", "DHCPSRV_CLOSE_DB", "closing currently open %1 database", diff --git a/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc b/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc index be29d5a048..ca285ec2b7 100644 --- a/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc +++ b/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc @@ -604,7 +604,7 @@ TEST_F(ClientClassDefParserTest, escapedName) { // A warning should be in logs. std::string warn = "DHCPSRV_CLASS_BAD_NAME class name 'foo bar' "; warn += "includes problematic characters: "; - warn += "its escaped form is 'foo%20bar'"; + warn += "suggest to use 'foo%20bar' instead"; EXPECT_EQ(1U, countFile(warn)); } @@ -626,7 +626,7 @@ TEST_F(ClientClassDefParserTest, escapedName2) { // A warning should not be in logs. std::string warn = "DHCPSRV_CLASS_BAD_NAME class name 'foo%bar' "; warn += "includes problematic characters: "; - warn += "its escaped form is 'foo%%bar'"; + warn += "suggest to use 'foo%%bar' instead"; EXPECT_EQ(0U, countFile(warn)); } diff --git a/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.cc b/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.cc index ac7f49d9d4..2fc7450b79 100644 --- a/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.cc @@ -1332,7 +1332,7 @@ GenericHostDataSourceTest::testGet4ByIdentifier( HostPtr host1 = HostDataSourceUtils::initializeHost4("192.0.2.1", identifier_type); HostPtr host2 = HostDataSourceUtils::initializeHost4("192.0.2.2", identifier_type); - // Sanity check: make sure the hosts have different identifiers.. + // Sanity check: make sure the hosts have different identifiers. ASSERT_FALSE(host1->getIdentifier() == host2->getIdentifier()); // Try to add both of them to the host data source.