From c88550c7b8fcfeabb79fa95c37c9d5d692de5ba2 Mon Sep 17 00:00:00 2001 From: liubingrun Date: Thu, 24 Oct 2024 06:02:12 -0400 Subject: [PATCH] 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 (cherry picked from commit 207003c512396b1381abbf1a4929186c21827060) --- src/common/Formatter.cc | 61 +++++++++++++++++++++----- src/test/common/test_json_formatter.cc | 24 ++++++++++ src/test/common/test_tableformatter.cc | 17 +++++++ src/test/common/test_xmlformatter.cc | 22 ++++++++++ 4 files changed, 112 insertions(+), 12 deletions(-) diff --git a/src/common/Formatter.cc b/src/common/Formatter.cc index d884b161bd601..7c61aebbc54fc 100644 --- a/src/common/Formatter.cc +++ b/src/common/Formatter.cc @@ -23,6 +23,8 @@ #include #include #include +#include +#include // ----------------------- 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{ + 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{ + 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)) << ""; + m_ss << "<" << e << " xmlns=\"" << ns << "\">" << xml_stream_escaper(std::string_view(buf.data(), len)) << ""; } else { - m_ss << "<" << e << ">" << xml_stream_escaper(std::string_view(buf, len)) << ""; + m_ss << "<" << e << ">" << xml_stream_escaper(std::string_view(buf.data(), len)) << ""; } 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{ + 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 9cc19b24ad1b8..d0ddd262c0a1a 100644 --- 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 b152014a2b5e3..90de133d31555 100644 --- 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 9ac6dde456e83..abbe9e4e25e94 100644 --- a/src/test/common/test_xmlformatter.cc +++ b/src/test/common/test_xmlformatter.cc @@ -163,3 +163,25 @@ TEST(xmlformatter, pretty_lowercased_underscored) "String\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 = "" + uri + "\n\n"; + + EXPECT_EQ(expected_output, sout.str()); +} \ No newline at end of file -- 2.39.5