Add XML parser bit_bound bounds check (#3975) (#3989)

* Refs #19354: Add XML parser bit_bound bounds check Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Refs #19354: Add regression test Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Refs #19354: Check empty name Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Refs #19354: Apply rev suggestion Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> --------- Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> (cherry picked from commit e318c5b) Co-authored-by: Jesús Poderoso <120394830+JesusPoderoso@users.noreply.github.com>
eProsima · Nov 13, 2023 · 12bb5b3 · 12bb5b3
1 parent a86b56d
commit 12bb5b3
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 2 deletions.
diff --git a/src/cpp/rtps/xmlparser/XMLDynamicParser.cpp b/src/cpp/rtps/xmlparser/XMLDynamicParser.cpp
@@ -616,11 +616,16 @@ XMLP_ret XMLParser::parseXMLBitmaskDynamicType(
     const char* anno_bit_bound = p_root->Attribute(BIT_BOUND);
     if (anno_bit_bound != nullptr)
     {
-        bit_bound = static_cast<uint16_t>(std::atoi(anno_bit_bound));
+        auto input_bit_bound = std::atoi(anno_bit_bound);
+        if (input_bit_bound < 1 || input_bit_bound > 64)
+        {
+            return XMLP_ret::XML_ERROR;
+        }
+        bit_bound = static_cast<uint16_t>(input_bit_bound);
     }
 
     const char* name = p_root->Attribute(NAME);
-    if (nullptr == name)
+    if (nullptr == name || name[0] == '\0')
     {
         return XMLP_ret::XML_ERROR;
     }

diff --git a/test/unittest/xmlparser/XMLParserTests.cpp b/test/unittest/xmlparser/XMLParserTests.cpp
@@ -61,6 +61,8 @@ TEST_F(XMLParserTests, regressions)
     EXPECT_EQ(XMLP_ret::XML_ERROR, XMLParser::loadXML("regressions/14456_profile_bin.xml", root));
     EXPECT_EQ(XMLP_ret::XML_ERROR, XMLParser::loadXML("regressions/15344_profile_bin.xml", root));
     EXPECT_EQ(XMLP_ret::XML_ERROR, XMLParser::loadXML("regressions/18395_profile_bin.xml", root));
+    EXPECT_EQ(XMLP_ret::XML_ERROR, XMLParser::loadXML("regressions/19354_profile_bin.xml", root));
+    EXPECT_EQ(XMLP_ret::XML_ERROR, XMLParser::loadXML("regressions/19354_2_profile_bin.xml", root));
     EXPECT_EQ(XMLP_ret::XML_ERROR, XMLParser::loadXML("regressions/simple_participant_profiles_nok.xml", root));
     EXPECT_EQ(XMLP_ret::XML_OK, XMLParser::loadXML("regressions/simple_participant_profiles_ok.xml", root));
 }

diff --git a/test/unittest/xmlparser/regressions/19354_2_profile_bin.xml b/test/unittest/xmlparser/regressions/19354_2_profile_bin.xml
@@ -0,0 +1 @@
+<types><type><bitmask bit_bound="2" name=""/></type></types>
diff --git a/test/unittest/xmlparser/regressions/19354_profile_bin.xml b/test/unittest/xmlparser/regressions/19354_profile_bin.xml
@@ -0,0 +1 @@
+<types><type><bitmask bit_bound="-2" name=""/></type></types>
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		<types><type><bitmask bit_bound="2" name=""/></type></types>
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		<types><type><bitmask bit_bound="-2" name=""/></type></types>