Validation in deserializer (#36)

* Validation in Deserializer

Added validation in CDR deserialization: max buffer length is checked
when deserializing fields and strings are checked for null-terminator
(except for wstrings, which are serialized without null-terminator).

Signed-off-by: Dennis Potman <dennis.potman@adlinktech.com>

* Catch exceptions in serdata functions

In serdata functions rmw_print, rmw_to_sample and rmw_from_sample
catch exceptions so that correct return code is given when functions
are called from ddsi.

Signed-off-by: Dennis Potman <dennis.potman@adlinktech.com>

* Improve deserialisation validation

Refactored the deserialisation validation functions so that sequence
length is checked more properly and protection against overflows.
Renamed source files for exceptions so that it conforms to ros2 /
google c++ style guide.

Signed-off-by: Dennis Potman <dennis.potman@adlinktech.com>

README.md

@@ -5,7 +5,7 @@ with [*Eclipse Cyclone DDS*](https://github.com/eclipse-cyclonedds/cyclonedds) a
 implementation.
 
 ## Getting, building and using it
-   
+
 All it takes to get Cyclone DDS support into ROS2 is to clone this repository into the ROS2 workspace
 source directory, and then run colcon build in the usual manner:
 
@@ -34,9 +34,3 @@ There are a number of known limitations:
 
 * Cyclone DDS does not yet implement DDS Security.  Consequently, there is no support for security
   in this RMW implementation either.
-
-* Deserialization only handles native format (it doesn't do any byte swapping).  This is pure
-  laziness, adding it is trivial.
-
-* Deserialization assumes the input is valid and will do terrible things if it isn't.  Again, pure
-  laziness, it's just adding some bounds checks and other validation code.

rmw_cyclonedds_cpp/CMakeLists.txt

@@ -56,6 +56,8 @@ add_library(rmw_cyclonedds_cpp
   src/serdes.cpp
   src/graphrhc.cpp
   src/u16string.cpp
+  src/exception.cpp
+  src/deserialization_exception.cpp
 )
 
 target_link_libraries(rmw_cyclonedds_cpp CycloneDDS::ddsc)

rmw_cyclonedds_cpp/include/rmw_cyclonedds_cpp/TypeSupport_impl.hpp

@@ -654,9 +654,7 @@ inline size_t get_submessage_array_deserialize(
   size_t max_align)
 {
   (void)member;
-  uint32_t vsize = 0;
-  // Deserialize length
-  deser >> vsize;
+  uint32_t vsize = deser.deserialize_len(1);
   auto vector = reinterpret_cast<std::vector<unsigned char> *>(field);
   if (call_new) {
     new(vector) std::vector<unsigned char>;
@@ -677,9 +675,7 @@ inline size_t get_submessage_array_deserialize(
   size_t)
 {
   (void)member;
-  // Deserialize length
-  uint32_t vsize = 0;
-  deser >> vsize;
+  uint32_t vsize = deser.deserialize_len(1);
   auto tmparray = static_cast<rosidl_generator_c__void__Sequence *>(field);
   rosidl_generator_c__void__Sequence__init(tmparray, vsize, sub_members_size);
   subros_message = reinterpret_cast<void *>(tmparray->data);
@@ -786,7 +782,7 @@ void print_field(const M * member, cycprint & deser, T & dummy)
     if (member->array_size_ && !member->is_upper_bound_) {
       deser.printA(&dummy, member->array_size_);
     } else {
-      int32_t dsize = deser.get32();
+      int32_t dsize = deser.get_len(1);
       deser.printA(&dummy, dsize);
     }
     deser.print_constant("}");
@@ -857,7 +853,7 @@ bool TypeSupport<MembersType>::printROSmessage(
             if (member->array_size_ && !member->is_upper_bound_) {
               array_size = member->array_size_;
             } else {
-              array_size = deser.get32();
+              array_size = deser.get_len(1);
             }
             deser.print_constant("{");
             for (size_t index = 0; index < array_size; ++index) {

rmw_cyclonedds_cpp/include/rmw_cyclonedds_cpp/deserialization_exception.hpp

@@ -0,0 +1,37 @@
+// Copyright 2018 to 2019 ADLINK Technology
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef RMW_CYCLONEDDS_CPP__DESERIALIZATION_EXCEPTION_HPP_
+#define RMW_CYCLONEDDS_CPP__DESERIALIZATION_EXCEPTION_HPP_
+
+#include "rmw_cyclonedds_cpp/exception.hpp"
+
+namespace rmw_cyclonedds_cpp
+{
+
+class DeserializationException : public Exception
+{
+public:
+  explicit DeserializationException(const char * const & message);
+  DeserializationException(const DeserializationException & ex);
+  DeserializationException & operator=(const DeserializationException & ex);
+
+  virtual ~DeserializationException() throw();
+
+  static const char * const DEFAULT_MESSAGE;
+};
+
+}  // namespace rmw_cyclonedds_cpp
+
+#endif  // RMW_CYCLONEDDS_CPP__DESERIALIZATION_EXCEPTION_HPP_

rmw_cyclonedds_cpp/include/rmw_cyclonedds_cpp/exception.hpp

@@ -0,0 +1,40 @@
+// Copyright 2018 to 2019 ADLINK Technology
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef RMW_CYCLONEDDS_CPP__EXCEPTION_HPP_
+#define RMW_CYCLONEDDS_CPP__EXCEPTION_HPP_
+
+#include <string>
+#include <exception>
+
+namespace rmw_cyclonedds_cpp
+{
+
+class Exception : public std::exception
+{
+public:
+  virtual ~Exception() throw();
+  virtual const char * what() const throw();
+
+protected:
+  explicit Exception(const char * const & message);
+  Exception(const Exception & ex);
+  Exception & operator=(const Exception & ex);
+
+  std::string m_message;
+};
+
+}  // namespace rmw_cyclonedds_cpp
+
+#endif  // RMW_CYCLONEDDS_CPP__EXCEPTION_HPP_

rmw_cyclonedds_cpp/include/rmw_cyclonedds_cpp/serdes.hpp

@@ -20,10 +20,15 @@
 #include <stdarg.h>
 
 #include <array>
+#include <cassert>
 #include <string>
 #include <vector>
 #include <type_traits>
 
+#include "rmw_cyclonedds_cpp/deserialization_exception.hpp"
+
+using rmw_cyclonedds_cpp::DeserializationException;
+
 class cycser
 {
 public:
@@ -159,17 +164,10 @@ class cycser
 class cycdeserbase
 {
 public:
-  explicit cycdeserbase(const char * data_);
+  explicit cycdeserbase(const char * data_, size_t lim_);
   cycdeserbase() = delete;
 
 protected:
-  inline void align(size_t a)
-  {
-    if ((pos % a) != 0) {
-      pos += a - (pos % a);
-    }
-  }
-
   inline uint16_t bswap2u(uint16_t x)
   {
     return (uint16_t) ((x >> 8) | (x << 8));
@@ -197,8 +195,32 @@ class cycdeserbase
     return (int64_t) bswap8u((uint64_t) x);
   }
 
+  inline void align(size_t a)
+  {
+    if ((pos % a) != 0) {
+      pos += a - (pos % a);
+      if (pos > lim) {
+        throw DeserializationException("invalid data size");
+      }
+    }
+  }
+  inline void validate_size(size_t count, size_t sz)
+  {
+    assert(sz == 1 || sz == 2 || sz == 4 || sz == 8);
+    if (count > (lim - pos) / sz) {
+      throw DeserializationException("invalid data size");
+    }
+  }
+  inline void validate_str(size_t sz)
+  {
+    if (sz > 0 && data[pos + sz - 1] != '\0') {
+      throw DeserializationException("string data is not null-terminated");
+    }
+  }
+
   const char * data;
   size_t pos;
+  size_t lim;
   bool swap_bytes;
 };
 
@@ -231,6 +253,7 @@ class cycdeser : cycdeserbase
 #define DESER8(T) DESER(T, )
 #define DESER(T, fn_swap) inline void deserialize(T & x) { \
     align(sizeof(x)); \
+    validate_size(1, sizeof(x)); \
     x = *reinterpret_cast<const T *>(data + pos); \
     if (swap_bytes) {x = fn_swap(x);} \
     pos += sizeof(x); \
@@ -258,30 +281,36 @@ class cycdeser : cycdeserbase
   {
     deserialize(*reinterpret_cast<uint64_t *>(&x));
   }
-  inline uint32_t deserialize32()
+  inline uint32_t deserialize_len(size_t el_sz)
   {
-    uint32_t sz; deserialize(sz); return sz;
+    uint32_t sz;
+    deserialize(sz);
+    validate_size(sz, el_sz);
+    return sz;
   }
   inline void deserialize(char * & x)
   {
-    const uint32_t sz = deserialize32();
+    const uint32_t sz = deserialize_len(sizeof(char));
+    validate_str(sz);
     x = reinterpret_cast<char *>(malloc(sz));
     memcpy(x, data + pos, sz);
     pos += sz;
   }
   inline void deserialize(std::string & x)
   {
-    const uint32_t sz = deserialize32();
+    const uint32_t sz = deserialize_len(sizeof(char));
     if (sz == 0) {
       x = std::string("");
     } else {
+      validate_str(sz);
       x = std::string(data + pos, sz - 1);
     }
     pos += sz;
   }
   inline void deserialize(std::wstring & x)
   {
-    const uint32_t sz = deserialize32();
+    const uint32_t sz = deserialize_len(sizeof(wchar_t));
+    // wstring is not null-terminated in cdr
     x = std::wstring(reinterpret_cast<const wchar_t *>(data + pos), sz);
     pos += sz * sizeof(wchar_t);
   }
@@ -290,6 +319,7 @@ class cycdeser : cycdeserbase
 #define DESER_A(T, fn_swap) inline void deserializeA(T * x, size_t cnt) { \
     if (cnt > 0) { \
       align(sizeof(T)); \
+      validate_size(cnt, sizeof(T)); \
       if (swap_bytes) { \
         for (size_t i = 0; i < cnt; i++) { \
           x[i] = fn_swap(*reinterpret_cast<const T *>(data + pos)); \
@@ -331,13 +361,13 @@ class cycdeser : cycdeserbase
   template<class T>
   inline void deserialize(std::vector<T> & x)
   {
-    const uint32_t sz = deserialize32();
+    const uint32_t sz = deserialize_len(1);
     x.resize(sz);
     deserializeA(x.data(), sz);
   }
   inline void deserialize(std::vector<bool> & x)
   {
-    const uint32_t sz = deserialize32();
+    const uint32_t sz = deserialize_len(sizeof(unsigned char));
     x.resize(sz);
     for (size_t i = 0; i < sz; i++) {
       x[i] = ((data + pos)[i] != 0);
@@ -349,9 +379,6 @@ class cycdeser : cycdeserbase
   {
     deserializeA(x.data(), x.size());
   }
-
-private:
-  size_t lim;   // ignored for now ... better provide correct input
 };
 
 class cycprint : cycdeserbase
@@ -388,6 +415,7 @@ class cycprint : cycdeserbase
 #define PRNT8(T, F) PRNT(T, F, )
 #define PRNT(T, F, fn_swap) inline void print(T & x) { \
     align(sizeof(x)); \
+    validate_size(1, sizeof(x)); \
     x = *reinterpret_cast<const T *>(data + pos); \
     if (swap_bytes) {x = fn_swap(x);} \
     prtf(&buf, &bufsize, F, x); \
@@ -413,6 +441,7 @@ class cycprint : cycdeserbase
   {
     union { uint32_t u; float f; } tmp;
     align(sizeof(x));
+    validate_size(1, sizeof(x));
     tmp.u = *reinterpret_cast<const uint32_t *>(data + pos);
     if (swap_bytes) {tmp.u = bswap4u(tmp.u);}
     static_cast<void>(tmp.u);
@@ -423,40 +452,46 @@ class cycprint : cycdeserbase
   {
     union { uint64_t u; double f; } tmp;
     align(sizeof(x));
+    validate_size(1, sizeof(x));
     tmp.u = *reinterpret_cast<const uint64_t *>(data + pos);
     if (swap_bytes) {tmp.u = bswap8u(tmp.u);}
     static_cast<void>(tmp.u);
     prtf(&buf, &bufsize, "%f", tmp.f);
     pos += sizeof(x);
   }
-  inline uint32_t get32()
+  inline uint32_t get_len(size_t el_sz)
   {
     uint32_t sz;
     align(sizeof(sz));
+    validate_size(1, sizeof(sz));
     sz = *reinterpret_cast<const uint32_t *>(data + pos);
     if (swap_bytes) {sz = bswap4u(sz);}
     pos += sizeof(sz);
+    validate_size(sz, el_sz);
     return sz;
   }
   inline void print(char * & x)
   {
-    const uint32_t sz = get32();
+    const uint32_t sz = get_len(sizeof(char));
+    validate_str(sz);
     const int len = (sz == 0) ? 0 : (sz > INT32_MAX) ? INT32_MAX : static_cast<int>(sz - 1);
     static_cast<void>(x);
     prtf(&buf, &bufsize, "\"%*.*s\"", len, len, static_cast<const char *>(data + pos));
     pos += sz;
   }
   inline void print(std::string & x)
   {
-    const uint32_t sz = get32();
+    const uint32_t sz = get_len(sizeof(char));
+    validate_str(sz);
     const int len = (sz == 0) ? 0 : (sz > INT32_MAX) ? INT32_MAX : static_cast<int>(sz - 1);
     static_cast<void>(x);
     prtf(&buf, &bufsize, "\"%*.*s\"", len, len, static_cast<const char *>(data + pos));
     pos += sz;
   }
   inline void print(std::wstring & x)
   {
-    const uint32_t sz = get32();
+    const uint32_t sz = get_len(sizeof(wchar_t));
+    // wstring is not null-terminated in cdr
     x = std::wstring(reinterpret_cast<const wchar_t *>(data + pos), sz);
     prtf(&buf, &bufsize, "\"%ls\"", x.c_str());
     pos += sz * sizeof(wchar_t);
@@ -476,7 +511,7 @@ class cycprint : cycdeserbase
   template<class T>
   inline void print(std::vector<T> & x)
   {
-    const uint32_t sz = get32();
+    const uint32_t sz = get_len(1);
     printA(x.data(), sz);
   }
   template<class T, size_t S>