From: liubingrun Date: Thu, 24 Oct 2024 10:02:12 +0000 (-0400) Subject: fix formatter buffer out-of-bounds X-Git-Tag: testing/wip-khiremat-testing-20250422.120708-squid-debug~76^2~1 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=2cd364d11bcd0454c394459389b9e5a07a074827;p=ceph-ci.git 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) Conflicts: src/test/common/test_json_formatter.cc missing test dump_inf_or_nan --- diff --git a/src/common/Formatter.cc b/src/common/Formatter.cc index b43e04a8bd0..a653a07929b 100644 --- a/src/common/Formatter.cc +++ b/src/common/Formatter.cc @@ -22,6 +22,8 @@ #include #include #include +#include +#include // ----------------------- namespace ceph { @@ -345,10 +347,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 @@ -530,15 +543,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) @@ -907,14 +932,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 8a0f547a929..983f1021527 100644 --- a/src/test/common/test_json_formatter.cc +++ b/src/test/common/test_json_formatter.cc @@ -79,3 +79,27 @@ TEST(formatter, utime) EXPECT_EQ(input.sec(), output.sec()); EXPECT_EQ(input.nsec(), output.nsec()); } + +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 b152014a2b5..90de133d315 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 9ac6dde456e..abbe9e4e25e 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