diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f0f8597f..4eabd4a2e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,10 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- Markdown replaces NUL bytes in inline code and rejects oversized input. + RTF rejects unreadable streams; legacy PowerPoint rejects truncated text records. + Both binary readers safely propagate I/O exceptions. + - OOXML parts consistently resolve absolute package targets. Optional relationships skip external or invalid paths. DOCX opens without styles, and PPTX retains a slide’s own background when it has no layout relationship. diff --git a/src/odr/internal/markdown/AGENTS.md b/src/odr/internal/markdown/AGENTS.md index 40f45ff55..3e2647d10 100644 --- a/src/odr/internal/markdown/AGENTS.md +++ b/src/odr/internal/markdown/AGENTS.md @@ -107,7 +107,8 @@ md4c reports a NUL as `MD_TEXT_NULLCHAR` from a verbatim block too, and then re-sends the byte at the head of the next `MD_TEXT_CODE` or `MD_TEXT_HTML` chunk. So the U+FFFD is buffered into `m_code` like the rest of a code block, dropped like the rest of a raw html block, and the re-sent byte is skipped -where the code text is buffered. +where the code text is buffered. Inline code delivers NUL bytes directly in +`MD_TEXT_CODE`; each becomes U+FFFD. ### Nesting is bounded diff --git a/src/odr/internal/markdown/markdown_parser.cpp b/src/odr/internal/markdown/markdown_parser.cpp index b7b9ba9ed..1ba37c065 100644 --- a/src/odr/internal/markdown/markdown_parser.cpp +++ b/src/odr/internal/markdown/markdown_parser.cpp @@ -213,7 +213,7 @@ class Parser final { table_detail->col_count); push_(element_id); - for (unsigned i = 0; i < table_detail->col_count; ++i) { + for (std::size_t i = 0; i < table_detail->col_count; ++i) { const auto &[column_id, column] = m_registry->create_element(ElementType::table_column); m_registry->append_column(element_id, column_id); @@ -350,7 +350,15 @@ class Parser final { } } } else { - append_text_(text); + // Inline code delivers NUL bytes without a separate NULLCHAR callback. + std::string_view remaining = text; + for (auto pos = remaining.find('\0'); pos != std::string_view::npos; + pos = remaining.find('\0')) { + append_text_(remaining.substr(0, pos)); + append_text_(replacement_character); + remaining.remove_prefix(pos + 1); + } + append_text_(remaining); } break; case MD_TEXT_HTML: @@ -523,6 +531,9 @@ namespace odr::internal { ElementIdentifier markdown::parse_tree(ElementRegistry ®istry, StyleRegistry &style_registry, const std::string_view text) { + if (!std::in_range(text.size())) { + throw std::length_error("markdown: input exceeds md4c's size limit"); + } Parser parser(registry, style_registry); MD_PARSER md_parser{}; diff --git a/src/odr/internal/oldms/presentation/ppt_io.cpp b/src/odr/internal/oldms/presentation/ppt_io.cpp index cb41d61fc..41106a9f4 100644 --- a/src/odr/internal/oldms/presentation/ppt_io.cpp +++ b/src/odr/internal/oldms/presentation/ppt_io.cpp @@ -71,13 +71,7 @@ std::u16string presentation::read_raw_text_chars(std::istream &in, std::string presentation::read_raw_text_bytes(std::istream &in, const std::uint32_t rec_len) { - std::string buffer; - // The record length is only a claim, so the buffer is cut to what was read. - buffer.resize_and_overwrite(rec_len, [&](char *out, const std::size_t size) { - in.read(out, static_cast(size)); - return static_cast(in.gcount()); - }); - return buffer; + return util::byte_stream::read_u8s(in, rec_len); } std::string presentation::decode_text_bytes(const std::string_view bytes) { diff --git a/src/odr/internal/rtf/rtf_tokenizer.cpp b/src/odr/internal/rtf/rtf_tokenizer.cpp index a538874af..a3f7e8e7e 100644 --- a/src/odr/internal/rtf/rtf_tokenizer.cpp +++ b/src/odr/internal/rtf/rtf_tokenizer.cpp @@ -1,5 +1,6 @@ #include +#include #include #include @@ -35,10 +36,10 @@ char two_hex_to_char(const char first, const char second) { } // namespace Tokenizer::Tokenizer(std::istream &in) : m_in{&in}, m_sb{in.rdbuf()} { - // One-time stream preparation (flush tied streams, state check) for the raw - // streambuf reads below; a sentry's effects live entirely in its - // constructor, so it is not kept as state. - const std::istream::sentry se(in, true); + const std::istream::sentry sentry(in, true); + if (!sentry) { + throw std::ios_base::failure("rtf: unreadable input stream"); + } } Tokenizer::int_type Tokenizer::geti() { @@ -58,33 +59,6 @@ Tokenizer::char_type Tokenizer::bumpc() { return static_cast(c); } -std::string Tokenizer::bumpnc(const std::size_t n) { - // In chunks, because `\binN` takes @p n from the file: allocating it up front - // lets a 22-byte rtf demand the two gigabytes its parameter claims, where - // growing with what the stream actually delivers throws after one chunk. - static constexpr std::size_t chunk_size = 4096; - - std::string result; - while (result.size() < n) { - const std::size_t offset = result.size(); - const auto m = - static_cast(std::min(chunk_size, n - offset)); - // The callback must not throw, so a short read reports itself by - // shrinking the string back. - result.resize_and_overwrite(offset + static_cast(m), - [&](char *out, const std::size_t size) { - return m_sb->sgetn(out + offset, m) == m - ? size - : offset; - }); - if (result.size() == offset) { - m_in->setstate(std::ios::eofbit); - throw std::runtime_error("unexpected stream exhaust"); - } - } - return result; -} - Token Tokenizer::read_token() { const int_type i = geti(); if (i == eof) { @@ -178,7 +152,9 @@ Token Tokenizer::read_control() { // desync the group nesting. if (name == "bin") { const std::int32_t n = parameter.value_or(0); - return Binary{n > 0 ? bumpnc(static_cast(n)) : std::string()}; + return Binary{n > 0 ? util::byte_stream::read_u8s( + *m_in, static_cast(n)) + : std::string()}; } return ControlWord{std::move(name), parameter}; diff --git a/src/odr/internal/rtf/rtf_tokenizer.hpp b/src/odr/internal/rtf/rtf_tokenizer.hpp index a7c1107f1..462fecfb2 100644 --- a/src/odr/internal/rtf/rtf_tokenizer.hpp +++ b/src/odr/internal/rtf/rtf_tokenizer.hpp @@ -29,7 +29,6 @@ class Tokenizer final { int_type geti(); char_type bumpc(); - std::string bumpnc(std::size_t n); /// With the leading `\` already consumed. [[nodiscard]] Token read_control(); diff --git a/test/src/internal/markdown/markdown_file_test.cpp b/test/src/internal/markdown/markdown_file_test.cpp index e42de69be..93b186fa7 100644 --- a/test/src/internal/markdown/markdown_file_test.cpp +++ b/test/src/internal/markdown/markdown_file_test.cpp @@ -457,6 +457,9 @@ TEST(MarkdownDocument, a_null_byte_becomes_the_replacement_character) { const std::string in_text("a\0b\n", 4); const std::string in_code("```\na\0b\n```\n", 12); const std::string in_html("
\0
\n", 13); + const Document inline_code = + document_of(std::string("`a\0b`", 5), TextEncoding::utf8); + EXPECT_EQ(text_of(root(inline_code)), "a\ufffdb"); const Document text = document_of(in_text, TextEncoding::utf8); const Document code = document_of(in_code, TextEncoding::utf8); diff --git a/test/src/internal/oldms/ppt_test.cpp b/test/src/internal/oldms/ppt_test.cpp index f1016543d..9a03f5895 100644 --- a/test/src/internal/oldms/ppt_test.cpp +++ b/test/src/internal/oldms/ppt_test.cpp @@ -8,7 +8,10 @@ #include #include +#include +#include #include +#include #include @@ -192,3 +195,17 @@ TEST(OldMs, ppt_style_various) { ASSERT_TRUE(link.font_color.has_value()); EXPECT_EQ(link.font_color->rgb(), 0x0000FFu); } + +TEST(OldMs, ppt_text_bytes_require_complete_record) { + using internal::oldms::presentation::read_raw_text_bytes; + std::istringstream valid("abcNEXT"); + EXPECT_EQ(read_raw_text_bytes(valid, 3), "abc"); + EXPECT_EQ(valid.peek(), 'N'); + std::istringstream truncated("ab"); + EXPECT_THROW( + read_raw_text_bytes(truncated, std::numeric_limits::max()), + std::runtime_error); + std::istringstream throwing("ab"); + throwing.exceptions(std::ios::failbit | std::ios::badbit); + EXPECT_THROW(read_raw_text_bytes(throwing, 3), std::ios_base::failure); +} diff --git a/test/src/internal/rtf/rtf_tokenizer_test.cpp b/test/src/internal/rtf/rtf_tokenizer_test.cpp index 6643d705b..cbd30e93f 100644 --- a/test/src/internal/rtf/rtf_tokenizer_test.cpp +++ b/test/src/internal/rtf/rtf_tokenizer_test.cpp @@ -153,3 +153,25 @@ TEST(RtfTokenizer, a_binary_length_is_not_allocated_before_it_is_read) { TEST(RtfTokenizer, a_trailing_backslash_throws) { EXPECT_THROW(tokens("ab\\"), std::runtime_error); } + +TEST(RtfTokenizer, unreadable_stream_throws) { + std::istream missing(nullptr); + EXPECT_THROW(rtf::Tokenizer{missing}, std::ios_base::failure); + std::istringstream failed("text"); + failed.setstate(std::ios::failbit); + EXPECT_THROW(rtf::Tokenizer{failed}, std::ios_base::failure); +} + +TEST(RtfTokenizer, binary_read_exception_propagates) { + class FailingBuffer final : public std::stringbuf { + public: + FailingBuffer() : std::stringbuf("\\bin1 x") {} + std::streamsize xsgetn(char *, std::streamsize) override { + throw std::runtime_error("read failed"); + } + } buffer; + std::istream in(&buffer); + in.exceptions(std::ios::badbit); + rtf::Tokenizer tokenizer(in); + EXPECT_THROW(static_cast(tokenizer.read_token()), std::runtime_error); +}