Merge pull request #9194

9d101d5 serialization: protect blob serialization from undefined behavior (jeffro256)

src/ringct/rctTypes.h

@@ -771,7 +771,7 @@ namespace std
 BLOB_SERIALIZER(rct::key);
 BLOB_SERIALIZER(rct::key64);
 BLOB_SERIALIZER(rct::ctkey);
-BLOB_SERIALIZER(rct::multisig_kLRki);
+BLOB_SERIALIZER_FORCED(rct::multisig_kLRki);
 BLOB_SERIALIZER(rct::boroSig);
 
 VARIANT_TAG(debug_archive, rct::key, "rct::key");

src/serialization/crypto.h

@@ -81,7 +81,7 @@ BLOB_SERIALIZER(crypto::chacha_iv);
 BLOB_SERIALIZER(crypto::hash);
 BLOB_SERIALIZER(crypto::hash8);
 BLOB_SERIALIZER(crypto::public_key);
-BLOB_SERIALIZER(crypto::secret_key);
+BLOB_SERIALIZER_FORCED(crypto::secret_key);
 BLOB_SERIALIZER(crypto::key_derivation);
 BLOB_SERIALIZER(crypto::key_image);
 BLOB_SERIALIZER(crypto::signature);

src/serialization/serialization.h

@@ -50,13 +50,16 @@
 #include <boost/type_traits/integral_constant.hpp>
 #include <boost/mpl/bool.hpp>
 
-/*! \struct is_blob_type 
+/*! \struct is_blob_type / is_blob_forced
  *
- * \brief a descriptor for dispatching serialize
+ * \brief descriptors for dispatching serialize: whether to take byte-wise copy/store to type
  */
 template <class T>
 struct is_blob_type { typedef boost::false_type type; };
 
+template <class T>
+struct is_blob_forced: std::false_type {};
+
 /*! \fn do_serialize(Archive &ar, T &v)
  *
  * \brief main function for dispatching serialization for a given pair of archive and value types
@@ -68,6 +71,8 @@ struct is_blob_type { typedef boost::false_type type; };
 template <class Archive, class T>
 inline std::enable_if_t<is_blob_type<T>::type::value, bool> do_serialize(Archive &ar, T &v)
 {
+  static_assert(std::is_trivially_copyable<T>() || is_blob_forced<T>(),
+    "sanity check: types that can't be trivially copied shouldn't be using the blob serializer");
   ar.serialize_blob(&v, sizeof(v));
   return true;
 }
@@ -94,6 +99,9 @@ inline bool do_serialize(Archive &ar, bool &v)
 /*! \macro BLOB_SERIALIZER
  *
  * \brief makes the type have a blob serializer trait defined
+ *
+ * In case your type is not a good candidate to be blob serialized, a static assertion may be thrown
+ * at compile-time.
  */
 #define BLOB_SERIALIZER(T)						\
   template<>								\
@@ -101,6 +109,20 @@ inline bool do_serialize(Archive &ar, bool &v)
     typedef boost::true_type type;					\
   }
 
+/*! \macro BLOB_SERIALIZER_FORCED
+ *
+ * \brief makes the type have a blob serializer trait defined, even if it isn't trivially copyable
+ *
+ * Caution: do NOT use this macro for your type <T>, unless you are absolutely sure that moving raw
+ * bytes in/out of this type will not cause undefined behavior. Any types with managed memory
+ * (e.g. vector, string, etc) will segfault and/or cause memory errors if you use this macro with
+ * that type.
+ */
+#define BLOB_SERIALIZER_FORCED(T) \
+  BLOB_SERIALIZER(T);             \
+  template<>                      \
+  struct is_blob_forced<T>: std::true_type {};
+
 /*! \macro VARIANT_TAG
  *
  * \brief Adds the tag \tag to the \a Archive of \a Type

tests/unit_tests/serialization.cpp

@@ -51,6 +51,11 @@
 using namespace std;
 using namespace crypto;
 
+static_assert(!std::is_trivially_copyable<std::vector<unsigned char>>(),
+  "should fail to compile when applying blob serializer");
+static_assert(!std::is_trivially_copyable<std::string>(),
+  "should fail to compile when applying blob serializer");
+
 struct Struct
 {
   int32_t a;