--- Begin Message ---
Package: release.debian.org
Severity: normal
Tags: buster
User: [email protected]
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 <[email protected]> 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 0000000..e26a0d1
--- /dev/null
+++
b/debian/patches/0010-Trap-for-overly-large-input-to-XmlRPCPP-which-could-.patch
@@ -0,0 +1,351 @@
+From: Sid Faber <[email protected]>
+Date: Tue, 15 Sep 2020 19:48:40 +0000
+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 <[email protected]>
+---
+ 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) || (clength > __INT_MAX__)) {
++ XmlRpcUtil::error("Error in XmlRpcClient::readHeader: Invalid
Content-length specified.");
+ // Close the socket because we can't make further use of it.
+ close();
+ return false;
+ }
++ _contentLength = int(clength);
+
+ XmlRpcUtil::log(4, "client read content length: %d", _contentLength);
+
+@@ -448,7 +458,6 @@ XmlRpcClient::readHeader()
+ return true; // Continue monitoring this source
+ }
+
+-
+ bool
+ XmlRpcClient::readResponse()
+ {
+@@ -464,6 +473,14 @@ XmlRpcClient::readResponse()
+ }
+ _response += buff;
+
++ // Avoid an overly large response
++ if (_response.length() > size_t(__INT_MAX__)) {
++ XmlRpcUtil::error("XmlRpcClient::readResponse: response length (%u)
exceeds the maximum allowed size (%u).",
++ _response.length(), __INT_MAX__);
++ _response.clear();
++ close();
++ return false;
++ }
+ // If we haven't gotten the entire _response yet, return (keep reading)
+ if (int(_response.length()) < _contentLength) {
+ if (_eof) {
+diff --git a/utilities/xmlrpcpp/src/XmlRpcServerConnection.cpp
b/utilities/xmlrpcpp/src/XmlRpcServerConnection.cpp
+index 26c997f..1aeba70 100644
+--- a/utilities/xmlrpcpp/src/XmlRpcServerConnection.cpp
++++ b/utilities/xmlrpcpp/src/XmlRpcServerConnection.cpp
+@@ -119,11 +119,14 @@ XmlRpcServerConnection::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("XmlRpcServerConnection::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) || (clength > __INT_MAX__)) {
++ XmlRpcUtil::error("XmlRpcServerConnection::readHeader: Invalid
Content-length specified.");
+ return false;
+ }
++ _contentLength = int(clength);
+
+ XmlRpcUtil::log(3, "XmlRpcServerConnection::readHeader: specified content
length is %d.", _contentLength);
+
+@@ -157,6 +160,13 @@ XmlRpcServerConnection::readRequest()
+ XmlRpcUtil::error("XmlRpcServerConnection::readRequest: read error
(%s).",XmlRpcSocket::getErrorMsg().c_str());
+ return false;
+ }
++ // Avoid an overly large request
++ if (_request.length() > size_t(__INT_MAX__)) {
++ XmlRpcUtil::error("XmlRpcServerConnection::readRequest: request length
(%u) exceeds the maximum allowed size (%u)",
++ _request.length(), __INT_MAX__);
++ _request.resize(__INT_MAX__);
++ return false;
++ }
+
+ // If we haven't gotten the entire request yet, return (keep reading)
+ if (int(_request.length()) < _contentLength) {
+@@ -334,8 +344,16 @@ XmlRpcServerConnection::generateResponse(std::string
const& resultXml)
+ std::string body = RESPONSE_1 + resultXml + RESPONSE_2;
+ std::string header = generateHeader(body);
+
+- _response = header + body;
+- XmlRpcUtil::log(5, "XmlRpcServerConnection::generateResponse:\n%s\n",
_response.c_str());
++ // Avoid an overly large response
++ if ((header.length() + body.length()) > size_t(__INT_MAX__)) {
++ XmlRpcUtil::error("XmlRpcServerConnection::generateResponse: response
length (%u) exceeds the maximum allowed size (%u).",
++ _response.length(), __INT_MAX__);
++ _response = "";
++ }
++ else {
++ _response = header + body;
++ XmlRpcUtil::log(5, "XmlRpcServerConnection::generateResponse:\n%s\n",
_response.c_str());
++ }
+ }
+
+ // Prepend http headers
+diff --git a/utilities/xmlrpcpp/src/XmlRpcSocket.cpp
b/utilities/xmlrpcpp/src/XmlRpcSocket.cpp
+index 8c5fce0..272e3d5 100644
+--- a/utilities/xmlrpcpp/src/XmlRpcSocket.cpp
++++ b/utilities/xmlrpcpp/src/XmlRpcSocket.cpp
+@@ -313,6 +313,13 @@ XmlRpcSocket::nbRead(int fd, std::string& s, bool *eof)
+ return false; // Error
+ }
+ }
++ // Watch for integer overrun
++ if (s.length() > size_t(__INT_MAX__)) {
++ XmlRpcUtil::error("XmlRpcSocket::nbRead: text size (%u) exceeds the
maximum allowed size (%s).",
++ s.length(), __INT_MAX__);
++ s.clear();
++ return false;
++ }
+ return true;
+ }
+
+@@ -321,6 +328,12 @@ XmlRpcSocket::nbRead(int fd, std::string& s, bool *eof)
+ bool
+ XmlRpcSocket::nbWrite(int fd, const std::string& s, int *bytesSoFar)
+ {
++ // Watch for integer overrun
++ if (s.length() > size_t(__INT_MAX__)) {
++ XmlRpcUtil::error("XmlRpcSocket::nbWrite: text size (%u) exceeds the
maximum allowed size(%s)",
++ s.length(), __INT_MAX__);
++ return false;
++ }
+ int nToWrite = int(s.length()) - *bytesSoFar;
+ char *sp = const_cast<char*>(s.c_str()) + *bytesSoFar;
+ bool wouldBlock = false;
+diff --git a/utilities/xmlrpcpp/src/XmlRpcUtil.cpp
b/utilities/xmlrpcpp/src/XmlRpcUtil.cpp
+index 901ebc8..ab0991d 100644
+--- a/utilities/xmlrpcpp/src/XmlRpcUtil.cpp
++++ b/utilities/xmlrpcpp/src/XmlRpcUtil.cpp
+@@ -108,6 +108,8 @@ void XmlRpcUtil::error(const char* fmt, ...)
+ std::string
+ XmlRpcUtil::parseTag(const char* tag, std::string const& xml, int* offset)
+ {
++ // 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();
+ size_t istart = xml.find(tag, *offset);
+ if (istart == std::string::npos) return std::string();
+@@ -126,6 +128,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 (xml.length() > size_t(__INT_MAX__)) return false;
+ if (*offset >= int(xml.length())) return false;
+ size_t istart = xml.find(tag, *offset);
+ if (istart == std::string::npos)
+@@ -141,6 +144,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 (xml.length() > size_t(__INT_MAX__)) return false;
+ if (*offset >= int(xml.length())) return false;
+ const char* cp = xml.c_str() + *offset;
+ int nc = 0;
+@@ -162,6 +166,7 @@ XmlRpcUtil::nextTagIs(const char* tag, std::string const&
xml, int* offset)
+ std::string
+ XmlRpcUtil::getNextTag(std::string const& xml, int* offset)
+ {
++ if (xml.length() > size_t(__INT_MAX__)) return std::string();
+ if (*offset >= int(xml.length())) return std::string();
+
+ size_t pos = *offset;
+diff --git a/utilities/xmlrpcpp/test/TestValues.cpp
b/utilities/xmlrpcpp/test/TestValues.cpp
+index 842e78e..91b676a 100644
+--- a/utilities/xmlrpcpp/test/TestValues.cpp
++++ b/utilities/xmlrpcpp/test/TestValues.cpp
+@@ -27,8 +27,9 @@
+ #include <stdlib.h>
+ #include <string>
+
+-#include "xmlrpcpp/XmlRpcValue.h"
+ #include "xmlrpcpp/XmlRpcException.h"
++#include "xmlrpcpp/XmlRpcUtil.h"
++#include "xmlrpcpp/XmlRpcValue.h"
+
+ #include <gtest/gtest.h>
+
+@@ -174,6 +175,30 @@ TEST(XmlRpc, testString) {
+ EXPECT_EQ("Now is the time <&", ss.str());
+ }
+
++//Test decoding of a well-formed but overly large XML input
++TEST(XmlRpc, testOversizeString) {
++ std::string xml = "<tag><nexttag>";
++ xml += std::string(__INT_MAX__, 'a');
++ xml += "a</nextag></tag>";
++ int offset;
++
++ offset = 0;
++ EXPECT_EQ(XmlRpcUtil::parseTag("<tag>", xml, &offset), std::string());
++ EXPECT_EQ(offset, 0);
++
++ offset = 0;
++ EXPECT_FALSE(XmlRpcUtil::findTag("<tag>", xml, &offset));
++ EXPECT_EQ(offset, 0);
++
++ offset = 0;
++ EXPECT_FALSE(XmlRpcUtil::nextTagIs("<tag>", xml, &offset));
++ EXPECT_EQ(offset, 0);
++
++ offset = 0;
++ EXPECT_EQ(XmlRpcUtil::getNextTag(xml, &offset), std::string());
++ EXPECT_EQ(offset, 0);
++}
++
+ TEST(XmlRpc, testDateTime) {
+ // DateTime
+ int offset = 0;
+diff --git a/utilities/xmlrpcpp/test/test_client.cpp
b/utilities/xmlrpcpp/test/test_client.cpp
+index 2318519..6c253b3 100644
+--- a/utilities/xmlrpcpp/test/test_client.cpp
++++ b/utilities/xmlrpcpp/test/test_client.cpp
+@@ -466,6 +466,11 @@ TEST(XmlRpcClient, generateRequest) {
+ "<methodCall><methodName>DoEmpty</methodName>\r\n"
+ "</methodCall>\r\n",
+ a._request);
++
++ // create a request where content fits but the message will overflow and
gets truncated
++ XmlRpcValue toolarge(std::string(__INT_MAX__ - 10, 'a'));
++ EXPECT_FALSE(a.generateRequest("DoFoo", toolarge));
++ EXPECT_EQ(a._request.length(), 0);
+ }
+
+ // Test generateHeader()
+@@ -621,6 +626,11 @@ const std::string header2 = "HTTP/1.0 200 OK\r\n"
+ "Date: Mon, 30 Oct 2017 22:28:12 GMT\r\n"
+ "Content-type: text/xml\r\n"
+ "Content-length: 114\r\n\r\n";
++// Header for testing a custom Content-length value
++const std::string header3 = "HTTP/1.1 200 OK\r\n"
++ "Server: XMLRPC++ 0.7\r\n"
++ "Content-Type: text/xml\r\n"
++ "Content-length: ";
+ // Generic response XML
+ const std::string response = "<?xml version=\"1.0\"?>\r\n"
+ "<methodResponse><params><param>\r\n"
+@@ -894,6 +904,31 @@ TEST_F(MockSocketTest, readHeader_partial_err) {
+ Expect_close(8);
+ }
+
++// Test to fail when content-length is too large
++TEST_F(MockSocketTest, readHeader_oversize) {
++ XmlRpcClientForTest a("localhost", 42);
++
++ // Hack us into the correct initial state.
++ a.setfd(7);
++ a._connectionState = XmlRpcClientForTest::READ_HEADER;
++
++ // Add a large content-length to the standard header
++ std::string header_cl = header3;
++ header_cl += std::to_string(size_t(__INT_MAX__) + 1);
++ header_cl += "\r\n\r\n ";
++
++ Expect_nbRead(7, header_cl, false, true);
++ Expect_close(7);
++
++ EXPECT_FALSE(a.readHeader());
++ EXPECT_EQ(0, a._contentLength); // Content length should be reset
++
++ // Check that all expected function calls were made before destruction.
++ CheckCalls();
++}
++
++
++
+ // Test readResponse()
+ // Test read of response in a single read call
+ // Test response spread across several read calls
+@@ -1064,6 +1099,36 @@ TEST_F(MockSocketTest, readResponse_eof) {
+ CheckCalls();
+ }
+
++// Test that readResponse closes the socket and truncates the response when
too
++// much data is received (even when content-length is legitimate)
++TEST_F(MockSocketTest, readResponse_oversize) {
++ XmlRpcClientForTest a("localhost", 42);
++
++ // Hack us into the correct initial state.
++ a.setfd(8);
++ a._connectionState = XmlRpcClientForTest::READ_RESPONSE;
++
++ // Create an overflow response
++ std::string response = std::string(__INT_MAX__, 'a');
++ response += "a";
++
++ // Start with a pre-populated content-length that is within bounds
++ a._contentLength = __INT_MAX__;
++
++ // Expect to read the socket
++ Expect_nbRead(8, response, true, true);
++ // Expect the socket to close
++ Expect_close(8);
++
++ // Expect readResponse to return false because the response is too long, and
++ // truncate the response.
++ EXPECT_FALSE(a.readResponse());
++ EXPECT_EQ(a._response.size(), 0);
++
++ CheckCalls();
++}
++
++
+ // Test parseResponse
+ // Test correct parsing of various response types: empty, int, double,
+ // string, bool, list, struct, date, base64, etc
diff --git
a/debian/patches/0011-Trap-for-memory-allocation-error-in-tests.patch
b/debian/patches/0011-Trap-for-memory-allocation-error-in-tests.patch
new file mode 100644
index 0000000..b25332f
--- /dev/null
+++ b/debian/patches/0011-Trap-for-memory-allocation-error-in-tests.patch
@@ -0,0 +1,164 @@
+From: Sid Faber <[email protected]>
+Date: Wed, 30 Sep 2020 15:33:48 -0400
+Subject: Trap for memory allocation error in tests
+
+Signed-off-by: Sid Faber <[email protected]>
+---
+ utilities/xmlrpcpp/src/XmlRpcClient.cpp | 1 +
+ utilities/xmlrpcpp/test/TestValues.cpp | 47 ++++++++++++++++--------------
+ utilities/xmlrpcpp/test/test_client.cpp | 51 +++++++++++++++++++++------------
+ 3 files changed, 60 insertions(+), 39 deletions(-)
+
+diff --git a/utilities/xmlrpcpp/src/XmlRpcClient.cpp
b/utilities/xmlrpcpp/src/XmlRpcClient.cpp
+index d53214e..ad5ba96 100644
+--- a/utilities/xmlrpcpp/src/XmlRpcClient.cpp
++++ b/utilities/xmlrpcpp/src/XmlRpcClient.cpp
+@@ -458,6 +458,7 @@ XmlRpcClient::readHeader()
+ return true; // Continue monitoring this source
+ }
+
++
+ bool
+ XmlRpcClient::readResponse()
+ {
+diff --git a/utilities/xmlrpcpp/test/TestValues.cpp
b/utilities/xmlrpcpp/test/TestValues.cpp
+index 91b676a..927c9a5 100644
+--- a/utilities/xmlrpcpp/test/TestValues.cpp
++++ b/utilities/xmlrpcpp/test/TestValues.cpp
+@@ -27,9 +27,9 @@
+ #include <stdlib.h>
+ #include <string>
+
++#include "xmlrpcpp/XmlRpcValue.h"
+ #include "xmlrpcpp/XmlRpcException.h"
+ #include "xmlrpcpp/XmlRpcUtil.h"
+-#include "xmlrpcpp/XmlRpcValue.h"
+
+ #include <gtest/gtest.h>
+
+@@ -177,26 +177,31 @@ TEST(XmlRpc, testString) {
+
+ //Test decoding of a well-formed but overly large XML input
+ TEST(XmlRpc, testOversizeString) {
+- std::string xml = "<tag><nexttag>";
+- xml += std::string(__INT_MAX__, 'a');
+- xml += "a</nextag></tag>";
+- int offset;
+-
+- offset = 0;
+- EXPECT_EQ(XmlRpcUtil::parseTag("<tag>", xml, &offset), std::string());
+- EXPECT_EQ(offset, 0);
+-
+- offset = 0;
+- EXPECT_FALSE(XmlRpcUtil::findTag("<tag>", xml, &offset));
+- EXPECT_EQ(offset, 0);
+-
+- offset = 0;
+- EXPECT_FALSE(XmlRpcUtil::nextTagIs("<tag>", xml, &offset));
+- EXPECT_EQ(offset, 0);
+-
+- offset = 0;
+- EXPECT_EQ(XmlRpcUtil::getNextTag(xml, &offset), std::string());
+- EXPECT_EQ(offset, 0);
++ try {
++ std::string xml = "<tag><nexttag>";
++ xml += std::string(__INT_MAX__, 'a');
++ xml += "a</nextag></tag>";
++ int offset;
++
++ offset = 0;
++ EXPECT_EQ(XmlRpcUtil::parseTag("<tag>", xml, &offset), std::string());
++ EXPECT_EQ(offset, 0);
++
++ offset = 0;
++ EXPECT_FALSE(XmlRpcUtil::findTag("<tag>", xml, &offset));
++ EXPECT_EQ(offset, 0);
++
++ offset = 0;
++ EXPECT_FALSE(XmlRpcUtil::nextTagIs("<tag>", xml, &offset));
++ EXPECT_EQ(offset, 0);
++
++ offset = 0;
++ EXPECT_EQ(XmlRpcUtil::getNextTag(xml, &offset), std::string());
++ EXPECT_EQ(offset, 0);
++ }
++ catch (std::bad_alloc& err) {
++ GTEST_SKIP() << "Unable to allocate memory for overflow tests\n";
++ }
+ }
+
+ TEST(XmlRpc, testDateTime) {
+diff --git a/utilities/xmlrpcpp/test/test_client.cpp
b/utilities/xmlrpcpp/test/test_client.cpp
+index 6c253b3..1c37e51 100644
+--- a/utilities/xmlrpcpp/test/test_client.cpp
++++ b/utilities/xmlrpcpp/test/test_client.cpp
+@@ -466,11 +466,20 @@ TEST(XmlRpcClient, generateRequest) {
+ "<methodCall><methodName>DoEmpty</methodName>\r\n"
+ "</methodCall>\r\n",
+ a._request);
++}
+
+- // create a request where content fits but the message will overflow and
gets truncated
+- XmlRpcValue toolarge(std::string(__INT_MAX__ - 10, 'a'));
+- EXPECT_FALSE(a.generateRequest("DoFoo", toolarge));
+- EXPECT_EQ(a._request.length(), 0);
++// create a request where content fits but the message will overflow and gets
truncated
++TEST(XmlRpcClient, generateOversizeRequest) {
++ XmlRpcClientForTest a("localhost", 42);
++ // Gracefully skip this test if there's not enough memory to run it
++ try {
++ XmlRpcValue toolarge(std::string(__INT_MAX__ - 10, 'a'));
++ EXPECT_FALSE(a.generateRequest("DoFoo", toolarge));
++ EXPECT_EQ(a._request.length(), 0);
++ }
++ catch (std::bad_alloc& err) {
++ GTEST_SKIP() << "Unable to allocate memory for overflow test\n";
++ }
+ }
+
+ // Test generateHeader()
+@@ -1108,24 +1117,30 @@ TEST_F(MockSocketTest, readResponse_oversize) {
+ a.setfd(8);
+ a._connectionState = XmlRpcClientForTest::READ_RESPONSE;
+
+- // Create an overflow response
+- std::string response = std::string(__INT_MAX__, 'a');
+- response += "a";
++ try {
++ // Create an overflow response
++ std::string response = std::string(__INT_MAX__, 'a');
++ response += "a";
+
+- // Start with a pre-populated content-length that is within bounds
+- a._contentLength = __INT_MAX__;
++ // Start with a pre-populated content-length that is within bounds
++ a._contentLength = __INT_MAX__;
+
+- // Expect to read the socket
+- Expect_nbRead(8, response, true, true);
+- // Expect the socket to close
+- Expect_close(8);
++ // Expect to read the socket
++ Expect_nbRead(8, response, true, true);
++ // Expect the socket to close
++ Expect_close(8);
+
+- // Expect readResponse to return false because the response is too long, and
+- // truncate the response.
+- EXPECT_FALSE(a.readResponse());
+- EXPECT_EQ(a._response.size(), 0);
++ // Expect readResponse to return false because the response is too long,
and
++ // truncate the response.
++ EXPECT_FALSE(a.readResponse());
++ EXPECT_EQ(a._response.size(), 0);
++
++ CheckCalls();
++ }
++ catch (std::bad_alloc& err) {
++ GTEST_SKIP() << "Unable to allocate memory for overflow test\n";
++ }
+
+- CheckCalls();
+ }
+
+
diff --git a/debian/patches/0012-Revert-earlier-change.patch
b/debian/patches/0012-Revert-earlier-change.patch
new file mode 100644
index 0000000..36d7996
--- /dev/null
+++ b/debian/patches/0012-Revert-earlier-change.patch
@@ -0,0 +1,22 @@
+From: Sid Faber <[email protected]>
+Date: Wed, 30 Sep 2020 15:39:49 -0400
+Subject: Revert earlier change
+
+Signed-off-by: Sid Faber <[email protected]>
+---
+ utilities/xmlrpcpp/src/XmlRpcClient.cpp | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/utilities/xmlrpcpp/src/XmlRpcClient.cpp
b/utilities/xmlrpcpp/src/XmlRpcClient.cpp
+index ad5ba96..4496fab 100644
+--- a/utilities/xmlrpcpp/src/XmlRpcClient.cpp
++++ b/utilities/xmlrpcpp/src/XmlRpcClient.cpp
+@@ -458,7 +458,7 @@ XmlRpcClient::readHeader()
+ return true; // Continue monitoring this source
+ }
+
+-
++
+ bool
+ XmlRpcClient::readResponse()
+ {
diff --git a/debian/patches/0013-Update-tests.patch
b/debian/patches/0013-Update-tests.patch
new file mode 100644
index 0000000..1143fff
--- /dev/null
+++ b/debian/patches/0013-Update-tests.patch
@@ -0,0 +1,101 @@
+From: Sid Faber <[email protected]>
+Date: Thu, 1 Oct 2020 12:19:54 -0400
+Subject: Update tests
+
+Replace call to GTEST_SKIP with output to stderr. Remove the
+redResponseOversize test since out-of-memory errors during
+testing cannot easily be handled within the existing test objects.
+
+Signed-off-by: Sid Faber <[email protected]>
+---
+ utilities/xmlrpcpp/test/TestValues.cpp | 2 +-
+ utilities/xmlrpcpp/test/test_client.cpp | 42 ++-------------------------------
+ 2 files changed, 3 insertions(+), 41 deletions(-)
+
+diff --git a/utilities/xmlrpcpp/test/TestValues.cpp
b/utilities/xmlrpcpp/test/TestValues.cpp
+index 927c9a5..6b9d119 100644
+--- a/utilities/xmlrpcpp/test/TestValues.cpp
++++ b/utilities/xmlrpcpp/test/TestValues.cpp
+@@ -200,7 +200,7 @@ TEST(XmlRpc, testOversizeString) {
+ EXPECT_EQ(offset, 0);
+ }
+ catch (std::bad_alloc& err) {
+- GTEST_SKIP() << "Unable to allocate memory for overflow tests\n";
++ std::cerr << "[ SKIPPED ] Unable to allocate memory for test
testOversizeString\n";
+ }
+ }
+
+diff --git a/utilities/xmlrpcpp/test/test_client.cpp
b/utilities/xmlrpcpp/test/test_client.cpp
+index 1c37e51..bdcb53c 100644
+--- a/utilities/xmlrpcpp/test/test_client.cpp
++++ b/utilities/xmlrpcpp/test/test_client.cpp
+@@ -478,7 +478,7 @@ TEST(XmlRpcClient, generateOversizeRequest) {
+ EXPECT_EQ(a._request.length(), 0);
+ }
+ catch (std::bad_alloc& err) {
+- GTEST_SKIP() << "Unable to allocate memory for overflow test\n";
++ std::cerr << "[ SKIPPED ] Unable to allocate memory for test
generateOversizeRequest\n";
+ }
+ }
+
+@@ -913,7 +913,7 @@ TEST_F(MockSocketTest, readHeader_partial_err) {
+ Expect_close(8);
+ }
+
+-// Test to fail when content-length is too large
++// Test that the read will fail when content-length is too large
+ TEST_F(MockSocketTest, readHeader_oversize) {
+ XmlRpcClientForTest a("localhost", 42);
+
+@@ -936,8 +936,6 @@ TEST_F(MockSocketTest, readHeader_oversize) {
+ CheckCalls();
+ }
+
+-
+-
+ // Test readResponse()
+ // Test read of response in a single read call
+ // Test response spread across several read calls
+@@ -1108,42 +1106,6 @@ TEST_F(MockSocketTest, readResponse_eof) {
+ CheckCalls();
+ }
+
+-// Test that readResponse closes the socket and truncates the response when
too
+-// much data is received (even when content-length is legitimate)
+-TEST_F(MockSocketTest, readResponse_oversize) {
+- XmlRpcClientForTest a("localhost", 42);
+-
+- // Hack us into the correct initial state.
+- a.setfd(8);
+- a._connectionState = XmlRpcClientForTest::READ_RESPONSE;
+-
+- try {
+- // Create an overflow response
+- std::string response = std::string(__INT_MAX__, 'a');
+- response += "a";
+-
+- // Start with a pre-populated content-length that is within bounds
+- a._contentLength = __INT_MAX__;
+-
+- // Expect to read the socket
+- Expect_nbRead(8, response, true, true);
+- // Expect the socket to close
+- Expect_close(8);
+-
+- // Expect readResponse to return false because the response is too long,
and
+- // truncate the response.
+- EXPECT_FALSE(a.readResponse());
+- EXPECT_EQ(a._response.size(), 0);
+-
+- CheckCalls();
+- }
+- catch (std::bad_alloc& err) {
+- GTEST_SKIP() << "Unable to allocate memory for overflow test\n";
+- }
+-
+-}
+-
+-
+ // Test parseResponse
+ // Test correct parsing of various response types: empty, int, double,
+ // string, bool, list, struct, date, base64, etc
diff --git a/debian/patches/0014-Improve-test-error-handling.patch
b/debian/patches/0014-Improve-test-error-handling.patch
new file mode 100644
index 0000000..49c83a2
--- /dev/null
+++ b/debian/patches/0014-Improve-test-error-handling.patch
@@ -0,0 +1,54 @@
+From: Sid Faber <[email protected]>
+Date: Fri, 2 Oct 2020 10:43:43 -0400
+Subject: Improve test error handling
+
+Use GTEST_SKIP if available, otherwise print to stderr. Remove test
+that's being killed because it takes too long to handle the oversize
+test values
+---
+ utilities/xmlrpcpp/test/TestValues.cpp | 6 +++++-
+ utilities/xmlrpcpp/test/test_client.cpp | 14 --------------
+ 2 files changed, 5 insertions(+), 15 deletions(-)
+
+diff --git a/utilities/xmlrpcpp/test/TestValues.cpp
b/utilities/xmlrpcpp/test/TestValues.cpp
+index 6b9d119..acd79c2 100644
+--- a/utilities/xmlrpcpp/test/TestValues.cpp
++++ b/utilities/xmlrpcpp/test/TestValues.cpp
+@@ -200,7 +200,11 @@ TEST(XmlRpc, testOversizeString) {
+ EXPECT_EQ(offset, 0);
+ }
+ catch (std::bad_alloc& err) {
+- std::cerr << "[ SKIPPED ] Unable to allocate memory for test
testOversizeString\n";
++#ifdef GTEST_SKIP
++ GTEST_SKIP() << "Unable to allocate memory to run test\n";
++#else
++ std::cerr << "[ SKIPPED ] XmlRpc.testOversizeString Unable to allocate
memory to run test\n";
++#endif
+ }
+ }
+
+diff --git a/utilities/xmlrpcpp/test/test_client.cpp
b/utilities/xmlrpcpp/test/test_client.cpp
+index bdcb53c..9457ea5 100644
+--- a/utilities/xmlrpcpp/test/test_client.cpp
++++ b/utilities/xmlrpcpp/test/test_client.cpp
+@@ -468,20 +468,6 @@ TEST(XmlRpcClient, generateRequest) {
+ a._request);
+ }
+
+-// create a request where content fits but the message will overflow and gets
truncated
+-TEST(XmlRpcClient, generateOversizeRequest) {
+- XmlRpcClientForTest a("localhost", 42);
+- // Gracefully skip this test if there's not enough memory to run it
+- try {
+- XmlRpcValue toolarge(std::string(__INT_MAX__ - 10, 'a'));
+- EXPECT_FALSE(a.generateRequest("DoFoo", toolarge));
+- EXPECT_EQ(a._request.length(), 0);
+- }
+- catch (std::bad_alloc& err) {
+- std::cerr << "[ SKIPPED ] Unable to allocate memory for test
generateOversizeRequest\n";
+- }
+-}
+-
+ // Test generateHeader()
+ // Correct header is generated for various sizes and content of request body.
+ TEST(XmlRpcClient, generateHeader) {
diff --git a/debian/patches/series b/debian/patches/series
index daf961c..70ba75d 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -7,3 +7,8 @@
0007-Build-Python-3-version-of-roslz4.patch
0008-fixing-string-check.patch
1741.patch
+0010-Trap-for-overly-large-input-to-XmlRPCPP-which-could-.patch
+0011-Trap-for-memory-allocation-error-in-tests.patch
+0012-Revert-earlier-change.patch
+0013-Update-tests.patch
+0014-Improve-test-error-handling.patch
--- End Message ---
