<common> fix formatter buffer out-of-bounds

  The length of buf in Formatter is 1024. If the output was truncated due to size limit,
  then the return value is the number of characters (excluding the terminating null byte)
  which would have been written to the final string if enough space had been available.
  So XMLFormatter::dump_format_va will create a string_view that out of bounds of buf.

  use boost small_vector to allocate more buffer when content is too
  long to fit in the buffer pre-alloced

Signed-off-by: liubingrun <liubr1@chinatelecom.cn>
(cherry picked from commit 207003c512396b1381abbf1a4929186c21827060)

diff --git a/src/common/Formatter.cc b/src/common/Formatter.cc
index d884b161bd60168fb91ce9e73ca365886ca61800..7c61aebbc54fc45b7de56473a3ef3932618b54ed 100644 (file)
--- a/src/common/Formatter.cc
+++ b/src/common/Formatter.cc
@@ -23,6 +23,8 @@
 #include <algorithm>
 #include <set>
 #include <limits>
+#include <utility>
+#include <boost/container/small_vector.hpp>
 
 // -----------------------
 namespace ceph {
@@ -358,10 +360,21 @@ std::ostream& JSONFormatter::dump_stream(std::string_view name)
 
 void JSONFormatter::dump_format_va(std::string_view name, const char *ns, bool quoted, const char *fmt, va_list ap)
 {
-  char buf[LARGE_SIZE];
-  vsnprintf(buf, LARGE_SIZE, fmt, ap);
+  auto buf = boost::container::small_vector<char, LARGE_SIZE>{
+      LARGE_SIZE, boost::container::default_init};
 
-  add_value(name, buf, quoted);
+  va_list ap_copy;
+  va_copy(ap_copy, ap);
+  int len = vsnprintf(buf.data(), buf.size(), fmt, ap_copy);
+  va_end(ap_copy);
+
+  if (std::cmp_greater_equal(len, buf.size())) {
+    // output was truncated, allocate a buffer large enough
+    buf.resize(len + 1, boost::container::default_init);
+    vsnprintf(buf.data(), buf.size(), fmt, ap);
+  }
+
+  add_value(name, buf.data(), quoted);
 }
 
 int JSONFormatter::get_len() const
@@ -543,15 +556,27 @@ std::ostream& XMLFormatter::dump_stream(std::string_view name)
 
 void XMLFormatter::dump_format_va(std::string_view name, const char *ns, bool quoted, const char *fmt, va_list ap)
 {
-  char buf[LARGE_SIZE];
-  size_t len = vsnprintf(buf, LARGE_SIZE, fmt, ap);
+  auto buf = boost::container::small_vector<char, LARGE_SIZE>{
+      LARGE_SIZE, boost::container::default_init};
+
+  va_list ap_copy;
+  va_copy(ap_copy, ap);
+  int len = vsnprintf(buf.data(), buf.size(), fmt, ap_copy);
+  va_end(ap_copy);
+
+  if (std::cmp_greater_equal(len, buf.size())) {
+    // output was truncated, allocate a buffer large enough
+    buf.resize(len + 1, boost::container::default_init);
+    vsnprintf(buf.data(), buf.size(), fmt, ap);
+  }
+
   auto e = get_xml_name(name);
 
   print_spaces();
   if (ns) {
-    m_ss << "<" << e << " xmlns=\"" << ns << "\">" << xml_stream_escaper(std::string_view(buf, len)) << "</" << e << ">";
+    m_ss << "<" << e << " xmlns=\"" << ns << "\">" << xml_stream_escaper(std::string_view(buf.data(), len)) << "</" << e << ">";
   } else {
-    m_ss << "<" << e << ">" << xml_stream_escaper(std::string_view(buf, len)) << "</" << e << ">";
+    m_ss << "<" << e << ">" << xml_stream_escaper(std::string_view(buf.data(), len)) << "</" << e << ">";
   }
 
   if (m_pretty)
@@ -920,14 +945,26 @@ void TableFormatter::dump_format_va(std::string_view name,
                                    const char *fmt, va_list ap)
 {
   finish_pending_string();
-  char buf[LARGE_SIZE];
-  vsnprintf(buf, LARGE_SIZE, fmt, ap);
+  auto buf = boost::container::small_vector<char, LARGE_SIZE>{
+      LARGE_SIZE, boost::container::default_init};
+
+  va_list ap_copy;
+  va_copy(ap_copy, ap);
+  int len = vsnprintf(buf.data(), buf.size(), fmt, ap_copy);
+  va_end(ap_copy);
+
+  if (std::cmp_greater_equal(len, buf.size())) {
+    // output was truncated, allocate a buffer large enough
+    buf.resize(len + 1, boost::container::default_init);
+    vsnprintf(buf.data(), buf.size(), fmt, ap); 
+  }
 
   size_t i = m_vec_index(name);
   if (ns) {
-    m_ss << ns << "." << buf;
-  } else
-    m_ss << buf;
+    m_ss << ns << "." << buf.data();
+  } else {
+    m_ss << buf.data();
+  }
 
   m_vec[i].push_back(std::make_pair(get_section_name(name), m_ss.str()));
   m_ss.clear();

diff --git a/src/test/common/test_json_formatter.cc b/src/test/common/test_json_formatter.cc
index 9cc19b24ad1b873d5b5640e6556dfbfc907c8723..d0ddd262c0a1ab655cfb18bc335ae0b140097341 100644 (file)
--- a/src/test/common/test_json_formatter.cc
+++ b/src/test/common/test_json_formatter.cc
@@ -102,3 +102,27 @@ TEST(formatter, dump_inf_or_nan)
   EXPECT_EQ(parser.find_obj("nan_val")->get_data(), "null");
   EXPECT_EQ(parser.find_obj("nan_val_alt")->get_data(), "null");
 }
+
+TEST(formatter, dump_large_item) {
+  JSONFormatter formatter;
+  formatter.open_object_section("large_item");
+
+  std::string base_url("http://example.com");
+  std::string bucket_name("bucket");
+  std::string object_key(1024, 'a');
+
+  std::string full_url = base_url + "/" + bucket_name + "/" + object_key;
+  formatter.dump_format("Location", "%s/%s/%s", base_url.c_str(), bucket_name.c_str(), object_key.c_str());
+
+  formatter.close_section();
+  bufferlist bl;
+  formatter.flush(bl);
+
+  // std::cout << std::string(bl.c_str(), bl.length()) << std::endl;
+
+  JSONParser parser;
+  parser.parse(bl.c_str(), bl.length());
+
+  EXPECT_TRUE(parser.parse(bl.c_str(), bl.length()));
+  EXPECT_EQ(parser.find_obj("Location")->get_data(), full_url);
+}

diff --git a/src/test/common/test_tableformatter.cc b/src/test/common/test_tableformatter.cc
index b152014a2b5e3d67ea5ad64fd6d9ddaa86c3920a..90de133d315550e00d2c2bf1fbe6934f99df11db 100644 (file)
--- a/src/test/common/test_tableformatter.cc
+++ b/src/test/common/test_tableformatter.cc
@@ -250,6 +250,23 @@ TEST(tableformatter, multiline_keyval)
   EXPECT_EQ(cmp, sout.str());
 }
 
+TEST(tableformatter, dump_large_item) {
+  std::stringstream sout;
+  TableFormatter* formatter = (TableFormatter*) Formatter::create("table-kv");
+
+  std::string base_url("http://example.com");
+  std::string bucket_name("bucket");
+  std::string object_key(1024, 'a');
+
+  std::string full_url = base_url + "/" + bucket_name + "/" + object_key;
+  formatter->dump_format("Location", "%s/%s/%s", base_url.c_str(), bucket_name.c_str(), object_key.c_str());
+  formatter->flush(sout);
+  delete formatter;
+
+  std::string cmp = "key::Location=\"" + full_url + "\" \n";
+  EXPECT_EQ(cmp, sout.str());
+}
+
 /*
  * Local Variables:
  * compile-command: "cd ../.. ; make -j4 &&

diff --git a/src/test/common/test_xmlformatter.cc b/src/test/common/test_xmlformatter.cc
index 9ac6dde456e831d2e20e5217579ed1b6d93a9065..abbe9e4e25e94be3173c05f8f95639e153b4b963 100644 (file)
--- a/src/test/common/test_xmlformatter.cc
+++ b/src/test/common/test_xmlformatter.cc
@@ -163,3 +163,25 @@ TEST(xmlformatter, pretty_lowercased_underscored)
     "<string_item>String</string_item>\n\n";
   EXPECT_EQ(cmp, sout.str());
 }
+
+TEST(xmlformatter, dump_format_large_item)
+{
+  std::stringstream sout;
+  XMLFormatter formatter(
+      true,  // pretty
+      false, // lowercased
+      false); // underscored
+
+  std::string base_url("http://example.com");
+  std::string bucket_name("bucket");
+  std::string object_key(1024, 'a');
+
+  formatter.dump_format("Location", "%s/%s/%s", base_url.c_str(), bucket_name.c_str(), object_key.c_str());
+
+  formatter.flush(sout);
+
+  std::string uri = base_url + "/" + bucket_name + "/" + object_key;
+  std::string expected_output = "<Location>" + uri + "</Location>\n\n";
+
+  EXPECT_EQ(expected_output, sout.str());
+}
\ No newline at end of file