Browse Source

tighten validation on the map tag length: due to the wire format limitation
it cannot exceed 255 bytes, but the "from string" constructor didn't check
it. added test cases for long tags including this specific case.


git-svn-id: svn://bind10.isc.org/svn/bind10/trunk@1666 e5f2f494-b856-4b98-b285-d166d9295462

JINMEI Tatuya 15 years ago
parent
commit
f625571df7
2 changed files with 45 additions and 19 deletions
  1. 18 18
      src/lib/cc/data.cc
  2. 27 1
      src/lib/cc/data_unittests.cc

+ 18 - 18
src/lib/cc/data.cc

@@ -438,21 +438,26 @@ from_stringstream_list(std::istream &in, const std::string& file, int& line, int
 }
 }
 
 
 ElementPtr
 ElementPtr
-from_stringstream_map(std::istream &in, const std::string& file, int& line, int& pos)
+from_stringstream_map(std::istream &in, const std::string& file, int& line,
+                      int& pos)
 {
 {
-    char c = 0;
     std::map<std::string, ElementPtr> m;
     std::map<std::string, ElementPtr> m;
-    std::pair<std::string, ElementPtr> p;
-    std::string cur_map_key;
-    ElementPtr cur_map_element;
     skip_chars(in, " \t\n", line, pos);
     skip_chars(in, " \t\n", line, pos);
-    c = in.peek();
+    char c = in.peek();
     if (c == '}') {
     if (c == '}') {
         // empty map, skip closing curly
         // empty map, skip closing curly
         c = in.get();
         c = in.get();
     } else {
     } else {
         while (c != EOF && c != '}') {
         while (c != EOF && c != '}') {
+            std::pair<std::string, ElementPtr> p;
+
             p.first = str_from_stringstream(in, file, line, pos);
             p.first = str_from_stringstream(in, file, line, pos);
+            if (p.first.length() > 255) {
+                // Map tag has one-byte length field in wire format, so the
+                // length cannot exceed 255.
+                throwParseError("Map tag is too long", file, line, pos);
+            }
+
             skip_to(in, file, line, pos, ":", " \t\n");
             skip_to(in, file, line, pos, ":", " \t\n");
             // skip the :
             // skip the :
             in.get();
             in.get();
@@ -929,15 +934,6 @@ ListElement::toWire(std::stringstream& ss, const int omit_length) {
     }
     }
 }
 }
 
 
-namespace {
-void
-encode_tag(std::stringstream& ss, const std::string &s) {
-    const unsigned char val = s.length() & 0x000000ff;
-
-    ss << val << s;
-}
-}
-
 void
 void
 MapElement::toWire(std::stringstream& ss, int omit_length) {
 MapElement::toWire(std::stringstream& ss, int omit_length) {
     std::stringstream ss2;
     std::stringstream ss2;
@@ -951,9 +947,13 @@ MapElement::toWire(std::stringstream& ss, int omit_length) {
     }
     }
 
 
     const std::map<std::string, ElementPtr>& m = mapValue();
     const std::map<std::string, ElementPtr>& m = mapValue();
-    for (std::map<std::string, ElementPtr>::const_iterator it = m.begin() ;
-         it != m.end() ; ++it) {
-        encode_tag(ss2, (*it).first);
+    for (std::map<std::string, ElementPtr>::const_iterator it = m.begin();
+         it != m.end(); ++it) {
+        const size_t taglen = (*it).first.length();
+        assert(taglen <= 0xff);
+        const unsigned char val = (taglen & 0x000000ff);
+        ss2 << val << (*it).first;
+
         (*it).second->toWire(ss2, 0);
         (*it).second->toWire(ss2, 0);
     }
     }
 
 

+ 27 - 1
src/lib/cc/data_unittests.cc

@@ -27,7 +27,7 @@ using std::oct;
 #include <iomanip>
 #include <iomanip>
 using std::setfill;
 using std::setfill;
 using std::setw;
 using std::setw;
-
+using std::string;
 
 
 TEST(Element, type) {
 TEST(Element, type) {
     // this tests checks whether the getType() function returns the
     // this tests checks whether the getType() function returns the
@@ -170,6 +170,15 @@ TEST(Element, ListElement) {
     EXPECT_EQ(el->get(2)->intValue(), 32);
     EXPECT_EQ(el->get(2)->intValue(), 32);
 }
 }
 
 
+namespace {
+const string long_maptag("0123456789abcdef1123456789abcdef2123456789abcdef"
+                         "3123456789abcdef4123456789abcdef5123456789abcdef"
+                         "6123456789abcdef7123456789abcdef8123456789abcdef"
+                         "9123456789abcdefa123456789abcdefb123456789abcdef"
+                         "c123456789abcdefd123456789abcdefe123456789abcdef"
+                         "f123456789abcdef");
+}
+
 TEST(Element, MapElement) {
 TEST(Element, MapElement) {
     // this function checks the specific functions for ListElements
     // this function checks the specific functions for ListElements
     ElementPtr el = Element::createFromString("{ \"name\": \"foo\", \"value1\": \"bar\", \"value2\": { \"number\": 42 } }");
     ElementPtr el = Element::createFromString("{ \"name\": \"foo\", \"value1\": \"bar\", \"value2\": { \"number\": 42 } }");
@@ -194,6 +203,23 @@ TEST(Element, MapElement) {
     
     
     EXPECT_TRUE(el->find("value1", el2));
     EXPECT_TRUE(el->find("value1", el2));
     EXPECT_FALSE(el->find("name/error", el2));
     EXPECT_FALSE(el->find("name/error", el2));
+
+    // A map element whose (only) element has the maximum length of tag.
+    string long_maptag("0123456789abcdef1123456789abcdef2123456789abcdef"
+                       "3123456789abcdef4123456789abcdef5123456789abcdef"
+                       "6123456789abcdef7123456789abcdef8123456789abcdef"
+                       "9123456789abcdefa123456789abcdefb123456789abcdef"
+                       "c123456789abcdefd123456789abcdefe123456789abcdef"
+                       "f123456789abcde");
+    EXPECT_EQ(255, long_maptag.length()); // check prerequisite
+    el = Element::createFromString("{ \"" + long_maptag + "\": \"bar\"}");
+    EXPECT_EQ("bar", el->find(long_maptag)->stringValue());
+
+    // A one-byte longer tag should trigger an exception.
+    long_maptag.push_back('f');
+    EXPECT_THROW(Element::createFromString("{ \"" + long_maptag +
+                                           "\": \"bar\"}"),
+                 ParseError);
 }
 }
 
 
 TEST(Element, to_and_from_wire) {
 TEST(Element, to_and_from_wire) {