BIND 10 #2549: Use IOAddress::fromBytes, isV4 and isV6 functions in DHCP code.

[BIND 10 Development]

#2549: Use IOAddress::fromBytes, isV4 and isV6 functions in DHCP code.
-------------------------------------+-------------------------------------
            Reporter:  marcin        |                        Owner:
                Type:  enhancement   |  marcin
            Priority:  low           |                       Status:
           Component:  dhcp          |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-DHCP-20130103
         Sub-Project:  DHCP          |                   Resolution:
Estimated Difficulty:  0             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by stephen):

 * owner:  UnAssigned => marcin


Comment:

 Reviewed commit 7cdae45e8e18a190a5d2097655f33c0684ef9b9f

 '''src/lib/dhcp/iface_mgr.cc'''[[BR]]
 Line 215 & 251: Strikes me that the comment should be "Skip all but V4
 addresses" with the test as "if (!addr->isV4())" and "Skip V6 addresses"
 if the test is "if (addr->isV6())".

 openSocket6(): a memcpy is dones from "addr", but there is no check that
 the address passed is a V6 address.  This means that the memcpy() coule be
 copying from beyond the end of a four-byte array.

 Line 953: comment is wrong - are only dealing with IPv6 addresses.

 '''src/lib/dhcp/option_definition.cc'''[[BR]]
 Line 385: as the address is neither V4 nor V6, the test in the exception
 message will always cause "v6" to be printed.  A message saying that
 "provided address <text> is not a valid V4 or V6 address" is required.

 ''''src/lib/dhcpsrv/addr_utilities.cc'''[[BR]]
 {first,last}AddrInPrefix6: "packed" is declared as length V6ADDRESS_LEN,
 then 16 bytes is copied into it.  The "memcpy" should use V6ADDRESS_LEN as
 the length of the data to copy.  Or the code should BOOST_STATIC_ASSERT
 that 16 <= V6ADDRESS_LEN.

 '''src/lib/dhcpsrv/alloc_engine.cc'''[[BR]]
 increaseAddress: As "packed" is used for both V4 and V6 addresses, the
 code should include a BOOST_STATIC_ASSERT to check that V6ADDRESS_LEN is
 greater than or equal to V4ADDRESS_LEN (thus ensuring that the buffer is
 large enough for both types of addresses).

 increaseAddress: the only reason for the "isV4()" check is to determine
 what amount of data is being returned.  But that can be done by using the
 intermediate vector returned by toBytes() and using its size() call to
 determine the amount of data to be copied.  A check should be made that
 the size of the data returned is less than or equal to the size of the
 "packed" buffer.

-- 
Ticket URL: <http://bind10.isc.org/ticket/2549#comment:5>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development