Trap for overly large input to XmlRPCPP

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) {

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

utilities/xmlrpcpp/src/XmlRpcSocket.cpp

@@ -317,6 +317,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;
 }
 
@@ -325,6 +332,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;

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;

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;

utilities/xmlrpcpp/test/test_client.cpp

@@ -492,6 +492,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()
@@ -647,6 +652,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"
@@ -920,6 +930,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
@@ -1090,6 +1125,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