From: Francis Dupont Date: Tue, 30 Jun 2026 17:46:46 +0000 (+0200) Subject: [#2260] Addressed third item X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=72ab74f2af764fcf1d76fa593f838b8ca7a37e40;p=thirdparty%2Fkea.git [#2260] Addressed third item --- diff --git a/src/hooks/dhcp/lease_cmds/lease_cmds.cc b/src/hooks/dhcp/lease_cmds/lease_cmds.cc index f43a1d70d1..f4552ebcb9 100644 --- a/src/hooks/dhcp/lease_cmds/lease_cmds.cc +++ b/src/hooks/dhcp/lease_cmds/lease_cmds.cc @@ -33,6 +33,7 @@ #include #include +#include #include #include @@ -752,7 +753,12 @@ LeaseCmdsImpl::getParameters(bool v6, const ConstElementPtr& params) { if (tmp->getType() != Element::integer) { isc_throw(BadValue, "'subnet-id' parameter is not integer."); } - x.subnet_id = tmp->intValue(); + int64_t tmp64 = tmp->intValue(); + if ((tmp64 < numeric_limits::min()) || + (tmp64 > numeric_limits::max())) { + isc_throw(BadValue, "'subnet-id' parameter is not a 32 bit unsigned integer."); + } + x.subnet_id = static_cast(tmp64); if (params->contains("iaid")) { x.iaid = params->get("iaid")->intValue(); @@ -923,17 +929,23 @@ LeaseCmdsImpl::leaseGetAllHandler(CalloutHandle& handle) { if (subnet_id->getType() != Element::integer) { isc_throw(BadValue, "listed subnet identifiers must be numbers"); } + auto subnet_id_ = subnet_id->intValue(); + if ((subnet_id_ < numeric_limits::min()) || + (subnet_id_ > numeric_limits::max())) { + isc_throw(BadValue, "out of range subnet identifier " + << subnet_id_); + } if (v4) { Lease4Collection leases = - LeaseMgrFactory::instance().getLeases4(subnet_id->intValue()); + LeaseMgrFactory::instance().getLeases4(subnet_id_); for (auto const& lease : leases) { ElementPtr lease_json = lease->toElement(); leases_json->add(lease_json); } } else { Lease6Collection leases = - LeaseMgrFactory::instance().getLeases6(subnet_id->intValue()); + LeaseMgrFactory::instance().getLeases6(subnet_id_); for (auto const& lease : leases) { ElementPtr lease_json = lease->toElement(); leases_json->add(lease_json); @@ -1316,7 +1328,12 @@ LeaseCmdsImpl::leaseGetByStateHandler(CalloutHandle& handle) { if (subnet->getType() != Element::integer) { isc_throw(BadValue, "'subnet-id' parameter must be a number"); } - subnet_id_ = subnet->intValue(); + auto id64 = subnet->intValue(); + if ((id64 < numeric_limits::min()) || + (id64 > numeric_limits::max())) { + isc_throw(BadValue, "'subnet-id' parameter must be a 32 bit unsigned integer"); + } + subnet_id_ = static_cast(id64); } 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 680b697265..40c12c0fe1 100644 --- a/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds4_unittest.cc +++ b/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds4_unittest.cc @@ -313,6 +313,10 @@ public: /// combinations). void testLease4GetByStateFindN(); + /// @brief Check that lease4-get-by-state raises an error when + /// the argument is bad. + void testLease4GetByStateBadArgs(); + /// @brief Check that lease4-get-by-hostname can handle a situation when /// the query is broken (required parameter is missing). void testLease4GetByHostnameParams(); @@ -1199,6 +1203,39 @@ void Lease4CmdsTest::testLease4GetMissingParams() { string exp_rsp = "Mandatory 'subnet-id' parameter missing."; testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp); + // Reject not numeric subnet id. + cmd = + "{\n" + " \"command\": \"lease4-get\",\n" + " \"arguments\": {" + " \"subnet-id\": \"foo\"" + " }\n" + "}"; + exp_rsp = "'subnet-id' parameter is not integer."; + testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp); + + // Reject negative subnet id. + cmd = + "{\n" + " \"command\": \"lease4-get\",\n" + " \"arguments\": {" + " \"subnet-id\": -1" + " }\n" + "}"; + exp_rsp = "'subnet-id' parameter is not a 32 bit unsigned integer."; + testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp); + + // Reject too large subnet id. + cmd = + "{\n" + " \"command\": \"lease4-get\",\n" + " \"arguments\": {" + " \"subnet-id\": 4294967297" + " }\n" + "}"; + exp_rsp = "'subnet-id' parameter is not a 32 bit unsigned integer."; + testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp); + // Just the subnet-id won't cut it, either. cmd = "{\n" @@ -1616,6 +1653,28 @@ void Lease4CmdsTest::testLease4GetBySubnetIdInvalidArguments() { "}"; exp_rsp = "listed subnet identifiers must be numbers"; testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp); + + // Subnets list must contain positive numbers.. + cmd = + "{\n" + " \"command\": \"lease4-get-all\",\n" + " \"arguments\": {" + " \"subnets\": [ 1, -1 ]\n" + " }\n" + "}"; + exp_rsp = "out of range subnet identifier -1"; + testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp); + + // Subnets list must contain 32 bit unsigned integers.. + cmd = + "{\n" + " \"command\": \"lease4-get-all\",\n" + " \"arguments\": {" + " \"subnets\": [ 1, 4294967297 ]\n" + " }\n" + "}"; + exp_rsp = "out of range subnet identifier 4294967297"; + testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp); } void Lease4CmdsTest::testLease4GetPaged() { @@ -2180,6 +2239,44 @@ void Lease4CmdsTest::testLease4GetByStateFindN() { } } +void Lease4CmdsTest::testLease4GetByStateBadArgs() { + // Subnet id must be a number. + string cmd = + "{\n" + " \"command\": \"lease4-get-by-state\",\n" + " \"arguments\": {" + " \"state\": 3,\n" + " \"subnet-id\": \"foo\"\n" + " }\n" + "}"; + string exp_rsp = "'subnet-id' parameter must be a number"; + testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp); + + // Subnet id must be positive. + cmd = + "{\n" + " \"command\": \"lease4-get-by-state\",\n" + " \"arguments\": {" + " \"state\": 3,\n" + " \"subnet-id\": -1\n" + " }\n" + "}"; + exp_rsp = "'subnet-id' parameter must be a 32 bit unsigned integer"; + testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp); + + // Subnet id must be fir into 32 bits. + cmd = + "{\n" + " \"command\": \"lease4-get-by-state\",\n" + " \"arguments\": {" + " \"state\": 3,\n" + " \"subnet-id\": 4294967297\n" + " }\n" + "}"; + exp_rsp = "'subnet-id' parameter must be a 32 bit unsigned integer"; + testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp); +} + void Lease4CmdsTest::testLease4GetByHostnameParams() { // No parameters whatsoever. string cmd = @@ -4348,6 +4445,15 @@ TEST_F(Lease4CmdsTest, lease4GetByStateFindNMultiThreading) { testLease4GetByStateFindN(); } +TEST_F(Lease4CmdsTest, lease4GetByStateBadArgs) { + testLease4GetByStateBadArgs(); +} + +TEST_F(Lease4CmdsTest, lease4GetByStateBadArgsMultiThreading) { + MultiThreadingTest mt(true); + testLease4GetByStateBadArgs(); +} + TEST_F(Lease4CmdsTest, lease4GetByHostnameParams) { testLease4GetByHostnameParams(); } 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 fd4ade3ac5..9fbe4a7bbb 100644 --- a/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds6_unittest.cc +++ b/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds6_unittest.cc @@ -341,6 +341,10 @@ public: /// combinations). void testLease6GetByStateFindN(); + /// @brief Check that lease6-get-by-state raises an error when + /// the argument is bad. + void testLease6GetByStateBadArgs(); + /// @brief Check that lease6-get-by-hostname can handle a situation when /// the query is broken (required parameter is missing). void testLease6GetByHostnameParams(); @@ -1452,6 +1456,39 @@ void Lease6CmdsTest::testLease6GetMissingParams() { string exp_rsp = "Mandatory 'subnet-id' parameter missing."; testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp); + // Reject not numeric subnet id. + cmd = + "{\n" + " \"command\": \"lease6-get\",\n" + " \"arguments\": {" + " \"subnet-id\": \"foo\"" + " }\n" + "}"; + exp_rsp = "'subnet-id' parameter is not integer."; + testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp); + + // Reject negative subnet id. + cmd = + "{\n" + " \"command\": \"lease6-get\",\n" + " \"arguments\": {" + " \"subnet-id\": -1" + " }\n" + "}"; + exp_rsp = "'subnet-id' parameter is not a 32 bit unsigned integer."; + testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp); + + // Reject too large subnet id. + cmd = + "{\n" + " \"command\": \"lease6-get\",\n" + " \"arguments\": {" + " \"subnet-id\": 4294967297" + " }\n" + "}"; + exp_rsp = "'subnet-id' parameter is not a 32 bit unsigned integer."; + testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp); + // Just the subnet-id won't cut it, either. cmd = "{\n" @@ -1880,6 +1917,28 @@ void Lease6CmdsTest::testLease6GetBySubnetIdInvalidArguments() { "}"; exp_rsp = "listed subnet identifiers must be numbers"; testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp); + + // Subnets list must contain positive numbers.. + cmd = + "{\n" + " \"command\": \"lease6-get-all\",\n" + " \"arguments\": {" + " \"subnets\": [ 1, -1 ]\n" + " }\n" + "}"; + exp_rsp = "out of range subnet identifier -1"; + testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp); + + // Subnets list must contain 32 bit unsigned integers.. + cmd = + "{\n" + " \"command\": \"lease6-get-all\",\n" + " \"arguments\": {" + " \"subnets\": [ 1, 4294967297 ]\n" + " }\n" + "}"; + exp_rsp = "out of range subnet identifier 4294967297"; + testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp); } void Lease6CmdsTest::testLease6GetPaged() { @@ -2450,6 +2509,44 @@ void Lease6CmdsTest::testLease6GetByStateFindN() { } } +void Lease6CmdsTest::testLease6GetByStateBadArgs() { + // Subnet id must be a number. + string cmd = + "{\n" + " \"command\": \"lease6-get-by-state\",\n" + " \"arguments\": {" + " \"state\": 3,\n" + " \"subnet-id\": \"foo\"\n" + " }\n" + "}"; + string exp_rsp = "'subnet-id' parameter must be a number"; + testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp); + + // Subnet id must be positive. + cmd = + "{\n" + " \"command\": \"lease6-get-by-state\",\n" + " \"arguments\": {" + " \"state\": 3,\n" + " \"subnet-id\": -1\n" + " }\n" + "}"; + exp_rsp = "'subnet-id' parameter must be a 32 bit unsigned integer"; + testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp); + + // Subnet id must be fir into 32 bits. + cmd = + "{\n" + " \"command\": \"lease6-get-by-state\",\n" + " \"arguments\": {" + " \"state\": 3,\n" + " \"subnet-id\": 4294967297\n" + " }\n" + "}"; + exp_rsp = "'subnet-id' parameter must be a 32 bit unsigned integer"; + testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp); +} + void Lease6CmdsTest::testLease6GetByHostnameParams() { // No parameters whatsoever. string cmd = @@ -5411,6 +5508,15 @@ TEST_F(Lease6CmdsTest, lease6GetByStateFindNMultiThreading) { testLease6GetByStateFindN(); } +TEST_F(Lease6CmdsTest, lease6GetByStateBadArgs) { + testLease6GetByStateBadArgs(); +} + +TEST_F(Lease6CmdsTest, lease6GetByStateBadArgsMultiThreading) { + MultiThreadingTest mt(true); + testLease6GetByStateBadArgs(); +} + TEST_F(Lease6CmdsTest, lease6GetByHostnameParams) { testLease6GetByHostnameParams(); }