diff --git a/CHANGELOG.md b/CHANGELOG.md index 607141f0c..f6cb68083 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,9 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- Legacy Word bounds font and piece tables by their declared lengths, checks + piece coverage and offsets, and reads PLC entries without unaligned access. + - Legacy Word validates FIB signatures and extension lengths. It stores the shared header fields by value, removing unsafe deletion through base pointers. diff --git a/src/odr/internal/oldms/text/AGENTS.md b/src/odr/internal/oldms/text/AGENTS.md index 483b73064..ec84e0af2 100644 --- a/src/odr/internal/oldms/text/AGENTS.md +++ b/src/odr/internal/oldms/text/AGENTS.md @@ -43,17 +43,20 @@ a future base `nFib` may still use the common prefix. No versioned object is allocated or deleted through a non-virtual base. **Piece table.** The `PlcPcd` is `n+1` ascending CP boundaries followed by `n` -`Pcd`s. `PlcPcdMap` is a zero-copy view over the raw bytes. Each `Pcd`'s -`FcCompressed`: `fCompressed == 0` is UTF-16 at `fc`, `== 1` is one byte per -CP at `fc/2`, with `0x82` to `0x9F` remapped by `uncompress_char` (§2.9.73) -and every other byte `b` mapped to `U+00b`. +`Pcd`s. `PlcPcdMap` views the raw bytes and copies each checked entry to avoid +unaligned access. The Clx and font table are bounded by their declared lengths. +Each `Pcd`'s `FcCompressed`: `fCompressed == 0` is UTF-16 at `fc`, `== 1` is +one byte per CP at `fc/2`, with `0x82` to `0x9F` remapped by `uncompress_char` +(§2.9.73) and every other byte `b` mapped to `U+00b`. **Fail early.** Throw on: `nFib` below `nFib97` or an unknown `nFibNew`; a `ccpText` with the sign bit set (§2.5.5); a `csw` or `cslw` count too small for an array the module reads; an unexpected Clx lead byte (not `0x01` or -`0x02`); non-ascending CP boundaries; a bad compressed byte; early EOF. Pass +`0x02`); non-contiguous CP boundaries; piece offsets beyond the 32-bit range; +missing body coverage; an FFN shorter than its fixed part; early EOF. Pass through what is not modelled: text after the main body, `Prc` formatting -runs, and every control or field character `TextCleaner` drops. +runs, every control or field character `TextCleaner` drops, and a font name +with an odd byte, extra bytes or no NUL. **Direct character formatting, resolved to styled spans.** Each span and each paragraph stores a style index in the element registry, resolved through the diff --git a/src/odr/internal/oldms/text/doc_helper.cpp b/src/odr/internal/oldms/text/doc_helper.cpp index 14cb73fa0..3ede47830 100644 --- a/src/odr/internal/oldms/text/doc_helper.cpp +++ b/src/odr/internal/oldms/text/doc_helper.cpp @@ -3,7 +3,6 @@ #include #include #include -#include #include #include @@ -12,6 +11,7 @@ #include #include #include +#include namespace odr::internal::oldms::text { @@ -62,11 +62,11 @@ text::CharacterIndex text::read_character_index(std::istream &in) { throw std::runtime_error("Unexpected input: " + std::to_string(c)); } const std::uint32_t lcb = util::byte_stream::read(in); - std::string plcPcd = util::stream::read(in, lcb); + std::string plcPcd = util::byte_stream::read_u8s(in, lcb); const PlcPcdMap plc_pcd_map(plcPcd.data(), plcPcd.size()); - const std::uint32_t count = plc_pcd_map.n(); - for (std::uint32_t i = 0; i < count; ++i) { + const std::size_t count = plc_pcd_map.n(); + for (std::size_t i = 0; i < count; ++i) { // aCp is strictly ascending ([MS-DOC] 2.8.35); otherwise the piece // length below would wrap around. if (plc_pcd_map.aCP(i + 1) <= plc_pcd_map.aCP(i)) { @@ -97,7 +97,8 @@ text::read_character_runs(std::istream &document_stream, const TextStyle default_style = styles.at(0); table_stream.seekg(plcf_bte_chpx.fc); - std::string plc_bytes = util::stream::read(table_stream, plcf_bte_chpx.lcb); + std::string plc_bytes = + util::byte_stream::read_u8s(table_stream, plcf_bte_chpx.lcb); const PlcBteChpxMap plc(plc_bytes.data(), plc_bytes.size()); // One resolved style per distinct Chpx byte sequence. @@ -108,6 +109,9 @@ text::read_character_runs(std::istream &document_stream, } const auto [it, inserted] = style_cache.try_emplace(std::string(grpprl)); if (inserted) { + if (!std::in_range(styles.size())) { + throw std::length_error("doc: too many character styles"); + } styles.push_back( apply_character_sprms(default_style, grpprl, font_names)); it->second = static_cast(styles.size() - 1); @@ -117,8 +121,8 @@ text::read_character_runs(std::istream &document_stream, // A ChpxFkp page ([MS-DOC] 2.9.33) is 512 bytes at pn * 512; rgb[j] * 2 // locates the run's Chpx within it, 0 meaning default properties. - const std::uint32_t page_count = plc.n(); - for (std::uint32_t i = 0; i < page_count; ++i) { + const std::size_t page_count = plc.n(); + for (std::size_t i = 0; i < page_count; ++i) { std::array page; document_stream.seekg(static_cast(plc.aData(i).pn) * 512); document_stream.read(page.data(), page.size()); diff --git a/src/odr/internal/oldms/text/doc_helper.hpp b/src/odr/internal/oldms/text/doc_helper.hpp index 9087c6540..f82f90ec3 100644 --- a/src/odr/internal/oldms/text/doc_helper.hpp +++ b/src/odr/internal/oldms/text/doc_helper.hpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -28,11 +29,18 @@ class CharacterIndex { void append(const std::size_t start_cp, const std::size_t length_cp, const std::size_t data_offset, const bool is_compressed) { - const std::size_t end_cp = start_cp + length_cp; - if (end_cp < last_cp()) { + if (start_cp != last_cp() || length_cp == 0 || + length_cp > std::numeric_limits::max() - start_cp) { + throw std::runtime_error( + "doc: character pieces must be contiguous and nonempty"); + } + constexpr auto max_offset = std::numeric_limits::max(); + if (data_offset > max_offset || + length_cp > (max_offset - data_offset) / (is_compressed ? 1 : 2)) { throw std::runtime_error( - "append must be used in order of increasing start_cp"); + "doc: character piece exceeds stream offset range"); } + const std::size_t end_cp = start_cp + length_cp; m_entries.emplace_back(end_cp, data_offset, is_compressed); } @@ -74,7 +82,7 @@ class CharacterIndex { } bool operator==(const Iterator &other) const { - return m_index == other.m_index; + return m_parent == other.m_parent && m_index == other.m_index; } private: @@ -93,7 +101,7 @@ class CharacterIndex { [[nodiscard]] Iterator end() const { return {*this, m_entries.size()}; } [[nodiscard]] Iterator find(const std::size_t cp) const { const auto it = - std::ranges::lower_bound(m_entries, cp, {}, &InternalEntry::end_cp); + std::ranges::upper_bound(m_entries, cp, {}, &InternalEntry::end_cp); if (it == m_entries.end()) { return end(); } diff --git a/src/odr/internal/oldms/text/doc_parser.cpp b/src/odr/internal/oldms/text/doc_parser.cpp index 5a669852b..23ab57d8a 100644 --- a/src/odr/internal/oldms/text/doc_parser.cpp +++ b/src/odr/internal/oldms/text/doc_parser.cpp @@ -9,6 +9,8 @@ #include #include #include +#include +#include #include #include @@ -107,6 +109,9 @@ struct StyledRun { std::vector decode_styled_runs( std::istream &document_stream, const text::CharacterIndex &character_index, const text::CharacterRuns &character_runs, const std::size_t ccp_text) { + if (character_index.last_cp() < ccp_text) { + throw std::runtime_error("doc: piece table does not cover the body"); + } std::vector runs; std::size_t consumed_cp = 0; @@ -158,7 +163,11 @@ ElementIdentifier text::parse_tree(ElementRegistry ®istry, const abstract::ReadableFilesystem &files) { auto [root_id, _] = registry.create_element(ElementType::root); - const auto document_stream = files.open(AbsPath("/WordDocument"))->stream(); + const auto document_file = files.open(AbsPath("/WordDocument")); + if (document_file == nullptr) { + throw std::runtime_error("doc: missing WordDocument stream"); + } + const auto document_stream = document_file->stream(); ParsedFib fib; read(*document_stream, fib); @@ -181,7 +190,10 @@ ElementIdentifier text::parse_tree(ElementRegistry ®istry, style_registry = StyleRegistry(std::move(styles)); table_stream->seekg(fib.fibRgFcLcb.clx.fc); - const CharacterIndex character_index = read_character_index(*table_stream); + const std::string clx_bytes = + util::byte_stream::read_u8s(*table_stream, fib.fibRgFcLcb.clx.lcb); + util::stream::ViewStream clx_stream(clx_bytes); + const CharacterIndex character_index = read_character_index(clx_stream); const auto ccp_text = static_cast(fib.ccpText()); const std::vector runs = decode_styled_runs( diff --git a/src/odr/internal/oldms/text/doc_structs.hpp b/src/odr/internal/oldms/text/doc_structs.hpp index a7326f29b..67f84ab79 100644 --- a/src/odr/internal/oldms/text/doc_structs.hpp +++ b/src/odr/internal/oldms/text/doc_structs.hpp @@ -1,5 +1,7 @@ #pragma once +#include + #include #include #include @@ -273,20 +275,31 @@ template class PlcMap { : m_data(data), m_cbPlc(cbPlc) {} /// Number of data elements; throws unless cbPlc yields a whole number. - [[nodiscard]] std::uint32_t n() const { + [[nodiscard]] std::size_t n() const { constexpr std::size_t stride = 4 + sizeof(Data); if (m_cbPlc < 4 || (m_cbPlc - 4) % stride != 0) { throw std::runtime_error("doc: malformed Plc size"); } - return static_cast((m_cbPlc - 4) / stride); + return (m_cbPlc - 4) / stride; } - [[nodiscard]] std::uint32_t aCP(const std::uint32_t i) const { - return reinterpret_cast(m_data)[i]; + [[nodiscard]] std::uint32_t aCP(const std::size_t i) const { + if (i > n()) { + throw std::out_of_range("doc: PLC boundary index out of range"); + } + util::byte_string::Reader cursor(std::string_view(m_data, m_cbPlc)); + cursor.skip(i * 4); + return cursor.read(); } - [[nodiscard]] Data aData(const std::uint32_t i) const { - return reinterpret_cast(m_data + (n() + 1) * 4)[i]; + [[nodiscard]] Data aData(const std::size_t i) const { + const std::size_t count = n(); + if (i >= count) { + throw std::out_of_range("doc: PLC data index out of range"); + } + util::byte_string::Reader cursor(std::string_view(m_data, m_cbPlc)); + cursor.skip((count + 1) * 4 + i * sizeof(Data)); + return cursor.read(); } private: diff --git a/src/odr/internal/oldms/text/doc_style.cpp b/src/odr/internal/oldms/text/doc_style.cpp index 673c23921..b8653b6e9 100644 --- a/src/odr/internal/oldms/text/doc_style.cpp +++ b/src/odr/internal/oldms/text/doc_style.cpp @@ -1,7 +1,7 @@ #include -#include #include +#include #include #include @@ -173,35 +173,41 @@ std::vector text::read_font_names(std::istream &table_stream, } table_stream.seekg(sttbf_ffn.fc); + const std::string body = + util::byte_stream::read_u8s(table_stream, sttbf_ffn.lcb); + util::byte_string::Reader cursor(body); + // SttbfFfn is a non-extended STTB ([MS-DOC] 2.2.4, 2.9.286): u16 cData, // u16 cbExtra (0), then per entry a u8 byte count and an FFN. - const auto c_data = util::byte_stream::read(table_stream); + const auto c_data = cursor.read(); if (c_data == 0xFFFF) { throw std::runtime_error("doc: unexpected extended SttbfFfn"); } - const auto cb_extra = util::byte_stream::read(table_stream); + const auto cb_extra = cursor.read(); + // Inside the declared length, a quirk of one font name does not refuse the + // document: an odd byte after the name and any cbExtra bytes are skipped, + // and a name without its NUL takes all of its units. result.reserve(c_data); - for (std::uint16_t i = 0; i < c_data; ++i) { - const auto cch_data = util::byte_stream::read(table_stream); + for (std::size_t i = 0; i < c_data; ++i) { + const auto cch_data = cursor.read(); if (cch_data < sizeof(FfnFixed)) { throw std::runtime_error("doc: FFN too short"); } - const auto ffn = util::byte_stream::read(table_stream); - (void)ffn; - - // xszFfn: null-terminated UTF-16; an alternative name may follow it. - const std::size_t name_units = (cch_data - sizeof(FfnFixed)) / 2; - std::u16string name = read_string_uncompressed(table_stream, name_units); - if ((cch_data - sizeof(FfnFixed)) % 2 != 0) { - table_stream.ignore(1); - } - if (const std::size_t nul = name.find(u'\0'); nul != std::u16string::npos) { - name.resize(nul); + cursor.skip(sizeof(FfnFixed)); + // xszFfn: NUL-terminated UTF-16; an alternative name may follow it. + const std::size_t name_bytes = cch_data - sizeof(FfnFixed); + std::u16string name; + bool terminated = false; + for (std::size_t unit = 0; unit < name_bytes / 2; ++unit) { + const auto character = cursor.read(); + terminated = terminated || character == 0; + if (!terminated) { + name.push_back(character); + } } + cursor.skip(name_bytes % 2 + cb_extra); result.push_back(util::string::u16string_to_string(name)); - - table_stream.ignore(cb_extra); } return result; diff --git a/test/src/internal/oldms/doc_test.cpp b/test/src/internal/oldms/doc_test.cpp index e437df687..0dc3a7b14 100644 --- a/test/src/internal/oldms/doc_test.cpp +++ b/test/src/internal/oldms/doc_test.cpp @@ -4,10 +4,12 @@ #include #include +#include #include #include #include #include +#include #include #include @@ -230,6 +232,16 @@ TEST(OldMs, doc_read_font_names) { ASSERT_EQ(names.size(), 2); EXPECT_EQ(names[0], "Arial"); EXPECT_EQ(names[1], "Times New Roman"); + in.clear(); + EXPECT_THROW(internal::oldms::text::read_font_names(in, {0, 4}), + std::runtime_error); + // a name without its NUL is kept whole rather than refusing the document + std::string unterminated = make_sttbf_ffn({"Arial"}); + unterminated[unterminated.size() - 2] = 'x'; + std::istringstream bad(unterminated); + EXPECT_EQ(internal::oldms::text::read_font_names( + bad, {0, static_cast(unterminated.size())}), + std::vector{"Arialx"}); } // End-to-end over a synthetic .doc: the PlcBteChpx/ChpxFkp run boundaries @@ -328,6 +340,25 @@ TEST(OldMs, doc_character_formatting) { EXPECT_EQ(paragraphs[0].as_paragraph().text_style().font_size, Measure("10pt")); + + // The same physical bytes cannot satisfy a shorter declared Clx or a longer + // body. + for (const bool short_clx : {true, false}) { + std::string invalid = word_document; + const std::string fib = + make_fib(short_clx ? 11 : 12, + {0, short_clx ? 5u : static_cast(clx.size())}, + {128, static_cast(plc_bte.size())}, + {64, static_cast(sttbf_ffn.size())}); + invalid.replace(0, fib.size(), fib); + auto invalid_files = std::make_shared(*files); + ASSERT_TRUE(invalid_files->remove(internal::AbsPath("/WordDocument"))); + ASSERT_TRUE( + invalid_files->copy(std::make_shared(invalid), + internal::AbsPath("/WordDocument"))); + EXPECT_THROW(internal::oldms::text::Document{invalid_files}, + std::runtime_error); + } } TEST(OldMs, doc_fib_versions_respect_counted_arrays) { @@ -373,3 +404,42 @@ TEST(OldMs, doc_fib_versions_respect_counted_arrays) { EXPECT_THROW(parse(invalid), std::runtime_error); EXPECT_THROW(parse(make_fib(0x80000000u, {}, {}, {})), std::runtime_error); } + +TEST(OldMs, doc_plc_reads_unaligned_bytes_and_checks_indices) { + namespace word = internal::oldms::text; + std::string bytes("x"); + append_u32(bytes, 0); + append_u32(bytes, 3); + append_u16(bytes, 0); + append_u32(bytes, (1u << 30) | 200u); + append_u16(bytes, 0); + const word::PlcPcdMap map(bytes.data() + 1, bytes.size() - 1); + EXPECT_EQ(map.n(), 1); + EXPECT_EQ(map.aCP(1), 3); + EXPECT_EQ(map.aData(0).fc.fc, 200); + EXPECT_THROW(static_cast(map.aCP(2)), std::out_of_range); + EXPECT_THROW(static_cast(map.aData(1)), std::out_of_range); + EXPECT_THROW(static_cast(word::PlcPcdMap(bytes.data(), 3).n()), + std::runtime_error); +} + +TEST(OldMs, doc_character_pieces_validate_ranges) { + namespace word = internal::oldms::text; + word::CharacterIndex index; + index.append(0, 3, 100, true); + index.append(3, 2, 200, false); + EXPECT_EQ((*index.find(2)).data_offset, 100); + EXPECT_EQ((*index.find(3)).data_offset, 200); + EXPECT_EQ(index.find(5), index.end()); + const word::CharacterIndex other; + EXPECT_NE(index.begin(), other.begin()); + EXPECT_THROW(index.append(6, 1, 300, true), std::runtime_error); + EXPECT_THROW(index.append(4, 2, 300, true), std::runtime_error); + EXPECT_THROW( + index.append(5, std::numeric_limits::max(), 0, true), + std::runtime_error); + EXPECT_THROW( + index.append(5, 2, std::numeric_limits::max() - 1, false), + std::runtime_error); + EXPECT_EQ(index.size(), 2); +}