Reject unterminated CDR strings during deserialization

src/cpp/Cdr.cpp

@@ -1639,13 +1639,17 @@ Cdr& Cdr::deserialize(
     }
     else if ((end_ - offset_) >= length)
     {
+        if ((&offset_)[length - 1] != '\0')
+        {
+            set_state(state_before_error);
+            throw BadParamException("The deserialized string is not null-terminated");
+        }
+
         // Save last datasize.
         last_data_size_ = sizeof(uint8_t);
 
         // Allocate memory.
-        string_t =
-                reinterpret_cast<char*>(calloc(length + ((&offset_)[length - 1] == '\0' ? 0 : 1),
-                sizeof(char)));
+        string_t = reinterpret_cast<char*>(calloc(length, sizeof(char)));
         memcpy(string_t, &offset_, length);
         offset_ += length;
         return *this;
@@ -1702,11 +1706,14 @@ const char* Cdr::read_string(
         last_data_size_ = sizeof(uint8_t);
 
         ret_value = &offset_;
-        offset_ += length;
-        if (ret_value[length - 1] == '\0')
+        if (ret_value[length - 1] != '\0')
         {
-            --length;
+            set_state(state_before_error);
+            throw BadParamException("The deserialized string is not null-terminated");
         }
+
+        offset_ += length;
+        --length;
         return ret_value;
     }
 

src/cpp/FastCdr.cpp

@@ -387,10 +387,14 @@ FastCdr& FastCdr::deserialize(
     }
     else if ((last_position_ - current_position_) >= length)
     {
+        if ((&current_position_)[length - 1] != '\0')
+        {
+            set_state(state_before_error);
+            throw BadParamException("The deserialized string is not null-terminated");
+        }
+
         // Allocate memory.
-        string_t =
-                reinterpret_cast<char*>(calloc(length + ((&current_position_)[length - 1] == '\0' ? 0 : 1),
-                sizeof(char)));
+        string_t = reinterpret_cast<char*>(calloc(length, sizeof(char)));
         memcpy(string_t, &current_position_, length);
         current_position_ += length;
         return *this;
@@ -453,11 +457,14 @@ const char* FastCdr::read_string(
     else if ((last_position_ - current_position_) >= length)
     {
         ret_value = &current_position_;
-        current_position_ += length;
-        if (ret_value[length - 1] == '\0')
+        if (ret_value[length - 1] != '\0')
         {
-            --length;
+            set_state(state_before_error);
+            throw BadParamException("The deserialized string is not null-terminated");
         }
+
+        current_position_ += length;
+        --length;
         return ret_value;
     }
 

test/cdr/SimpleTest.cpp

@@ -2311,7 +2311,7 @@ TEST(CDRTests, DeserializeIntoANonEmptyMapInXCDRv1)
         0x00, 0x00, 0x00, 0x01,  // Map length
         0x00, 0x02,              // Key
         0x00, 0x00,              // Alignment
-        0x00, 0x00, 0x00, 0x01,  // Length
+        0x00, 0x00, 0x00, 0x02,  // Length, including the null terminator
         65,  0x00                // 'A'
     };
 
@@ -7176,6 +7176,66 @@ TEST(FastCDRTests, StringWithNullChars)
         BadParamException);
 }
 
+TEST(CDRTests, UnterminatedStringDeserialization)
+{
+    char buffer[] = {0x05, 0x00, 0x00, 0x00, 'c', 'r', 'h', 'y', 'v'};
+    FastBuffer cdrbuffer(buffer, sizeof(buffer));
+    Cdr cdr_des(cdrbuffer, Cdr::LITTLE_ENDIANNESS);
+    std::string value;
+
+    EXPECT_THROW(cdr_des >> value, BadParamException);
+    EXPECT_EQ(cdr_des.get_serialized_data_length(), 0u);
+}
+
+TEST(FastCDRTests, UnterminatedStringDeserialization)
+{
+    char buffer[sizeof(uint32_t) + 5] = {};
+    FastBuffer cdrbuffer(buffer, sizeof(buffer));
+    FastCdr cdr_ser(cdrbuffer);
+    cdr_ser << static_cast<uint32_t>(5);
+    buffer[4] = 'c';
+    buffer[5] = 'r';
+    buffer[6] = 'h';
+    buffer[7] = 'y';
+    buffer[8] = 'v';
+    FastCdr cdr_des(cdrbuffer);
+    std::string value;
+
+    EXPECT_THROW(cdr_des >> value, BadParamException);
+    EXPECT_EQ(cdr_des.get_serialized_data_length(), 0u);
+}
+
+TEST(CDRTests, UnterminatedCStringDeserialization)
+{
+    char buffer[] = {0x05, 0x00, 0x00, 0x00, 'c', 'r', 'h', 'y', 'v'};
+    FastBuffer cdrbuffer(buffer, sizeof(buffer));
+    Cdr cdr_des(cdrbuffer, Cdr::LITTLE_ENDIANNESS);
+    char* value = nullptr;
+
+    EXPECT_THROW(cdr_des >> value, BadParamException);
+    EXPECT_EQ(value, nullptr);
+    EXPECT_EQ(cdr_des.get_serialized_data_length(), 0u);
+}
+
+TEST(FastCDRTests, UnterminatedCStringDeserialization)
+{
+    char buffer[sizeof(uint32_t) + 5] = {};
+    FastBuffer cdrbuffer(buffer, sizeof(buffer));
+    FastCdr cdr_ser(cdrbuffer);
+    cdr_ser << static_cast<uint32_t>(5);
+    buffer[4] = 'c';
+    buffer[5] = 'r';
+    buffer[6] = 'h';
+    buffer[7] = 'y';
+    buffer[8] = 'v';
+    FastCdr cdr_des(cdrbuffer);
+    char* value = nullptr;
+
+    EXPECT_THROW(cdr_des >> value, BadParamException);
+    EXPECT_EQ(value, nullptr);
+    EXPECT_EQ(cdr_des.get_serialized_data_length(), 0u);
+}
+
 TEST(CDRTests, EmptyStringSerializationSize)
 {
     std::string str;