Merge branch 'trac2749'

src/bin/dhcp6/dhcp6_srv.cc

@@ -646,10 +646,11 @@ Dhcpv6Srv::generateServerID() {
         seconds -= DUID_TIME_EPOCH;
 
         OptionBuffer srvid(8 + iface->getMacLen());
-        writeUint16(DUID::DUID_LLT, &srvid[0]);
-        writeUint16(HWTYPE_ETHERNET, &srvid[2]);
-        writeUint32(static_cast<uint32_t>(seconds), &srvid[4]);
-        memcpy(&srvid[0] + 8, iface->getMac(), iface->getMacLen());
+        // We know that the buffer is more than 8 bytes long at this point.
+        writeUint16(DUID::DUID_LLT, &srvid[0], 2);
+        writeUint16(HWTYPE_ETHERNET, &srvid[2], 2);
+        writeUint32(static_cast<uint32_t>(seconds), &srvid[4], 4);
+        memcpy(&srvid[8], iface->getMac(), iface->getMacLen());
 
         serverid_ = OptionPtr(new Option(Option::V6, D6O_SERVERID,
                                          srvid.begin(), srvid.end()));
@@ -662,8 +663,8 @@ Dhcpv6Srv::generateServerID() {
     // See Section 9.3 of RFC3315 for details.
 
     OptionBuffer srvid(12);
-    writeUint16(DUID::DUID_EN, &srvid[0]);
-    writeUint32(ENTERPRISE_ID_ISC, &srvid[2]);
+    writeUint16(DUID::DUID_EN, &srvid[0], srvid.size());
+    writeUint32(ENTERPRISE_ID_ISC, &srvid[2], srvid.size() - 2);
 
     // Length of the identifier is company specific. I hereby declare
     // ISC "standard" of 6 bytes long pseudo-random numbers.
@@ -2388,10 +2389,13 @@ Dhcpv6Srv::unpackOptions(const OptionBuffer& buf,
     // The buffer being read comprises a set of options, each starting with
     // a two-byte type code and a two-byte length field.
     while (offset + 4 <= length) {
-        uint16_t opt_type = isc::util::readUint16(&buf[offset]);
+        // At this point, from the while condition, we know that there
+        // are at least 4 bytes available following offset in the
+        // buffer.
+        uint16_t opt_type = isc::util::readUint16(&buf[offset], 2);
         offset += 2;
 
-        uint16_t opt_len = isc::util::readUint16(&buf[offset]);
+        uint16_t opt_len = isc::util::readUint16(&buf[offset], 2);
         offset += 2;
 
         if (offset + opt_len > length) {

src/lib/asiodns/io_fetch.cc

@@ -162,7 +162,7 @@ struct IOFetchData {
     // we sent.
     bool responseOK() {
         return (*remote_snd == *remote_rcv && cumulative >= 2 &&
-                readUint16(received->getData()) == qid);
+                readUint16(received->getData(), received->getLength()) == qid);
     }
 };
 

src/lib/asiodns/tests/io_fetch_unittest.cc

@@ -280,7 +280,8 @@ public:
         cumulative_ += length;
         bool complete = false;
         if (cumulative_ > 2) {
-            uint16_t dns_length = readUint16(receive_buffer_);
+            uint16_t dns_length = readUint16(receive_buffer_,
+                                             sizeof(receive_buffer_));
             complete = ((dns_length + 2) == cumulative_);
         }
 
@@ -310,7 +311,10 @@ public:
         send_buffer_.clear();
         send_buffer_.push_back(0);
         send_buffer_.push_back(0);
-        writeUint16(return_data_.size(), &send_buffer_[0]);
+        // send_buffer_.capacity() seems more logical below, but the
+        // code above fills in the first two bytes and size() becomes 2
+        // (sizeof uint16_t).
+        writeUint16(return_data_.size(), &send_buffer_[0], send_buffer_.size());
         copy(return_data_.begin(), return_data_.end(), back_inserter(send_buffer_));
         if (return_data_.size() >= 2) {
             send_buffer_[2] = qid_0;

src/lib/asiolink/tcp_socket.h

@@ -358,7 +358,7 @@ TCPSocket<C>::processReceivedData(const void* staging, size_t length,
         }
 
         // Have enough data to interpret the packet count, so do so now.
-        expected = isc::util::readUint16(data);
+        expected = isc::util::readUint16(data, cumulative);
 
         // We have two bytes less of data to process.  Point to the start of the
         // data and adjust the packet size.  Note that at this point,

src/lib/asiolink/tests/tcp_socket_unittest.cc

@@ -227,7 +227,7 @@ serverRead(tcp::socket& socket, TCPCallback& server_cb) {
         // If we have read at least two bytes, we can work out how much we
         // should be reading.
         if (server_cb.cumulative() >= 2) {
-           server_cb.expected() = readUint16(server_cb.data());
+            server_cb.expected() = readUint16(server_cb.data(), server_cb.length());
             if ((server_cb.expected() + 2) == server_cb.cumulative()) {
 
                 // Amount of data read from socket equals the size of the
@@ -261,7 +261,7 @@ TEST(TCPSocket, processReceivedData) {
     }
 
     // Check that the method will handle various receive sizes.
-    writeUint16(PACKET_SIZE, inbuff);
+    writeUint16(PACKET_SIZE, inbuff, sizeof(inbuff));
 
     cumulative = 0;
     offset = 0;
@@ -317,7 +317,7 @@ TEST(TCPSocket, processReceivedData) {
 // Tests the operation of a TCPSocket by opening it, sending an asynchronous
 // message to a server, receiving an asynchronous message from the server and
 // closing.
-TEST(TCPSocket, SequenceTest) {
+TEST(TCPSocket, sequenceTest) {
 
     // Common objects.
     IOService   service;                    // Service object for async control
@@ -408,7 +408,7 @@ TEST(TCPSocket, SequenceTest) {
     server_cb.length() = 0;
     server_cb.cumulative() = 0;
 
-    writeUint16(sizeof(INBOUND_DATA), server_cb.data());
+    writeUint16(sizeof(INBOUND_DATA), server_cb.data(), TCPCallback::MIN_SIZE);
     copy(INBOUND_DATA, (INBOUND_DATA + sizeof(INBOUND_DATA) - 1),
         (server_cb.data() + 2));
     server_socket.async_send(asio::buffer(server_cb.data(),

src/lib/asiolink/tests/udp_socket_unittest.cc

@@ -188,7 +188,7 @@ TEST(UDPSocket, processReceivedData) {
     // two bytes of the buffer.
     uint16_t count = 0;
     for (uint32_t i = 0; i < (2 << 16); ++i, ++count) {
-        writeUint16(count, inbuff);
+        writeUint16(count, inbuff, sizeof(inbuff));
 
         // Set some random values
         cumulative = 5;

src/lib/dhcp/libdhcp++.cc

@@ -231,10 +231,10 @@ size_t LibDHCP::unpackOptions6(const OptionBuffer& buf,
     // The buffer being read comprises a set of options, each starting with
     // a two-byte type code and a two-byte length field.
     while (offset + 4 <= length) {
-        uint16_t opt_type = isc::util::readUint16(&buf[offset]);
+        uint16_t opt_type = isc::util::readUint16(&buf[offset], 2);
         offset += 2;
 
-        uint16_t opt_len = isc::util::readUint16(&buf[offset]);
+        uint16_t opt_len = isc::util::readUint16(&buf[offset], 2);
         offset += 2;
 
         if (offset + opt_len > length) {
@@ -413,10 +413,10 @@ size_t LibDHCP::unpackVendorOptions6(const uint32_t vendor_id,
     // The buffer being read comprises a set of options, each starting with
     // a two-byte type code and a two-byte length field.
     while (offset + 4 <= length) {
-        uint16_t opt_type = isc::util::readUint16(&buf[offset]);
+        uint16_t opt_type = isc::util::readUint16(&buf[offset], 2);
         offset += 2;
 
-        uint16_t opt_len = isc::util::readUint16(&buf[offset]);
+        uint16_t opt_len = isc::util::readUint16(&buf[offset], 2);
         offset += 2;
 
         if (offset + opt_len > length) {

src/lib/dhcp/option.cc

@@ -250,35 +250,28 @@ uint8_t Option::getUint8() {
 }
 
 uint16_t Option::getUint16() {
-    if (data_.size() < sizeof(uint16_t) ) {
-        isc_throw(OutOfRange, "Attempt to read uint16 from option " << type_
-                  << " that has size " << data_.size());
-    }
-
-    return ( readUint16(&data_[0]) );
+    // readUint16() checks and throws OutOfRange if data_ is too small.
+    return (readUint16(&data_[0], data_.size()));
 }
 
 uint32_t Option::getUint32() {
-    if (data_.size() < sizeof(uint32_t) ) {
-        isc_throw(OutOfRange, "Attempt to read uint32 from option " << type_
-                  << " that has size " << data_.size());
-    }
-    return ( readUint32(&data_[0]) );
+    // readUint32() checks and throws OutOfRange if data_ is too small.
+    return (readUint32(&data_[0], data_.size()));
 }
 
 void Option::setUint8(uint8_t value) {
-  data_.resize(1);
-  data_[0] = value;
+    data_.resize(sizeof(value));
+    data_[0] = value;
 }
 
 void Option::setUint16(uint16_t value) {
-  data_.resize(2);
-  writeUint16(value, &data_[0]);
+    data_.resize(sizeof(value));
+    writeUint16(value, &data_[0], data_.size());
 }
 
 void Option::setUint32(uint32_t value) {
-  data_.resize(4);
-  writeUint32(value, &data_[0]);
+    data_.resize(sizeof(value));
+    writeUint32(value, &data_[0], data_.size());
 }
 
 bool Option::equal(const OptionPtr& other) const {

src/lib/dhcp/option4_addrlst.cc

@@ -53,7 +53,7 @@ Option4AddrLst::Option4AddrLst(uint8_t type, OptionBufferConstIter first,
 
     while (first != last) {
         const uint8_t* ptr = &(*first);
-        addAddress(IOAddress(readUint32(ptr)));
+        addAddress(IOAddress(readUint32(ptr, distance(first, last))));
         first += V4ADDRESS_LEN;
     }
 }

src/lib/dhcp/option6_ia.cc

@@ -72,12 +72,12 @@ void Option6IA::unpack(OptionBufferConstIter begin,
     if (distance(begin, end) < OPTION6_IA_LEN) {
         isc_throw(OutOfRange, "Option " << type_ << " truncated");
     }
-    iaid_ = readUint32( &(*begin) );
+    iaid_ = readUint32(&(*begin), distance(begin, end));
     begin += sizeof(uint32_t);
-    t1_ = readUint32( &(*begin) );
+    t1_ = readUint32(&(*begin), distance(begin, end));
     begin += sizeof(uint32_t);
 
-    t2_ = readUint32( &(*begin) );
+    t2_ = readUint32(&(*begin), distance(begin, end));
     begin += sizeof(uint32_t);
 
     unpackOptions(OptionBuffer(begin, end));

src/lib/dhcp/option6_iaaddr.cc

@@ -78,10 +78,10 @@ void Option6IAAddr::unpack(OptionBuffer::const_iterator begin,
     addr_ = IOAddress::fromBytes(AF_INET6, &(*begin));
     begin += V6ADDRESS_LEN;
 
-    preferred_ = readUint32( &(*begin) );
+    preferred_ = readUint32(&(*begin), distance(begin, end));
     begin += sizeof(uint32_t);
 
-    valid_ = readUint32( &(*begin) );
+    valid_ = readUint32(&(*begin), distance(begin, end));
     begin += sizeof(uint32_t);
 
     unpackOptions(OptionBuffer(begin, end));

src/lib/dhcp/option6_iaprefix.cc

@@ -76,10 +76,10 @@ void Option6IAPrefix::unpack(OptionBuffer::const_iterator begin,
         isc_throw(OutOfRange, "Option " << type_ << " truncated");
     }
 
-    preferred_ = readUint32( &(*begin) );
+    preferred_ = readUint32(&(*begin), distance(begin, end));
     begin += sizeof(uint32_t);
 
-    valid_ = readUint32( &(*begin) );
+    valid_ = readUint32(&(*begin), distance(begin, end));
     begin += sizeof(uint32_t);
 
     prefix_len_ = *begin;

src/lib/dhcp/option_data_types.h

@@ -298,12 +298,12 @@ public:
         case 2:
             // Calling readUint16 works either for unsigned
             // or signed types.
-            value = isc::util::readUint16(&(*buf.begin()));
+            value = isc::util::readUint16(&(*buf.begin()), buf.size());
             break;
         case 4:
             // Calling readUint32 works either for unsigned
             // or signed types.
-            value = isc::util::readUint32(&(*buf.begin()));
+            value = isc::util::readUint32(&(*buf.begin()), buf.size());
             break;
         default:
             // This should not happen because we made checks on data types
@@ -331,11 +331,11 @@ public:
             break;
         case 2:
             buf.resize(buf.size() + 2);
-            isc::util::writeUint16(static_cast<uint16_t>(value), &buf[buf.size() - 2]);
+            isc::util::writeUint16(static_cast<uint16_t>(value), &buf[buf.size() - 2], 2);
             break;
         case 4:
             buf.resize(buf.size() + 4);
-            isc::util::writeUint32(static_cast<uint32_t>(value), &buf[buf.size() - 4]);
+            isc::util::writeUint32(static_cast<uint32_t>(value), &buf[buf.size() - 4], 4);
             break;
         default:
             // The cases above cover whole range of possible data lengths because

src/lib/dhcp/option_int.h

@@ -142,10 +142,12 @@ public:
             value_ = *begin;
             break;
         case 2:
-            value_ = isc::util::readUint16(&(*begin));
+            value_ = isc::util::readUint16(&(*begin),
+                                           std::distance(begin, end));
             break;
         case 4:
-            value_ = isc::util::readUint32(&(*begin));
+            value_ = isc::util::readUint32(&(*begin),
+                                           std::distance(begin, end));
             break;
         default:
             isc_throw(dhcp::InvalidDataType, "non-integer type");

src/lib/dhcp/option_int_array.h

@@ -201,10 +201,12 @@ public:
                 values_.push_back(*begin);
                 break;
             case 2:
-                values_.push_back(isc::util::readUint16(&(*begin)));
+                values_.push_back(isc::util::readUint16(&(*begin),
+                                      std::distance(begin, end)));
                 break;
             case 4:
-                values_.push_back(isc::util::readUint32(&(*begin)));
+                values_.push_back(isc::util::readUint32(&(*begin),
+                                      std::distance(begin, end)));
                 break;
             default:
                 isc_throw(dhcp::InvalidDataType, "non-integer type");

src/lib/dhcp/option_vendor.cc

@@ -54,7 +54,7 @@ void OptionVendor::unpack(OptionBufferConstIter begin,
                   << ", length=" << distance(begin, end));
     }
 
-    vendor_id_ = isc::util::readUint32(&(*begin));
+    vendor_id_ = isc::util::readUint32(&(*begin), distance(begin, end));
 
     OptionBuffer vendor_buffer(begin +4, end);
 

src/lib/resolve/tests/recursive_query_unittest_2.cc

@@ -462,7 +462,8 @@ public:
         tcp_cumulative_ += length;
         bool complete = false;
         if (tcp_cumulative_ > 2) {
-            uint16_t dns_length = readUint16(tcp_receive_buffer_);
+            uint16_t dns_length = readUint16(tcp_receive_buffer_,
+                                             sizeof(tcp_receive_buffer_));
             complete = ((dns_length + 2) == tcp_cumulative_);
         }
 

src/lib/resolve/tests/recursive_query_unittest_3.cc

@@ -336,7 +336,8 @@ public:
         tcp_cumulative_ += length;
         bool complete = false;
         if (tcp_cumulative_ > 2) {
-            uint16_t dns_length = readUint16(tcp_receive_buffer_);
+            uint16_t dns_length = readUint16(tcp_receive_buffer_,
+                                             sizeof(tcp_receive_buffer_));
             complete = ((dns_length + 2) == tcp_cumulative_);
         }
 

src/lib/util/io_utilities.h

@@ -15,6 +15,7 @@
 #ifndef IO_UTILITIES_H
 #define IO_UTILITIES_H
 
+#include <exceptions/exceptions.h>
 #include <cstddef>
 
 namespace isc {
@@ -28,10 +29,17 @@ namespace util {
 /// \param buffer Data buffer at least two bytes long of which the first two
 ///        bytes are assumed to represent a 16-bit integer in network-byte
 ///        order.
+/// \param length Length of the data buffer.
 ///
 /// \return Value of 16-bit integer
 inline uint16_t
-readUint16(const void* buffer) {
+readUint16(const void* buffer, size_t length) {
+    if (length < sizeof(uint16_t)) {
+        isc_throw(isc::OutOfRange,
+                  "Length (" << length << ") of buffer is insufficient " <<
+                  "to read a uint16_t");
+    }
+
     const uint8_t* byte_buffer = static_cast<const uint8_t*>(buffer);
 
     uint16_t result = (static_cast<uint16_t>(byte_buffer[0])) << 8;
@@ -48,10 +56,17 @@ readUint16(const void* buffer) {
 /// \param value 16-bit value to convert
 /// \param buffer Data buffer at least two bytes long into which the 16-bit
 ///        value is written in network-byte order.
+/// \param length Length of the data buffer.
 ///
 /// \return pointer to the next byte after stored value
 inline uint8_t*
-writeUint16(uint16_t value, void* buffer) {
+writeUint16(uint16_t value, void* buffer, size_t length) {
+    if (length < sizeof(uint16_t)) {
+        isc_throw(isc::OutOfRange,
+                  "Length (" << length << ") of buffer is insufficient " <<
+                  "to write a uint16_t");
+    }
+
     uint8_t* byte_buffer = static_cast<uint8_t*>(buffer);
 
     byte_buffer[0] = static_cast<uint8_t>((value & 0xff00U) >> 8);
@@ -65,10 +80,17 @@ writeUint16(uint16_t value, void* buffer) {
 /// \param buffer Data buffer at least four bytes long of which the first four
 ///        bytes are assumed to represent a 32-bit integer in network-byte
 ///        order.
+/// \param length Length of the data buffer.
 ///
 /// \return Value of 32-bit unsigned integer
 inline uint32_t
-readUint32(const uint8_t* buffer) {
+readUint32(const uint8_t* buffer, size_t length) {
+    if (length < sizeof(uint32_t)) {
+        isc_throw(isc::OutOfRange,
+                  "Length (" << length << ") of buffer is insufficient " <<
+                  "to read a uint32_t");
+    }
+
     const uint8_t* byte_buffer = static_cast<const uint8_t*>(buffer);
 
     uint32_t result = (static_cast<uint32_t>(byte_buffer[0])) << 24;
@@ -84,10 +106,17 @@ readUint32(const uint8_t* buffer) {
 /// \param value 32-bit value to convert
 /// \param buffer Data buffer at least four bytes long into which the 32-bit
 ///        value is written in network-byte order.
+/// \param length Length of the data buffer.
 ///
 /// \return pointer to the next byte after stored value
 inline uint8_t*
-writeUint32(uint32_t value, uint8_t* buffer) {
+writeUint32(uint32_t value, uint8_t* buffer, size_t length) {
+    if (length < sizeof(uint32_t)) {
+        isc_throw(isc::OutOfRange,
+                  "Length (" << length << ") of buffer is insufficient " <<
+                  "to write a uint32_t");
+    }
+
     uint8_t* byte_buffer = static_cast<uint8_t*>(buffer);
 
     byte_buffer[0] = static_cast<uint8_t>((value & 0xff000000U) >> 24);

src/lib/util/tests/io_utilities_unittest.cc

@@ -42,11 +42,15 @@ TEST(asioutil, readUint16) {
             data[0] = i8;
             data[1] = j8;
             buffer.setPosition(0);
-            EXPECT_EQ(buffer.readUint16(), readUint16(data));
+            EXPECT_EQ(buffer.readUint16(), readUint16(data, sizeof(data)));
         }
     }
 }
 
+TEST(asioutil, readUint16OutOfRange) {
+    uint8_t data;
+    EXPECT_THROW(readUint16(&data, sizeof(data)), isc::OutOfRange);
+}
 
 TEST(asioutil, writeUint16) {
 
@@ -64,7 +68,7 @@ TEST(asioutil, writeUint16) {
         buffer.writeUint16(i16);
 
         // ... and the test data
-        writeUint16(i16, test);
+        writeUint16(i16, test, sizeof(test));
 
         // ... and compare
         const uint8_t* ref = static_cast<const uint8_t*>(buffer.getData());
@@ -73,6 +77,12 @@ TEST(asioutil, writeUint16) {
     }
 }
 
+TEST(asioutil, writeUint16OutOfRange) {
+    uint16_t i16 = 42;
+    uint8_t data;
+    EXPECT_THROW(writeUint16(i16, &data, sizeof(data)), isc::OutOfRange);
+}
+
 // test data shared amount readUint32 and writeUint32 tests
 const static uint32_t test32[] = {
     0,
@@ -93,11 +103,15 @@ TEST(asioutil, readUint32) {
             uint32_t tmp = htonl(test32[i]);
             memcpy(&data[offset], &tmp, sizeof(uint32_t));
 
-            EXPECT_EQ(test32[i], readUint32(&data[offset]));
+            EXPECT_EQ(test32[i], readUint32(&data[offset], sizeof(uint32_t)));
         }
     }
 }
 
+TEST(asioutil, readUint32OutOfRange) {
+    uint8_t data[3];
+    EXPECT_THROW(readUint32(data, sizeof(data)), isc::OutOfRange);
+}
 
 TEST(asioutil, writeUint32) {
     uint8_t data[8];
@@ -107,7 +121,8 @@ TEST(asioutil, writeUint32) {
     // it 4 times.
     for (int offset=0; offset < 4; offset++) {
         for (int i=0; i < sizeof(test32)/sizeof(uint32_t); i++) {
-            uint8_t* ptr = writeUint32(test32[i], &data[offset]);
+            uint8_t* ptr = writeUint32(test32[i], &data[offset],
+                                       sizeof(uint32_t));
 
             EXPECT_EQ(&data[offset]+sizeof(uint32_t), ptr);
 
@@ -117,3 +132,9 @@ TEST(asioutil, writeUint32) {
         }
     }
 }
+
+TEST(asioutil, writeUint32OutOfRange) {
+    uint32_t i32 = 28;
+    uint8_t data[3];
+    EXPECT_THROW(writeUint32(i32, data, sizeof(data)), isc::OutOfRange);
+}