Fix RPC/SCGI security and crash bugs by @sirus20x6

src/rpc/lua.cc

@@ -211,7 +211,7 @@ object_to_target(const torrent::Object& obj, int call_flags, rpc::target_type* t
         auto tracker = new torrent::tracker::Tracker(rpc.slot_find_tracker()(download, std::stoi(std::string(index))));
 
         *deleter = [tracker]() { delete tracker; };
-        *target = rpc::make_target(command_base::target_tracker, target);
+        *target  = rpc::make_target(command_base::target_tracker, tracker);
       }
       break;
 

src/rpc/object_storage.h

@@ -17,7 +17,7 @@ namespace rpc {
 
 struct object_storage_node {
   torrent::Object object;
-  char            flags;
+  unsigned int    flags;
 };
 
 typedef std::unordered_map<fixed_key_type<64>, object_storage_node, hash_fixed_key_type> object_storage_base_type;

src/rpc/scgi.cc

@@ -53,7 +53,7 @@ SCgi::open_port(sockaddr* sa, unsigned int length, bool dont_route) {
 
 void
 SCgi::open_named(const std::string& filename) {
-  if (filename.empty() || filename.size() > 4096)
+  if (filename.empty() || filename.size() >= sizeof(sockaddr_un::sun_path))
     throw torrent::resource_error("Invalid filename length.");
 
   auto buffer = std::make_unique<char[]>(sizeof(sockaddr_un) + filename.size() + 1);

src/rpc/xmlrpc.cc

@@ -9,20 +9,21 @@
 
 namespace rpc {
 
-std::vector<std::unique_ptr<const char>> XmlRpc::m_command_names;
+std::vector<std::string> XmlRpc::m_command_names;
 
 const char*
 XmlRpc::store_command_name(const char* name) {
-  if (::strnlen(name, 8192) == 8192)
+  if (::strnlen(name, 1024 + 1) > 1024)
     throw torrent::input_error("XMLRPC command name too long, limit is 8192 characters.");
 
   for (const auto& itr : m_command_names) {
-    if (::strcmp(itr.get(), name) == 0)
+    if (itr == name)
-      return itr.get();
+      return itr.c_str();
   }
 
-  m_command_names.push_back(std::unique_ptr<const char>(::strdup(name)));
+  m_command_names.push_back(name);
-  return m_command_names.back().get();
+
+  return m_command_names.back().c_str();
 }
 
 #ifndef HAVE_XMLRPC_C

src/rpc/xmlrpc.h

@@ -61,7 +61,7 @@ public:
 private:
   static const char*  store_command_name(const char* name);
 
-  static std::vector<std::unique_ptr<const char>> m_command_names;
+  static std::vector<std::string> m_command_names;
 
   slot_download       m_slotFindDownload;
   slot_file           m_slotFindFile;

src/rpc/xmlrpc_c.cc

@@ -272,10 +272,10 @@ object_to_xmlrpc(xmlrpc_env* env, const torrent::Object& object) {
 #ifdef XMLRPC_HAVE_I8
     if (rpc.dialect() != XmlRpc::dialect_generic)
       return xmlrpc_i8_new(env, object.as_value());
-#else
-    return xmlrpc_int_new(env, object.as_value());
 #endif
 
+    return xmlrpc_int_new(env, object.as_value());
+
   case torrent::Object::TYPE_STRING:
   {
 #ifdef XMLRPC_HAVE_I8

src/rpc/xmlrpc_tinyxml2.cc

@@ -66,56 +66,85 @@ xml_value_to_object(const tinyxml2::XMLNode* elem) {
   if (elem == nullptr) {
     throw rpc_error(XMLRPC_INTERNAL_ERROR, "received null element to convert");
   }
+
   if (std::strncmp(elem->Value(), "value", sizeof("value")) != 0) {
     throw rpc_error(XMLRPC_INTERNAL_ERROR, "received non-value element to convert");
   }
+
   auto root_element = elem->FirstChild();
+
+  if (root_element == nullptr)
+    throw rpc_error(XMLRPC_TYPE_ERROR, "empty value element");
+
   auto root_type    = root_element->Value();
+
   if (std::strncmp(root_type, "string", sizeof("string")) == 0) {
     auto child_element = root_element->FirstChild();
-    if (child_element == nullptr) {
+
+    if (child_element == nullptr)
       return torrent::Object("");
-    }
+
     return torrent::Object(child_element->ToText()->Value());
+
   } else if (std::strncmp(root_type, "int", sizeof("int")) == 0 ||
              std::strncmp(root_type, "i4", sizeof("i4")) == 0 ||
              std::strncmp(root_type, "i8", sizeof("i8")) == 0) {
     return torrent::Object(element_to_int(root_element));
+
   } else if (std::strncmp(root_type, "boolean", sizeof("boolean")) == 0) {
-    auto boolean_text = std::string(root_element->FirstChild()->ToText()->Value());
+    auto child_element = root_element->FirstChild();
-    if (boolean_text == "1") {
+
+    if (child_element == nullptr)
+      throw rpc_error(XMLRPC_TYPE_ERROR, "empty boolean element");
+
+    auto boolean_text = std::string(child_element->ToText()->Value());
+
+    if (boolean_text == "1")
       return torrent::Object((int64_t)1);
-    } else if (boolean_text == "0") {
+    else if (boolean_text == "0")
       return torrent::Object((int64_t)0);
-    }
+
     throw rpc_error(XMLRPC_TYPE_ERROR, "unknown boolean value: " + boolean_text);
+
   } else if (std::strncmp(root_type, "array", sizeof("array")) == 0) {
     auto  array_raw    = torrent::Object::create_list();
     auto& array        = array_raw.as_list();
     auto  data_element = root_element->ToElement()->FirstChildElement("data");
+
     if (data_element == nullptr)
       throw rpc_error(XMLRPC_PARSE_ERROR, "could not find expected data element in array");
-    for (auto child = data_element->FirstChildElement("value"); child; child = child->NextSiblingElement("value")) {
+
+    for (auto child = data_element->FirstChildElement("value"); child; child = child->NextSiblingElement("value"))
       array.push_back(xml_value_to_object(child));
-    }
+
     return array_raw;
   } else if (std::strncmp(root_type, "struct", sizeof("struct")) == 0) {
     auto  map_raw = torrent::Object::create_map();
     auto& map     = map_raw.as_map();
+
     for (auto child = root_element->FirstChildElement("member"); child; child = child->NextSiblingElement("member")) {
-      auto key = child->FirstChildElement("name")->GetText();
+      auto name_element = child->FirstChildElement("name");
-      map[key] = std::move(xml_value_to_object(child->FirstChildElement("value")));
+
+      if (name_element == nullptr)
+        throw rpc_error(XMLRPC_PARSE_ERROR, "struct member missing name element");
+
+      map[name_element->GetText()] = std::move(xml_value_to_object(child->FirstChildElement("value")));
     }
+
     return map_raw;
+
   } else if (std::strncmp(root_type, "base64", sizeof("base64")) == 0) {
     auto child_element = root_element->FirstChild();
-    if (child_element == nullptr) {
+
+    if (child_element == nullptr)
       return torrent::Object("");
-    }
+
     return torrent::Object(utils::decode_base64(utils::remove_newlines(child_element->ToText()->Value())));
+
   } else {
     throw rpc_error(XMLRPC_INTERNAL_ERROR, "received unsupported value type: " + std::string(root_type));
   }
+
   return torrent::Object();
 }