Package: release.debian.org
Severity: normal
Tags: buster
User: release.debian@packages.debian.org
Usertags: pu
[ Reason ]
CVE-2020-16124 was published with a number of integer overflow in the
XML RPC layer of 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 stable.
[ Tests ]
The patch adds a unit test and I ran manual tests using the relay
command from the topic-tools package.
[ Risks ]
I see the code as rather trivial, and the risk as low.
[ 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 patch adds size checks and unit tests.
[ Other info ]
I left the patches as they where merged upstream but can squash them if
that would be easier for you.
diff --git a/debian/changelog b/debian/changelog
index 2f80bb1..420c997 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+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)
+
+ -- Jochen Sprickerhof Fri, 16 Oct 2020 17:48:57 +0200
+
ros-ros-comm (1.14.3+ds1-5+deb10u1) stable; urgency=high
* Add https://github.com/ros/ros_comm/pull/1771 (Fix CVE-2019-13566,
CVE-2019-13465)
diff --git a/debian/patches/0007-Build-Python-3-version-of-roslz4.patch
b/debian/patches/0007-Build-Python-3-version-of-roslz4.patch
index 9487775..ab177c6 100644
--- a/debian/patches/0007-Build-Python-3-version-of-roslz4.patch
+++ b/debian/patches/0007-Build-Python-3-version-of-roslz4.patch
@@ -6,6 +6,8 @@ Subject: Build Python 3 version of roslz4
CMakeLists.txt | 9 +
1 file changed, 9 insertions(+)
+diff --git a/CMakeLists.txt b/CMakeLists.txt
+index 169420f..eb45865 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -20,3 +20,12 @@ add_subdirectory(tools/rosout)
diff --git
a/debian/patches/0010-Trap-for-overly-large-input-to-XmlRPCPP-which-could-.patch
b/debian/patches/0010-Trap-for-overly-large-input-to-XmlRPCPP-which-could-.patch
new file mode 100644
index 000..e26a0d1
--- /dev/null
+++
b/debian/patches/0010-Trap-for-overly-large-input-to-XmlRPCPP-which-could-.patch
@@ -0,0 +1,351 @@
+From: Sid Faber
+Date: Tue, 15 Sep 2020 19:48:40 +
+Subject: Trap for overly large input to XmlRPCPP which could cause problems
+ with int <-> size_t conversions.
+
+ - In XmlRpcClient, XmlRpcServerConnection and XmlRpcSocket, recognize when
incoming or outgoing data is too large, generate an error and discard the data
when practical.
+ - Use the safe strtol() rather than atoi() to decode an incoming
content-length header, and generate an error if the length is invalid or too
large.
+ - In XmlRpcUtil, prevent attempts to parse overly large XML input.
+ - Add tests where they can reasonably be inserted into existing test routines.
+
+Although this fix could be cleaner the update is written to make the update
ABI compatible.
+
+This fix addresses CVE-2020-16124 / Integer overflow in ros_comm.
+
+Signed-off-by: Sid Faber
+---
+ utilities/xmlrpcpp/src/XmlRpcClient.cpp | 25 +++--
+ utilities/xmlrpcpp/src/XmlRpcServerConnection.cpp | 28 --
+ utilities/xmlrpcpp/src/XmlRpcSocket.cpp | 13 +
+ utilities/xmlrpcpp/src/XmlRpcUtil.cpp | 5 ++
+ utilities/xmlrpcpp/test/TestValues.cpp| 27 +-
+ utilities/xmlrpcpp/test/test_client.cpp | 65 +++
+ 6 files changed, 153 insertions(+), 10 deletions(-)
+
+diff --git a/utilities/xmlrpcpp/src/XmlRpcClient.cpp
b/utilities/xmlrpcpp/src/XmlRpcClient.cpp
+index 2d42bb8..d53214e 100644
+--- a/utilities/xmlrpcpp/src/XmlRpcClient.cpp
b/utilities/xmlrpcpp/src/XmlRpcClient.cpp
+@@ -312,6 +312,13 @@ XmlRpcClient::generateRequest(const char* methodName,
XmlRpcValue const& params)
+ header.length(), body.length());
+
+ _request = header + body;
++ // Limit the size of the request to avoid integer overruns
++ if (_request.length() > size_t(__INT_MAX__)) {
++XmlRpcUtil::error("XmlRpcClient::generateRequest: request length (%u)
exceeds maximum allowed size (%u).",
++ _request.length(), __INT_MAX__);
++_request.clear();
++return false;
++ }
+ return true;
+ }
+
+@@ -431,13 +438,16 @@ XmlRpcClient::readHeader()
+ return false; // We could try to figure it out by parsing as we read,
but for now...
+ }
+
+- _contentLength = atoi(lp);
+- if (_contentLength <= 0) {
+-XmlRpcUtil::error("Error in XmlRpcClient::readHeader: Invalid
Content-length specified (%d).", _contentLength);
++ // avoid overly large or improperly formatted content-length
++ long int clength = 0;
++ clength = strtol(lp, nullptr, 10);
++ if ((clength <= 0) ||
