ORC-317: [C++] check that indices in the protobuf type tree are valid

c++/src/Reader.cc

@@ -897,6 +897,34 @@ namespace orc {
     return REDUNDANT_MOVE(postscript);
   }
 
+  // ORC-317: check that indices in the type tree are valid, so we won't crash
+  // when we convert the proto::Types to TypeImpls.
+  void checkProtoTypeIds(const proto::Footer &footer) {
+    std::stringstream msg;
+    int maxId = footer.types_size();
+    for (int i = 0; i < maxId; ++i) {
+      const proto::Type& type = footer.types(i);
+      for (int j = 0; j < type.subtypes_size(); ++j) {
+        int subTypeId = static_cast<int>(type.subtypes(j));
+        if (subTypeId <= i) {
+          msg << "Footer is corrupt: malformed link from type " << i << " to "
+              << subTypeId;
+          throw ParseError(msg.str());
+        }
+        if (subTypeId >= maxId) {
+          msg << "Footer is corrupt: types(" << subTypeId << ") not exists";
+          throw ParseError(msg.str());
+        }
+        if (j > 0 && static_cast<int>(type.subtypes(j - 1)) >= subTypeId) {
+          msg << "Footer is corrupt: subType(" << (j-1) << ") >= subType(" << j
+              << ") in types(" << i << "). (" << type.subtypes(j - 1) << " >= "
+              << subTypeId << ")";
+          throw ParseError(msg.str());
+        }
+      }
+    }
+  }
+
   /**
    * Parse the footer from the given buffer.
    * @param stream the file's stream
@@ -926,6 +954,8 @@ namespace orc {
       throw ParseError("Failed to parse the footer from " +
                        stream->getName());
     }
+
+    checkProtoTypeIds(*footer);
     return REDUNDANT_MOVE(footer);
   }
 

c++/test/TestType.cc

@@ -23,6 +23,7 @@
 #include "wrap/gtest-wrapper.h"
 
 #include "TypeImpl.hh"
+#include "Reader.cc"
 
 namespace orc {
 
@@ -339,4 +340,68 @@ namespace orc {
     testCorruptHelper(illUnionType, footer,
         "Illegal UNION type that doesn't contain any subtypes");
   }
+
+  void expectParseError(const proto::Footer &footer, const char* errMsg) {
+    try {
+      checkProtoTypeIds(footer);
+      FAIL() << "Should throw ParseError for ill ids";
+    } catch (ParseError& e) {
+      EXPECT_EQ(e.what(), std::string(errMsg));
+    } catch (...) {
+      FAIL() << "Should only throw ParseError for ill ids";
+    }
+  }
+
+  TEST(TestType, testCheckProtoTypeIds) {
+    proto::Footer footer;
+    proto::Type rootType;
+    rootType.set_kind(proto::Type_Kind_STRUCT);
+    rootType.add_subtypes(1); // add a non existent type id
+    *(footer.add_types()) = rootType;
+    expectParseError(footer, "Footer is corrupt: types(1) not exists");
+
+    footer.clear_types();
+    rootType.clear_subtypes();
+    proto::Type structType;
+    structType.set_kind(proto::Type_Kind_STRUCT);
+    structType.add_subtypes(0);  // construct a loop back to root
+    rootType.add_subtypes(1);
+    *(footer.add_types()) = rootType;
+    *(footer.add_types()) = structType;
+    expectParseError(footer,
+        "Footer is corrupt: malformed link from type 1 to 0");
+
+    footer.clear_types();
+    rootType.clear_subtypes();
+    proto::Type listType;
+    listType.set_kind(proto::Type_Kind_LIST);
+    proto::Type mapType;
+    mapType.set_kind(proto::Type_Kind_MAP);
+    proto::Type unionType;
+    unionType.set_kind(proto::Type_Kind_UNION);
+    rootType.add_subtypes(1);   // 0 -> 1
+    listType.add_subtypes(2);   // 1 -> 2
+    mapType.add_subtypes(3);    // 2 -> 3
+    unionType.add_subtypes(1);  // 3 -> 1
+    *(footer.add_types()) = rootType;   // 0
+    *(footer.add_types()) = listType;   // 1
+    *(footer.add_types()) = mapType;    // 2
+    *(footer.add_types()) = unionType;  // 3
+    expectParseError(footer,
+        "Footer is corrupt: malformed link from type 3 to 1");
+
+    footer.clear_types();
+    rootType.clear_subtypes();
+    proto::Type intType;
+    intType.set_kind(proto::Type_Kind_INT);
+    proto::Type strType;
+    strType.set_kind(proto::Type_Kind_STRING);
+    rootType.add_subtypes(2);
+    rootType.add_subtypes(1);
+    *(footer.add_types()) = rootType;
+    *(footer.add_types()) = intType;
+    *(footer.add_types()) = strType;
+    expectParseError(footer,
+        "Footer is corrupt: subType(0) >= subType(1) in types(0). (2 >= 1)");
+  }
 }