[3618] augmented unit tests for trancated options and vendor options

src/lib/dhcp/option_vendor.cc

@@ -58,7 +58,7 @@ void OptionVendor::unpack(OptionBufferConstIter begin,
 
     vendor_id_ = isc::util::readUint32(&(*begin), distance(begin, end));
 
-    OptionBuffer vendor_buffer(begin +4, end);
+    OptionBuffer vendor_buffer(begin + 4, end);
 
     if (universe_ == Option::V6) {
         LibDHCP::unpackVendorOptions6(vendor_id_, vendor_buffer, options_);

src/lib/dhcp/tests/pkt4_unittest.cc

@@ -654,6 +654,105 @@ TEST_F(Pkt4Test, unpackOptions) {
     verifyParsedOptions(pkt);
 }
 
+// Checks if the code is able to handle a malformed option
+TEST_F(Pkt4Test, unpackMalformed) {
+
+    vector<uint8_t> orig = generateTestPacket2();
+
+    orig.push_back(0x63);
+    orig.push_back(0x82);
+    orig.push_back(0x53);
+    orig.push_back(0x63);
+
+    orig.push_back(53); // Message Type 
+    orig.push_back(1); // length=1
+    orig.push_back(2); // type=2
+
+    orig.push_back(12); // Hostname
+    orig.push_back(3); // length=3
+    orig.push_back(102); // data="foo"
+    orig.push_back(111);
+    orig.push_back(111);
+
+    // That's our original content. It should be sane.
+    Pkt4Ptr success(new Pkt4(&orig[0], orig.size()));
+    EXPECT_NO_THROW(success->unpack());
+
+    // At the exception of END and PAD an option must have a length byte
+    vector<uint8_t> nolength = orig;
+    nolength.resize(orig.size() - 4);
+    Pkt4Ptr no_length_pkt(new Pkt4(&nolength[0], nolength.size()));
+    EXPECT_THROW(no_length_pkt->unpack(), OutOfRange);
+
+    // Truncated data is not accepted too but doesn't throw
+    vector<uint8_t> shorty = orig;
+    shorty.resize(orig.size() - 1);
+    Pkt4Ptr too_short_pkt(new Pkt4(&shorty[0], shorty.size()));
+    EXPECT_NO_THROW(too_short_pkt->unpack());
+}
+
+// Checks if the code is able to handle a malformed vendor option
+TEST_F(Pkt4Test, unpackVendorMalformed) {
+
+    vector<uint8_t> orig = generateTestPacket2();
+
+    orig.push_back(0x63);
+    orig.push_back(0x82);
+    orig.push_back(0x53);
+    orig.push_back(0x63);
+
+    orig.push_back(53); // Message Type 
+    orig.push_back(1); // length=1
+    orig.push_back(2); // type=2
+
+    orig.push_back(125); // vivso suboptions
+    size_t full_len_index = orig.size();
+    orig.push_back(15); // length=15
+    orig.push_back(1); // vendor_id=0x1020304
+    orig.push_back(2);
+    orig.push_back(3);
+    orig.push_back(4);
+    size_t data_len_index = orig.size();
+    orig.push_back(10); // data-len=10
+    orig.push_back(128); // suboption type=128
+    orig.push_back(3); // suboption length=3
+    orig.push_back(102); // data="foo"
+    orig.push_back(111);
+    orig.push_back(111);
+    orig.push_back(129); // suboption type=129
+    orig.push_back(3); // suboption length=3
+    orig.push_back(99); // data="bar"
+    orig.push_back(98);
+    orig.push_back(114);
+
+    // That's our original content. It should be sane.
+    Pkt4Ptr success(new Pkt4(&orig[0], orig.size()));
+    EXPECT_NO_THROW(success->unpack());
+
+    // Data-len must match but it doesn't throw
+    vector<uint8_t> baddatalen = orig;
+    baddatalen.resize(orig.size() - 5);
+    baddatalen[full_len_index] = 10;
+    Pkt4Ptr bad_data_len_pkt(new Pkt4(&baddatalen[0], baddatalen.size()));
+    EXPECT_NO_THROW(bad_data_len_pkt->unpack());
+
+    // At the exception of END and PAD, a suboption must have a length byte
+    vector<uint8_t> nolength = orig;
+    nolength.resize(orig.size() - 4);
+    nolength[full_len_index] = 11;
+    nolength[data_len_index] = 6;
+    Pkt4Ptr no_length_pkt(new Pkt4(&nolength[0], nolength.size()));
+    EXPECT_THROW(no_length_pkt->unpack(), InvalidOptionValue);
+
+    // Truncated data is not accepted too
+    vector<uint8_t> shorty = orig;
+    shorty.resize(orig.size() - 1);
+    shorty[full_len_index] = 14;
+    shorty[data_len_index] = 9;
+    Pkt4Ptr too_short_pkt(new Pkt4(&shorty[0], shorty.size()));
+    EXPECT_THROW(too_short_pkt->unpack(), InvalidOptionValue);
+}
+
 // This test verifies that it is possible to specify custom implementation of
 // the option parsing algorithm by installing a callback function.
 TEST_F(Pkt4Test, unpackOptionsWithCallback) {

src/lib/dhcp/tests/pkt6_unittest.cc

@@ -379,6 +379,56 @@ TEST_F(Pkt6Test, unpackMalformed) {
     EXPECT_FALSE(trunc_option->getOption(123));
 }
 
+// Checks if the code is able to handle a malformed vendor option
+TEST_F(Pkt6Test, unpackVendorMalformed) {
+    // Get a packet. We're really interested in its on-wire
+    // representation only.
+    scoped_ptr<Pkt6> donor(capture1());
+
+    // Add a vendor option
+    OptionBuffer orig = donor->data_;
+
+    orig.push_back(0); // vendor options
+    orig.push_back(17);
+    orig.push_back(0);
+    size_t len_index = orig.size();
+    orig.push_back(20); // length=18
+    orig.push_back(1); // vendor_id=0x1020304
+    orig.push_back(2);
+    orig.push_back(3);
+    orig.push_back(4);
+    orig.push_back(1); // suboption type=0x101
+    orig.push_back(1); 
+    orig.push_back(0); // suboption length=3
+    orig.push_back(3);
+    orig.push_back(102); // data="foo"
+    orig.push_back(111);
+    orig.push_back(111);
+    orig.push_back(1); // suboption type=0x102
+    orig.push_back(2);
+    orig.push_back(0); // suboption lenth=3
+    orig.push_back(3);
+    orig.push_back(99); // data="bar'
+    orig.push_back(98);
+    orig.push_back(114);
+
+    Pkt6Ptr success(new Pkt6(&orig[0], orig.size()));
+    EXPECT_NO_THROW(success->unpack());
+
+    // Truncated vendor option is not accepted but doesn't throw
+    vector<uint8_t> shortv = orig;
+    shortv.resize(orig.size() - 2);
+    Pkt6Ptr too_short_vendor_pkt(new Pkt6(&shortv[0], shortv.size()));
+    EXPECT_NO_THROW(too_short_vendor_pkt->unpack());
+    
+    // Truncated data is not accepted but doesn't throw
+    vector<uint8_t> shorto = orig;
+    shorto.resize(orig.size() - 2);
+    shorto[len_index] = 18;
+    Pkt6Ptr too_short_option_pkt(new Pkt6(&shorto[0], shorto.size()));
+    EXPECT_NO_THROW(too_short_option_pkt->unpack());
+}
+
 // This test verifies that it is possible to specify custom implementation of
 // the option parsing algorithm by installing a callback function.
 TEST_F(Pkt6Test, packUnpackWithCallback) {