Bug#1000608: buster-pu: package ros-ros-comm/1.14.3+ds1-5+deb10u2
Package: release.debian.org
Severity: normal
Tags: buster
User: release.debian.org@packages.debian.org
Usertags: pu
X-Debbugs-Cc: jspricke@debian.org
[ Reason ]
CVE-2021-37146 was published with a denial of service against
ros-ros-comm.
[ Impact ]
The impact is rather low as the ROS middleware has no authentication nor
security features implemented and should only be used behind a firewall.
Still would be good to get it fixed in old-stable.
[ Tests ]
The patch adds a unit test and I ran manual tests using the relay
command from the topic-tools package.
[ Risks ]
Except for one new method (nextTagData) I see the code as rather simple,
and the risk as low.
For nextTagData the difference is that it is more strict in parsing only
the next xml tag which should be fine in the defined domain. Also this
is part of the upstream releases and also in unstable since some time.
[ Checklist ]
[X] *all* changes are documented in the d/changelog
[X] I reviewed all changes and I approve them
[X] attach debdiff against the package in (old)stable
[X] the issue is verified as fixed in unstable
[ Changes ]
The patches add three things:
- Null pointer checks in XmlRpc.
- Add and update unit tests for the new changes.
- A new nextTagData method. This is an improved version of the old
parseTag version. Both methods extract the data inside of a given xml
tag in a string. The old parseTag used find to search for the
requested tag. The new nextTagData only allows space characters in
front of the expected xml tag.
[ Other info ]
I kept the individual patches as upstream merged them, hope that is
fine.
>From 1e0c5a384e036b2b4ee513c3f8514de3a8f77c9f Mon Sep 17 00:00:00 2001
From: Jochen Sprickerhof <git@jochen.sprickerhof.de>
Date: Wed, 20 Oct 2021 21:44:38 +0200
Subject: [PATCH] 1.14.3+ds1-5+deb10u3 (CVE-2021-37146)
---
debian/changelog | 6 +
.../0015-Fix-oversize-string-test.patch | 25 +
...fensive-checks-for-offset-being-NULL.patch | 45 ++
...-tests-for-XML-tag-utility-functions.patch | 653 ++++++++++++++++++
...18-Add-implementation-of-nextTagData.patch | 167 +++++
...h-structFromXml-to-using-nextTagData.patch | 22 +
debian/patches/series | 5 +
7 files changed, 923 insertions(+)
create mode 100644 debian/patches/0015-Fix-oversize-string-test.patch
create mode 100644 debian/patches/0016-Add-defensive-checks-for-offset-being-NULL.patch
create mode 100644 debian/patches/0017-Add-unit-tests-for-XML-tag-utility-functions.patch
create mode 100644 debian/patches/0018-Add-implementation-of-nextTagData.patch
create mode 100644 debian/patches/0019-Switch-structFromXml-to-using-nextTagData.patch
diff --git a/debian/changelog b/debian/changelog
index 420c997..c3cc30a 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+ros-ros-comm (1.14.3+ds1-5+deb10u3) buster; urgency=medium
+
+ * Add https://github.com/ros/ros_comm/pull/2186 (Fix CVE-2021-37146)
+
+ -- Jochen Sprickerhof <jspricke@debian.org> Wed, 20 Oct 2021 21:43:47 +0200
+
ros-ros-comm (1.14.3+ds1-5+deb10u2) buster; urgency=high
* Add https://github.com/ros/ros_comm/pull/2065 (Fix CVE-2020-16124)
diff --git a/debian/patches/0015-Fix-oversize-string-test.patch b/debian/patches/0015-Fix-oversize-string-test.patch
new file mode 100644
index 0000000..489b651
--- /dev/null
+++ b/debian/patches/0015-Fix-oversize-string-test.patch
@@ -0,0 +1,25 @@
+From: Chris Lalancette <clalancette@openrobotics.org>
+Date: Wed, 7 Jul 2021 14:34:14 +0000
+Subject: Fix oversize string test.
+
+It claims to be "well-formed", but the closing tag was wrong.
+Fix that here.
+
+Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
+---
+ utilities/xmlrpcpp/test/TestValues.cpp | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/utilities/xmlrpcpp/test/TestValues.cpp b/utilities/xmlrpcpp/test/TestValues.cpp
+index acd79c2..48730fd 100644
+--- a/utilities/xmlrpcpp/test/TestValues.cpp
++++ b/utilities/xmlrpcpp/test/TestValues.cpp
+@@ -180,7 +180,7 @@ TEST(XmlRpc, testOversizeString) {
+ try {
+ std::string xml = "<tag><nexttag>";
+ xml += std::string(__INT_MAX__, 'a');
+- xml += "a</nextag></tag>";
++ xml += "a</nexttag></tag>";
+ int offset;
+
+ offset = 0;
diff --git a/debian/patches/0016-Add-defensive-checks-for-offset-being-NULL.patch b/debian/patches/0016-Add-defensive-checks-for-offset-being-NULL.patch
new file mode 100644
index 0000000..b0e024b
--- /dev/null
+++ b/debian/patches/0016-Add-defensive-checks-for-offset-being-NULL.patch
@@ -0,0 +1,45 @@
+From: Chris Lalancette <clalancette@openrobotics.org>
+Date: Wed, 7 Jul 2021 17:23:39 +0000
+Subject: Add defensive checks for offset being NULL.
+
+Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
+---
+ utilities/xmlrpcpp/src/XmlRpcUtil.cpp | 4 ++++
+ 1 file changed, 4 insertions(+)
+
+diff --git a/utilities/xmlrpcpp/src/XmlRpcUtil.cpp b/utilities/xmlrpcpp/src/XmlRpcUtil.cpp
+index ab0991d..a964b94 100644
+--- a/utilities/xmlrpcpp/src/XmlRpcUtil.cpp
++++ b/utilities/xmlrpcpp/src/XmlRpcUtil.cpp
+@@ -108,6 +108,7 @@ void XmlRpcUtil::error(const char* fmt, ...)
+ std::string
+ XmlRpcUtil::parseTag(const char* tag, std::string const& xml, int* offset)
+ {
++ if (offset == NULL) return std::string();
+ // avoid attempting to parse overly long xml input
+ if (xml.length() > size_t(__INT_MAX__)) return std::string();
+ if (*offset >= int(xml.length())) return std::string();
+@@ -128,6 +129,7 @@ XmlRpcUtil::parseTag(const char* tag, std::string const& xml, int* offset)
+ bool
+ XmlRpcUtil::findTag(const char* tag, std::string const& xml, int* offset)
+ {
++ if (offset == NULL) return false;
+ if (xml.length() > size_t(__INT_MAX__)) return false;
+ if (*offset >= int(xml.length())) return false;
+ size_t istart = xml.find(tag, *offset);
+@@ -144,6 +146,7 @@ XmlRpcUtil::findTag(const char* tag, std::string const& xml, int* offset)
+ bool
+ XmlRpcUtil::nextTagIs(const char* tag, std::string const& xml, int* offset)
+ {
++ if (offset == NULL) return false;
+ if (xml.length() > size_t(__INT_MAX__)) return false;
+ if (*offset >= int(xml.length())) return false;
+ const char* cp = xml.c_str() + *offset;
+@@ -166,6 +169,7 @@ XmlRpcUtil::nextTagIs(const char* tag, std::string const& xml, int* offset)
+ std::string
+ XmlRpcUtil::getNextTag(std::string const& xml, int* offset)
+ {
++ if (offset == NULL) return std::string();
+ if (xml.length() > size_t(__INT_MAX__)) return std::string();
+ if (*offset >= int(xml.length())) return std::string();
+
diff --git a/debian/patches/0017-Add-unit-tests-for-XML-tag-utility-functions.patch b/debian/patches/0017-Add-unit-tests-for-XML-tag-utility-functions.patch
new file mode 100644
index 0000000..3e31739
--- /dev/null
+++ b/debian/patches/0017-Add-unit-tests-for-XML-tag-utility-functions.patch
@@ -0,0 +1,653 @@
+From: Chris Lalancette <clalancette@openrobotics.org>
+Date: Wed, 7 Jul 2021 17:23:55 +0000
+Subject: Add unit tests for XML tag utility functions.
+
+This includes parseTag, findTag, nextTagIs, and getNextTag.
+
+Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
+---
+ utilities/xmlrpcpp/test/TestValues.cpp | 624 +++++++++++++++++++++++++++++++++
+ 1 file changed, 624 insertions(+)
+
+diff --git a/utilities/xmlrpcpp/test/TestValues.cpp b/utilities/xmlrpcpp/test/TestValues.cpp
+index 48730fd..5d22c1e 100644
+--- a/utilities/xmlrpcpp/test/TestValues.cpp
++++ b/utilities/xmlrpcpp/test/TestValues.cpp
+@@ -208,6 +208,129 @@ TEST(XmlRpc, testOversizeString) {
+ }
+ }
+
++TEST(XmlRpc, testParseTag) {
++ int offset = 0;
++
++ // Test a null tag
++ EXPECT_EQ(XmlRpcUtil::parseTag(NULL, "", &offset), std::string());
++ EXPECT_EQ(offset, 0);
++
++ // Test a null offset
++ EXPECT_EQ(XmlRpcUtil::parseTag("<tag>", "", NULL), std::string());
++ EXPECT_EQ(offset, 0);
++
++ // Test if the offset is beyond the end of the input xml
++ offset = 20;
++ EXPECT_EQ(XmlRpcUtil::parseTag("<tag>", "", &offset), std::string());
++ EXPECT_EQ(offset, 20);
++
++ // Test if the tag is not found in the input xml
++ offset = 0;
++ EXPECT_EQ(XmlRpcUtil::parseTag("<tag>", "<foo></foo>", &offset), std::string());
++ EXPECT_EQ(offset, 0);
++
++ // Test if the tag is found, but the end tag is not
++ EXPECT_EQ(XmlRpcUtil::parseTag("<tag>", "<tag>", &offset), std::string());
++ EXPECT_EQ(offset, 0);
++
++ // Test if the tag is found, the end tag is found, and there is a value in the middle
++ EXPECT_EQ(XmlRpcUtil::parseTag("<tag>", "<tag>foo</tag>", &offset), "foo");
++ EXPECT_EQ(offset, 14);
++}
++
++TEST(XmlRpc, testFindTag) {
++ int offset = 0;
++
++ // Test a null tag
++ EXPECT_FALSE(XmlRpcUtil::findTag(NULL, "", &offset));
++ EXPECT_EQ(offset, 0);
++
++ // Test a null offset
++ EXPECT_FALSE(XmlRpcUtil::findTag("<tag>", "", NULL));
++ EXPECT_EQ(offset, 0);
++
++ // Test if the offset is beyond the end of the input xml
++ offset = 20;
++ EXPECT_FALSE(XmlRpcUtil::findTag("<tag>", "", &offset));
++ EXPECT_EQ(offset, 20);
++
++ // Test that the offset moves when finding a tag
++ offset = 0;
++ EXPECT_TRUE(XmlRpcUtil::findTag("<subtag>", "<tag><subtag></subtag></tag>", &offset));
++ EXPECT_EQ(offset, 13);
++}
++
++TEST(XmlRpc, testNextTagIs) {
++ int offset = 0;
++
++ // Test a null tag
++ EXPECT_FALSE(XmlRpcUtil::nextTagIs(NULL, "", &offset));
++ EXPECT_EQ(offset, 0);
++
++ // Test a null offset
++ EXPECT_FALSE(XmlRpcUtil::nextTagIs("<tag>", "", NULL));
++ EXPECT_EQ(offset, 0);
++
++ // Test if the offset is beyond the end of the input xml
++ offset = 20;
++ EXPECT_FALSE(XmlRpcUtil::nextTagIs("<tag>", "", &offset));
++ EXPECT_EQ(offset, 20);
++
++ // Test that the offset moves when finding a tag with no whitespace
++ offset = 0;
++ EXPECT_TRUE(XmlRpcUtil::nextTagIs("<tag>", "<tag></tag>", &offset));
++ EXPECT_EQ(offset, 5);
++
++ // Test that the offset moves when finding a tag with whitespace
++ offset = 0;
++ EXPECT_TRUE(XmlRpcUtil::nextTagIs("<tag>", " <tag></tag>", &offset));
++ EXPECT_EQ(offset, 11);
++
++ // Test that the offset doesn't move when the tag is not found
++ offset = 0;
++ EXPECT_FALSE(XmlRpcUtil::nextTagIs("<tag>", " <footag></footag>", &offset));
++ EXPECT_EQ(offset, 0);
++}
++
++TEST(XmlRpc, testGetNextTag) {
++ int offset = 0;
++
++ // Test a null offset
++ EXPECT_EQ(XmlRpcUtil::getNextTag("", NULL), std::string());
++ EXPECT_EQ(offset, 0);
++
++ // Test if the offset is beyond the end of the input xml
++ offset = 20;
++ EXPECT_EQ(XmlRpcUtil::getNextTag("<tag>", &offset), std::string());
++ EXPECT_EQ(offset, 20);
++
++ // Test that the offset moves when finding a tag with no whitespace
++ offset = 0;
++ EXPECT_EQ(XmlRpcUtil::getNextTag("<tag></tag>", &offset), "<tag>");
++ EXPECT_EQ(offset, 5);
++
++ // Test that the offset moves when finding a tag with whitespace
++ offset = 0;
++ EXPECT_EQ(XmlRpcUtil::getNextTag(" <tag></tag>", &offset), "<tag>");
++ EXPECT_EQ(offset, 11);
++
++ // Test that the offset doesn't move if there are no tags
++ offset = 0;
++ EXPECT_EQ(XmlRpcUtil::getNextTag(" foo", &offset), std::string());
++ EXPECT_EQ(offset, 0);
++
++ // Test that the offset moves if there is a start < but no end >
++ offset = 0;
++ // FIXME: this should fail, but currently does not
++ EXPECT_EQ(XmlRpcUtil::getNextTag("<foo", &offset), "<foo");
++ EXPECT_EQ(offset, 4);
++
++ // Test what happens if there is no data in the tag
++ offset = 0;
++ EXPECT_EQ(XmlRpcUtil::getNextTag("<>", &offset), "<>");
++ EXPECT_EQ(offset, 2);
++}
++
+ TEST(XmlRpc, testDateTime) {
+ // DateTime
+ int offset = 0;
+@@ -546,6 +669,507 @@ TEST(XmlRpc, array_errors) {
+ EXPECT_THROW(ref[2], XmlRpcException);
+ }
+
++TEST(XmlRpc, fromXmlInvalid) {
++ int offset = 0;
++ XmlRpcValue val;
++
++ // Test what happens with a null offset
++ val.fromXml("", NULL);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInvalid);
++
++ // Test what happens with an offset far beyond the xml
++ offset = 20;
++ val.fromXml("", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInvalid);
++ EXPECT_EQ(offset, 20);
++
++ // Test what happens with no <value> tag
++ offset = 0;
++ val.fromXml("<foo>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInvalid);
++ EXPECT_EQ(offset, 0);
++
++ // Test what happens with <value> tag but nothing else
++ offset = 0;
++ val.fromXml("<value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInvalid);
++ EXPECT_EQ(offset, 0);
++
++ // A <value><invalid></invalid></value> is invalid
++ offset = 0;
++ val.fromXml("<value><invalid></invalid></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInvalid);
++ EXPECT_EQ(offset, 0);
++
++ // A <value></value> combination is an implicit empty string
++ offset = 0;
++ val.fromXml("<value></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeString);
++ EXPECT_EQ(offset, 15);
++ EXPECT_EQ(static_cast<std::string>(val), "");
++}
++
++TEST(XmlRpc, fromXmlBoolean) {
++ int offset = 0;
++ XmlRpcValue val;
++
++ // A <value><boolean></boolean></value> is invalid
++ offset = 0;
++ val.fromXml("<value><boolean></boolean></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInvalid);
++ EXPECT_EQ(offset, 0);
++
++ // A <value><boolean></value> is invalid
++ offset = 0;
++ val.fromXml("<value><boolean></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInvalid);
++ EXPECT_EQ(offset, 0);
++
++ // A <value><boolean>foo</boolean></value> is invalid
++ offset = 0;
++ val.fromXml("<value><boolean>foo</boolean></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInvalid);
++ EXPECT_EQ(offset, 0);
++
++ // A <value><boolean>25</boolean></value> is invalid
++ offset = 0;
++ val.fromXml("<value><boolean>25</boolean></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInvalid);
++ EXPECT_EQ(offset, 0);
++
++ // A <value><boolean>1foo</boolean></value> is valid
++ offset = 0;
++ // FIXME: this should fail, but currently does not
++ val.fromXml("<value><boolean>1foo</boolean></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeBoolean);
++ EXPECT_EQ(offset, 38);
++ EXPECT_EQ(static_cast<bool>(val), true);
++
++ // A <value><boolean>1</value> is valid
++ offset = 0;
++ // FIXME: this should fail, but currently does not
++ val.fromXml("<value><boolean>1</value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeBoolean);
++ EXPECT_EQ(offset, 25);
++ EXPECT_EQ(static_cast<bool>(val), true);
++
++ // A <value><boolean>0</boolean></value> is valid
++ offset = 0;
++ val.fromXml("<value><boolean>0</boolean></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeBoolean);
++ EXPECT_EQ(offset, 35);
++ EXPECT_EQ(static_cast<bool>(val), false);
++
++ // A <value><boolean>1</boolean></value> is valid
++ offset = 0;
++ val.fromXml("<value><boolean>1</boolean></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeBoolean);
++ EXPECT_EQ(offset, 35);
++ EXPECT_EQ(static_cast<bool>(val), true);
++}
++
++TEST(XmlRpc, fromXmlI4) {
++ int offset = 0;
++ XmlRpcValue val;
++
++ // A <value><i4></i4></value> is invalid
++ offset = 0;
++ val.fromXml("<value><i4></i4></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInvalid);
++ EXPECT_EQ(offset, 0);
++
++ // A <value><i4></value> is invalid
++ offset = 0;
++ val.fromXml("<value><i4></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInvalid);
++ EXPECT_EQ(offset, 0);
++
++ // A <value><i4>foo</i4></value> is invalid
++ offset = 0;
++ val.fromXml("<value><i4>foo</i4></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInvalid);
++ EXPECT_EQ(offset, 0);
++
++ // A <value><i4>25</i4></value> is valid
++ offset = 0;
++ val.fromXml("<value><i4>25</i4></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInt);
++ EXPECT_EQ(offset, 26);
++ EXPECT_EQ(static_cast<int>(val), 25);
++
++ // A <value><i4>1foo</i4></value> is valid
++ offset = 0;
++ // FIXME: this should fail, but currently does not
++ val.fromXml("<value><i4>1foo</i4></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInt);
++ EXPECT_EQ(offset, 28);
++ EXPECT_EQ(static_cast<int>(val), 1);
++
++ // A <value><i4>1</value> is valid
++ offset = 0;
++ // FIXME: this should fail, but currently does not
++ val.fromXml("<value><i4>99</value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInt);
++ EXPECT_EQ(offset, 21);
++ EXPECT_EQ(static_cast<int>(val), 99);
++}
++
++TEST(XmlRpc, fromXmlInt) {
++ int offset = 0;
++ XmlRpcValue val;
++
++ // A <value><int></int></value> is invalid
++ offset = 0;
++ val.fromXml("<value><int></int></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInvalid);
++ EXPECT_EQ(offset, 0);
++
++ // A <value><int></value> is invalid
++ offset = 0;
++ val.fromXml("<value><int></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInvalid);
++ EXPECT_EQ(offset, 0);
++
++ // A <value><int>foo</int></value> is invalid
++ offset = 0;
++ val.fromXml("<value><int>foo</int></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInvalid);
++ EXPECT_EQ(offset, 0);
++
++ // A <value><int>25</int></value> is valid
++ offset = 0;
++ val.fromXml("<value><int>25</int></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInt);
++ EXPECT_EQ(offset, 28);
++ EXPECT_EQ(static_cast<int>(val), 25);
++
++ // A <value><int>1foo</int></value> is valid
++ offset = 0;
++ // FIXME: this should fail, but currently does not
++ val.fromXml("<value><int>1foo</int></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInt);
++ EXPECT_EQ(offset, 30);
++ EXPECT_EQ(static_cast<int>(val), 1);
++
++ // A <value><int>1</value> is valid
++ offset = 0;
++ // FIXME: this should fail, but currently does not
++ val.fromXml("<value><int>99</value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInt);
++ EXPECT_EQ(offset, 22);
++ EXPECT_EQ(static_cast<int>(val), 99);
++}
++
++TEST(XmlRpc, fromXmlDouble) {
++ int offset = 0;
++ XmlRpcValue val;
++
++ // A <value><double></double></value> is invalid
++ offset = 0;
++ val.fromXml("<value><double></double></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInvalid);
++ EXPECT_EQ(offset, 0);
++
++ // A <value><double></value> is invalid
++ offset = 0;
++ val.fromXml("<value><double></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInvalid);
++ EXPECT_EQ(offset, 0);
++
++ // A <value><double>foo</double></value> is invalid
++ offset = 0;
++ val.fromXml("<value><double>foo</double></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInvalid);
++ EXPECT_EQ(offset, 0);
++
++ // A <value><double>25</double></value> is valid
++ offset = 0;
++ val.fromXml("<value><double>25</double></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeDouble);
++ EXPECT_EQ(offset, 34);
++ EXPECT_EQ(static_cast<double>(val), 25.0);
++
++ // A <value><double>25.876</double></value> is valid
++ offset = 0;
++ val.fromXml("<value><double>25.876</double></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeDouble);
++ EXPECT_EQ(offset, 38);
++ EXPECT_NEAR(static_cast<double>(val), 25.876, 0.01);
++
++ // A <value><double>1foo</double></value> is valid
++ offset = 0;
++ // FIXME: this should fail, but currently does not
++ val.fromXml("<value><double>1foo</double></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeDouble);
++ EXPECT_EQ(offset, 36);
++ EXPECT_EQ(static_cast<double>(val), 1);
++}
++
++TEST(XmlRpc, fromXmlImplicitString) {
++ int offset = 0;
++ XmlRpcValue val;
++
++ // A <value><foo></foo></value> is invalid
++ offset = 0;
++ val.fromXml("<value><foo></foo></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInvalid);
++ EXPECT_EQ(offset, 0);
++
++ // A <value>foo</value> is valid
++ offset = 0;
++ val.fromXml("<value>foo</value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeString);
++ EXPECT_EQ(offset, 18);
++ EXPECT_EQ(static_cast<std::string>(val), "foo");
++ EXPECT_EQ(val.size(), 3);
++}
++
++TEST(XmlRpc, fromXmlExplicitString) {
++ int offset = 0;
++ XmlRpcValue val;
++
++ // A <value><string> is invalid
++ offset = 0;
++ val.fromXml("<value><string>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInvalid);
++ EXPECT_EQ(offset, 0);
++
++ // A <value><string></value> is valid
++ offset = 0;
++ // FIXME: this should fail, but currently does not
++ val.fromXml("<value><string></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeString);
++ EXPECT_EQ(offset, 23);
++ EXPECT_EQ(static_cast<std::string>(val), "");
++ EXPECT_EQ(val.size(), 0);
++
++ // A <value><string>foo</string></value> is valid
++ offset = 0;
++ val.fromXml("<value><string>foo</string></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeString);
++ EXPECT_EQ(offset, 35);
++ EXPECT_EQ(static_cast<std::string>(val), "foo");
++ EXPECT_EQ(val.size(), 3);
++}
++
++TEST(XmlRpc, fromXmlDateTime) {
++ int offset = 0;
++ XmlRpcValue val;
++ struct tm expected{};
++ struct tm returned;
++
++ // A <value><dateTime.iso8601></dateTime.iso8601></value> is invalid
++ offset = 0;
++ val.fromXml("<value><dateTime.iso8601></dateTime.iso8601></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInvalid);
++ EXPECT_EQ(offset, 0);
++
++ // A <value><dateTime.iso8601> is invalid
++ offset = 0;
++ val.fromXml("<value><dateTime.iso8601>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInvalid);
++ EXPECT_EQ(offset, 0);
++
++ // A <value><dateTime.iso8601></value> is invalid
++ offset = 0;
++ val.fromXml("<value><dateTime.iso8601></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInvalid);
++ EXPECT_EQ(offset, 0);
++
++ // A <value><dateTime.iso8601>0000000T00:00<dateTime.iso8601></value> is invalid
++ offset = 0;
++ val.fromXml("<value><dateTime.iso8601>0000000T00:00<dateTime.iso8601></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInvalid);
++ EXPECT_EQ(offset, 0);
++
++ // A <value><dateTime.iso8601>000000T00:00:00<dateTime.iso8601></value> is invalid
++ offset = 0;
++ val.fromXml("<value><dateTime.iso8601>000000T00:00:00<dateTime.iso8601></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInvalid);
++ EXPECT_EQ(offset, 0);
++
++ // A <value><dateTime.iso8601>0000000T00:00:00</value> is valid
++ offset = 0;
++ // FIXME: this should fail, but currently does not
++ // FIXME: this currently leaves the returned struct tm fields 'tm_wday' and 'tm_yday' uninitialized
++ val.fromXml("<value><dateTime.iso8601>0000000T00:00:00</value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeDateTime);
++ EXPECT_EQ(offset, 49);
++ returned = static_cast<struct tm>(val);
++ EXPECT_EQ(returned.tm_sec, expected.tm_sec);
++ EXPECT_EQ(returned.tm_min, expected.tm_min);
++ EXPECT_EQ(returned.tm_hour, expected.tm_hour);
++ EXPECT_EQ(returned.tm_mday, expected.tm_mday);
++ EXPECT_EQ(returned.tm_mon, expected.tm_mon);
++ EXPECT_EQ(returned.tm_year, expected.tm_year);
++ EXPECT_EQ(returned.tm_isdst, -1);
++
++ // A <value><dateTime.iso8601>0000000T00:00:0<dateTime.iso8601></value> is valid
++ offset = 0;
++ // FIXME: this should fail, but currently does not
++ val.fromXml("<value><dateTime.iso8601>0000000T00:00:0<dateTime.iso8601></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeDateTime);
++ EXPECT_EQ(offset, 66);
++ returned = static_cast<struct tm>(val);
++ EXPECT_EQ(returned.tm_sec, expected.tm_sec);
++ EXPECT_EQ(returned.tm_min, expected.tm_min);
++ EXPECT_EQ(returned.tm_hour, expected.tm_hour);
++ EXPECT_EQ(returned.tm_mday, expected.tm_mday);
++ EXPECT_EQ(returned.tm_mon, expected.tm_mon);
++ EXPECT_EQ(returned.tm_year, expected.tm_year);
++ EXPECT_EQ(returned.tm_isdst, -1);
++
++ // A <value><dateTime.iso8601>0000000T00:00:00<dateTime.iso8601></value> is valid
++ offset = 0;
++ // FIXME: this currently leaves the returned struct tm fields 'tm_wday' and 'tm_yday' uninitialized
++ val.fromXml("<value><dateTime.iso8601>0000000T00:00:00<dateTime.iso8601></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeDateTime);
++ EXPECT_EQ(offset, 67);
++ returned = static_cast<struct tm>(val);
++ EXPECT_EQ(returned.tm_sec, expected.tm_sec);
++ EXPECT_EQ(returned.tm_min, expected.tm_min);
++ EXPECT_EQ(returned.tm_hour, expected.tm_hour);
++ EXPECT_EQ(returned.tm_mday, expected.tm_mday);
++ EXPECT_EQ(returned.tm_mon, expected.tm_mon);
++ EXPECT_EQ(returned.tm_year, expected.tm_year);
++ EXPECT_EQ(returned.tm_isdst, -1);
++}
++
++TEST(XmlRpc, fromXmlBase64) {
++ int offset = 0;
++ XmlRpcValue val;
++
++ // A <value><base64> is invalid
++ offset = 0;
++ val.fromXml("<value><base64>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInvalid);
++ EXPECT_EQ(offset, 0);
++
++ // A <value><base64></base64></value> is valid
++ offset = 0;
++ val.fromXml("<value><base64></base64></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeBase64);
++ EXPECT_EQ(offset, 32);
++ EXPECT_EQ(static_cast<const XmlRpc::XmlRpcValue::BinaryData &>(val), XmlRpc::XmlRpcValue::BinaryData());
++ EXPECT_EQ(val.size(), 0);
++
++ // A <value><base64>____</base64></value> is valid
++ offset = 0;
++ // FIXME: the underscore character is illegal in base64, so this should thrown an error
++ val.fromXml("<value><base64>____</base64></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeBase64);
++ EXPECT_EQ(offset, 36);
++ EXPECT_EQ(static_cast<const XmlRpc::XmlRpcValue::BinaryData &>(val), XmlRpc::XmlRpcValue::BinaryData());
++ EXPECT_EQ(val.size(), 0);
++
++ // A <value><base64>aGVsbG8=</base64></value> is valid
++ XmlRpc::XmlRpcValue::BinaryData expected{'h', 'e', 'l', 'l', 'o'};
++ offset = 0;
++ val.fromXml("<value><base64>aGVsbG8=</base64></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeBase64);
++ EXPECT_EQ(offset, 40);
++ EXPECT_EQ(static_cast<const XmlRpc::XmlRpcValue::BinaryData &>(val), expected);
++ EXPECT_EQ(val.size(), 5);
++}
++
++TEST(XmlRpc, fromXmlArray) {
++ int offset = 0;
++ XmlRpcValue val;
++
++ // A <value><array> is invalid
++ offset = 0;
++ val.fromXml("<value><array>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInvalid);
++ EXPECT_EQ(offset, 0);
++
++ // A <value><array></array></value> is invalid (no <data> tag)
++ offset = 0;
++ val.fromXml("<value><array></array></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeInvalid);
++ EXPECT_EQ(offset, 0);
++
++ // A <value><array><data></data></array></value> is valid
++ offset = 0;
++ val.fromXml("<value><array><data></data></array></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeArray);
++ EXPECT_EQ(offset, 43);
++ EXPECT_EQ(val.size(), 0);
++
++ // A <value><array><data><value><boolean>1</boolean></value></data></array></value> is valid
++ offset = 0;
++ val.fromXml("<value><array><data><value><boolean>1</boolean></value></data></array></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeArray);
++ EXPECT_EQ(offset, 78);
++ EXPECT_EQ(val.size(), 1);
++ EXPECT_EQ(static_cast<bool>(val[0]), true);
++
++ // A <value><array><data><value><boolean>1</boolean></value></array></value> is valid
++ offset = 0;
++ // FIXME: this should fail (missing an end </data>), but currently does not
++ val.fromXml("<value><array><data><value><boolean>1</boolean></value></array></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeArray);
++ EXPECT_EQ(offset, 71);
++ EXPECT_EQ(val.size(), 1);
++ EXPECT_EQ(static_cast<bool>(val[0]), true);
++
++ // A <value><array><data><value><boolean>1</boolean></value><value><double>23.4</double></value></data></array></value> is valid
++ offset = 0;
++ val.fromXml("<value><array><data><value><boolean>1</boolean></value><value><double>23.4</double></value></data></array></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeArray);
++ EXPECT_EQ(offset, 114);
++ EXPECT_EQ(val.size(), 2);
++ EXPECT_EQ(static_cast<bool>(val[0]), true);
++ EXPECT_NEAR(static_cast<double>(val[1]), 23.4, 0.01);
++}
++
++TEST(XmlRpc, fromXmlStruct) {
++ int offset = 0;
++ XmlRpcValue val;
++
++ // A <value><struct> is valid
++ offset = 0;
++ // FIXME: this should fail, but currently does not
++ val.fromXml("<value><struct>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeStruct);
++ EXPECT_EQ(offset, 15);
++ EXPECT_EQ(val.size(), 0);
++
++ // A <value><struct><member><value><boolean>1</value> is valid
++ offset = 0;
++ // FIXME: this should fail (it is missing many end tags and a <name> tag), but currently does not
++ val.fromXml("<value><struct><member><value><boolean>1</value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeStruct);
++ EXPECT_EQ(offset, 48);
++ EXPECT_EQ(val.size(), 1);
++ EXPECT_EQ(static_cast<bool>(val[""]), true);
++
++ // A <value><struct><member><value><boolean>1</boolean></value></member></struct></value> is valid
++ offset = 0;
++ // FIXME: this should fail (it is missing a <name> tag), but currently does not
++ val.fromXml("<value><struct><member><value><boolean>1</boolean></value></member></struct></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeStruct);
++ EXPECT_EQ(offset, 84);
++ EXPECT_EQ(val.size(), 1);
++ EXPECT_EQ(static_cast<bool>(val[""]), true);
++
++ // A <value><struct><member><name></name><value><boolean>1</boolean></value></member></struct></value> is valid
++ offset = 0;
++ // FIXME: this should fail (the name tag is empty), but currently does not
++ val.fromXml("<value><struct><member><name></name><value><boolean>1</boolean></value></member></struct></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeStruct);
++ EXPECT_EQ(offset, 97);
++ EXPECT_EQ(val.size(), 1);
++ EXPECT_EQ(static_cast<bool>(val[""]), true);
++
++ // A <value><struct><member><name>foo</name><value><boolean>1</boolean></value></member></struct></value> is valid
++ offset = 0;
++ val.fromXml("<value><struct><member><name>foo</name><value><boolean>1</boolean></value></member></struct></value>", &offset);
++ EXPECT_EQ(val.getType(), XmlRpcValue::Type::TypeStruct);
++ EXPECT_EQ(offset, 100);
++ EXPECT_EQ(val.size(), 1);
++ EXPECT_EQ(static_cast<bool>(val["foo"]), true);
++}
++
+ int main(int argc, char **argv)
+ {
+ ::testing::InitGoogleTest(&argc, argv);
diff --git a/debian/patches/0018-Add-implementation-of-nextTagData.patch b/debian/patches/0018-Add-implementation-of-nextTagData.patch
new file mode 100644
index 0000000..278c9b0
--- /dev/null
+++ b/debian/patches/0018-Add-implementation-of-nextTagData.patch
@@ -0,0 +1,167 @@
+From: Chris Lalancette <clalancette@openrobotics.org>
+Date: Fri, 9 Jul 2021 15:36:52 +0000
+Subject: Add implementation of nextTagData.
+
+Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
+---
+ utilities/xmlrpcpp/include/xmlrpcpp/XmlRpcUtil.h | 7 ++-
+ utilities/xmlrpcpp/src/XmlRpcUtil.cpp | 54 ++++++++++++++++++++++++
+ utilities/xmlrpcpp/test/TestValues.cpp | 48 +++++++++++++++++++++
+ 3 files changed, 108 insertions(+), 1 deletion(-)
+
+diff --git a/utilities/xmlrpcpp/include/xmlrpcpp/XmlRpcUtil.h b/utilities/xmlrpcpp/include/xmlrpcpp/XmlRpcUtil.h
+index d63576c..d0d121d 100644
+--- a/utilities/xmlrpcpp/include/xmlrpcpp/XmlRpcUtil.h
++++ b/utilities/xmlrpcpp/include/xmlrpcpp/XmlRpcUtil.h
+@@ -86,7 +86,9 @@ namespace XmlRpc {
+ class XMLRPCPP_DECL XmlRpcUtil {
+ public:
+ // hokey xml parsing
+- //! Returns contents between <tag> and </tag>, updates offset to char after </tag>
++ //! Returns contents between <tag> and </tag>, updates offset to char after </tag>.
++ //! This method will skip *any* intermediate string to find the tag; as such, it is
++ //! unsafe to use in general, and `nextTagData` should be used instead.
+ static std::string parseTag(const char* tag, std::string const& xml, int* offset);
+
+ //! Returns true if the tag is found and updates offset to the char after the tag
+@@ -100,6 +102,9 @@ namespace XmlRpc {
+ //! and updates offset to the char after the tag
+ static bool nextTagIs(const char* tag, std::string const& xml, int* offset);
+
++ //! Returns contents between <tag> and </tag> at the specified offset (modulo any whitespace),
++ //! and updates offset to char after </tag>
++ static std::string nextTagData(const char* tag, std::string const& xml, int* offset);
+
+ //! Convert raw text to encoded xml.
+ static std::string xmlEncode(const std::string& raw);
+diff --git a/utilities/xmlrpcpp/src/XmlRpcUtil.cpp b/utilities/xmlrpcpp/src/XmlRpcUtil.cpp
+index a964b94..440d87d 100644
+--- a/utilities/xmlrpcpp/src/XmlRpcUtil.cpp
++++ b/utilities/xmlrpcpp/src/XmlRpcUtil.cpp
+@@ -105,6 +105,8 @@ void XmlRpcUtil::error(const char* fmt, ...)
+
+
+ // Returns contents between <tag> and </tag>, updates offset to char after </tag>
++// This method will skip *any* intermediate string to find the tag; as such, it is
++// unsafe to use in general, and `nextTagData` should be used instead.
+ std::string
+ XmlRpcUtil::parseTag(const char* tag, std::string const& xml, int* offset)
+ {
+@@ -164,6 +166,58 @@ XmlRpcUtil::nextTagIs(const char* tag, std::string const& xml, int* offset)
+ return false;
+ }
+
++// Returns contents between <tag> and </tag> at the specified offset (modulo any whitespace),
++// and updates offset to char after </tag>
++std::string
++XmlRpcUtil::nextTagData(const char* tag, std::string const& xml, int* offset)
++{
++ if (offset == NULL) return std::string();
++ if (xml.length() > size_t(__INT_MAX__)) return std::string();
++ if (*offset >= int(xml.length())) return std::string();
++
++ const char* start_cp = xml.c_str() + *offset;
++ const char* cp = start_cp;
++ while (*cp && isspace(*cp)) {
++ ++cp;
++ }
++
++ const int len = int(strnlen(tag, xml.length()));
++ // Check if the tag is next; if not, we'll get out of here
++ if (!(*cp) || (strncmp(cp, tag, len) != 0)) {
++ return std::string();
++ }
++
++ cp += len;
++
++ // Now collect all of the data up to the next tag
++ std::string ret;
++ while (*cp) {
++ if (*cp == '<') {
++ break;
++ }
++ ret += *cp;
++ cp++;
++ }
++
++ if (!(*cp)) {
++ return std::string();
++ }
++
++ // Now find the end tag
++ std::string etag = "</";
++ etag += tag + 1;
++
++ if (strncmp(cp, etag.c_str(), etag.length()) != 0) {
++ return std::string();
++ }
++
++ cp += etag.length();
++
++ *offset += (cp - start_cp);
++
++ return ret;
++}
++
+ // Returns the next tag and updates offset to the char after the tag, or empty string
+ // if the next non-whitespace character is not '<'
+ std::string
+diff --git a/utilities/xmlrpcpp/test/TestValues.cpp b/utilities/xmlrpcpp/test/TestValues.cpp
+index 5d22c1e..f0226de 100644
+--- a/utilities/xmlrpcpp/test/TestValues.cpp
++++ b/utilities/xmlrpcpp/test/TestValues.cpp
+@@ -331,6 +331,54 @@ TEST(XmlRpc, testGetNextTag) {
+ EXPECT_EQ(offset, 2);
+ }
+
++TEST(XmlRpc, testNextTagData)
++{
++ int offset = 0;
++
++ // Test a null tag
++ EXPECT_EQ(XmlRpcUtil::nextTagData(NULL, "", &offset), std::string());
++ EXPECT_EQ(offset, 0);
++
++ // Test a null offset
++ EXPECT_EQ(XmlRpcUtil::nextTagData("<tag>", "", NULL), std::string());
++ EXPECT_EQ(offset, 0);
++
++ // Test if the offset is beyond the end of the input xml
++ offset = 20;
++ EXPECT_EQ(XmlRpcUtil::nextTagData("<tag>", "", &offset), std::string());
++ EXPECT_EQ(offset, 20);
++
++ // Test that the offset moves when finding a tag with no whitespace
++ offset = 0;
++ EXPECT_EQ(XmlRpcUtil::nextTagData("<tag>", "<tag></tag>", &offset), "");
++ EXPECT_EQ(offset, 11);
++
++ // Test that the offset moves when finding a tag with whitespace
++ offset = 0;
++ EXPECT_EQ(XmlRpcUtil::nextTagData("<tag>", " <tag></tag>", &offset), "");
++ EXPECT_EQ(offset, 14);
++
++ // Test that the offset moves when finding a tag with whitespace
++ offset = 0;
++ EXPECT_EQ(XmlRpcUtil::nextTagData("<tag>", " <tag>foo</tag>", &offset), "foo");
++ EXPECT_EQ(offset, 17);
++
++ // Test that the offset doesn't move when missing the tag
++ offset = 0;
++ EXPECT_EQ(XmlRpcUtil::nextTagData("<tag>", " <foo></foo>", &offset), "");
++ EXPECT_EQ(offset, 0);
++
++ // Test that the offset doesn't move when the close tag is after other tags
++ offset = 0;
++ EXPECT_EQ(XmlRpcUtil::nextTagData("<tag>", " <tag><foo></tag>", &offset), "");
++ EXPECT_EQ(offset, 0);
++
++ // Test that the offset doesn't move if there is no closing tag
++ offset = 0;
++ EXPECT_EQ(XmlRpcUtil::nextTagData("<tag>", " <tag>foo", &offset), "");
++ EXPECT_EQ(offset, 0);
++}
++
+ TEST(XmlRpc, testDateTime) {
+ // DateTime
+ int offset = 0;
diff --git a/debian/patches/0019-Switch-structFromXml-to-using-nextTagData.patch b/debian/patches/0019-Switch-structFromXml-to-using-nextTagData.patch
new file mode 100644
index 0000000..298192e
--- /dev/null
+++ b/debian/patches/0019-Switch-structFromXml-to-using-nextTagData.patch
@@ -0,0 +1,22 @@
+From: Chris Lalancette <clalancette@openrobotics.org>
+Date: Fri, 9 Jul 2021 15:37:04 +0000
+Subject: Switch structFromXml to using nextTagData.
+
+Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
+---
+ utilities/xmlrpcpp/src/XmlRpcValue.cpp | 2 ++
+ 1 file changed, 2 insertions(+)
+
+diff --git a/utilities/xmlrpcpp/src/XmlRpcValue.cpp b/utilities/xmlrpcpp/src/XmlRpcValue.cpp
+index 029337a..792d486 100644
+--- a/utilities/xmlrpcpp/src/XmlRpcValue.cpp
++++ b/utilities/xmlrpcpp/src/XmlRpcValue.cpp
+@@ -217,6 +217,8 @@ namespace XmlRpc {
+ // should be the start of a <value> tag. Destroys any existing value.
+ bool XmlRpcValue::fromXml(std::string const& valueXml, int* offset)
+ {
++ if (offset == NULL) return false;
++
+ int savedOffset = *offset;
+
+ invalidate();
diff --git a/debian/patches/series b/debian/patches/series
index 70ba75d..013123a 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -12,3 +12,8 @@
0012-Revert-earlier-change.patch
0013-Update-tests.patch
0014-Improve-test-error-handling.patch
+0015-Fix-oversize-string-test.patch
+0016-Add-defensive-checks-for-offset-being-NULL.patch
+0017-Add-unit-tests-for-XML-tag-utility-functions.patch
+0018-Add-implementation-of-nextTagData.patch
+0019-Switch-structFromXml-to-using-nextTagData.patch
--
2.34.0
Reply to: