From e318c5b99f476f6cfe9224515d9d96e91d6d588d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Poderoso?= <120394830+JesusPoderoso@users.noreply.github.com> Date: Fri, 3 Nov 2023 22:45:15 +0100 Subject: [PATCH] Add XML parser bit_bound bounds check (#3975) * Refs #19354: Add XML parser bit_bound bounds check Signed-off-by: JesusPoderoso * Refs #19354: Add regression test Signed-off-by: JesusPoderoso * Refs #19354: Check empty name Signed-off-by: JesusPoderoso * Refs #19354: Apply rev suggestion Signed-off-by: JesusPoderoso --------- Signed-off-by: JesusPoderoso --- src/cpp/rtps/xmlparser/XMLDynamicParser.cpp | 9 +++++++-- test/unittest/xmlparser/XMLParserTests.cpp | 2 ++ .../xmlparser/regressions/19354_2_profile_bin.xml | 1 + .../unittest/xmlparser/regressions/19354_profile_bin.xml | 1 + 4 files changed, 11 insertions(+), 2 deletions(-) create mode 100644 test/unittest/xmlparser/regressions/19354_2_profile_bin.xml create mode 100644 test/unittest/xmlparser/regressions/19354_profile_bin.xml diff --git a/src/cpp/rtps/xmlparser/XMLDynamicParser.cpp b/src/cpp/rtps/xmlparser/XMLDynamicParser.cpp index 7529f39678e..e1e173feb95 100644 --- 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(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(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 index bc24dbdf314..dfe1eba3b12 100644 --- 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 new file mode 100644 index 00000000000..b99632a5565 --- /dev/null +++ b/test/unittest/xmlparser/regressions/19354_2_profile_bin.xml @@ -0,0 +1 @@ + diff --git a/test/unittest/xmlparser/regressions/19354_profile_bin.xml b/test/unittest/xmlparser/regressions/19354_profile_bin.xml new file mode 100644 index 00000000000..30504474872 --- /dev/null +++ b/test/unittest/xmlparser/regressions/19354_profile_bin.xml @@ -0,0 +1 @@ +