Merge pull request ceph#60429 from VVoidV/lbr-fix-formatter

<common> fix formatter buffer out-of-bounds

Reviewed-by: Casey Bodley <[email protected]>

Author: cbodley

src/common/Formatter.cc

@@ -23,6 +23,8 @@
 #include <algorithm>
 #include <set>
 #include <limits>
+#include <utility>
+#include <boost/container/small_vector.hpp>
 
 // -----------------------
 namespace ceph {
@@ -365,10 +367,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
@@ -550,15 +563,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)
@@ -927,14 +952,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();

src/common/HTMLFormatter.cc

@@ -23,6 +23,8 @@
 #include <stdlib.h>
 #include <string>
 #include <string.h>     // for strdup
+#include <boost/container/small_vector.hpp>
+#include <utility>  // for std::cmp_greater_equal
 
 #include "common/escape.h"
 
@@ -138,17 +140,27 @@ std::ostream& HTMLFormatter::dump_stream(std::string_view name)
 
 void HTMLFormatter::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);
+  size_t len = vsnprintf(buf.data(), buf.size(), fmt, ap);
+  va_end(ap_copy);
+
+  if(std::cmp_greater_equal(len, buf.size())){
+    buf.resize(len + 1, boost::container::default_init);
+    vsnprintf(buf.data(), buf.size(), fmt, ap_copy);
+  }
 
   std::string e(name);
   print_spaces();
   if (ns) {
     m_ss << "<li xmlns=\"" << ns << "\">" << e << ": "
-	 << xml_stream_escaper(std::string_view(buf, len)) << "</li>";
+	 << xml_stream_escaper(std::string_view(buf.data(), len)) << "</li>";
   } else {
     m_ss << "<li>" << e << ": "
-	 << xml_stream_escaper(std::string_view(buf, len)) << "</li>";
+	 << xml_stream_escaper(std::string_view(buf.data(), len)) << "</li>";
   }
 
   if (m_pretty)

src/test/common/CMakeLists.txt

@@ -229,6 +229,12 @@ add_executable(unittest_xmlformatter
 add_ceph_unittest(unittest_xmlformatter)
 target_link_libraries(unittest_xmlformatter ceph-common)
 
+add_executable(unittest_htmlformatter
+    test_htmlformatter.cc
+    )
+add_ceph_unittest(unittest_htmlformatter)
+target_link_libraries(unittest_htmlformatter ceph-common)
+
 # unittest_bit_vector
 add_executable(unittest_bit_vector
   test_bit_vector.cc

src/test/common/test_htmlformatter.cc

@@ -0,0 +1,26 @@
+#include "gtest/gtest.h"
+
+#include "common/HTMLFormatter.h"
+#include <sstream>
+#include <string>
+
+using namespace ceph;
+
+TEST(htmlformatter, dump_format_large_item)
+{
+  std::stringstream sout;
+  HTMLFormatter formatter(false);
+
+  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 = "<li>Location: " + uri + "</li>";
+
+  EXPECT_EQ(expected_output, sout.str());
+}

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);
+}

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 &&

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());
+}