diff --git a/CHANGELOG.md b/CHANGELOG.md index f3c05471b..607141f0c 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 validates FIB signatures and extension lengths. It stores the + shared header fields by value, removing unsafe deletion through base pointers. + - Legacy PowerPoint validates nested picture bounds, record lengths, and formatting counts, limits text-container nesting, and avoids overflow in frame dimensions. diff --git a/src/odr/internal/oldms/text/AGENTS.md b/src/odr/internal/oldms/text/AGENTS.md index 940a60399..483b73064 100644 --- a/src/odr/internal/oldms/text/AGENTS.md +++ b/src/odr/internal/oldms/text/AGENTS.md @@ -17,7 +17,7 @@ text. | File (`oldms/text/`) | Role | |---|---| -| `doc_structs.hpp` | `#pragma pack(1)` PODs (`FibBase`, the `FibRgFcLcb97` to `FibRgFcLcb2007` chain, `Sprm`, `FcCompressed`, `Pcd`, `PnFkpChpx`, `FfnFixed`), `PlcMap` (`PlcPcdMap`, `PlcBteChpxMap`), `ParsedFib`, the character SPRM opcodes | +| `doc_structs.hpp` | `#pragma pack(1)` PODs (`FibBase`, `FibRgFcLcb97`, `Sprm`, `FcCompressed`, `Pcd`, `PnFkpChpx`, `FfnFixed`), `PlcMap` (`PlcPcdMap`, `PlcBteChpxMap`), `ParsedFib`, the character SPRM opcodes | | `doc_io.{hpp,cpp}` | `read(...)` over `std::istream`: variable-length FIB, Clx walk, string decoding, `uncompress_char` | | `doc_helper.{hpp,cpp}` | `CharacterIndex` (decoded piece table) and `read_character_index`; `CharacterRuns` (fc-keyed style-index runs) and `read_character_runs` | | `doc_style.{hpp,cpp}` | `StyleRegistry` (resolved `TextStyle`s by index, font-name store), `apply_character_sprms`, `read_font_names` (SttbfFfn) | @@ -35,14 +35,12 @@ body with headers, footnotes and annotations, and the FIB's `ccp*` counts partition the CP space. The parser takes the first `ccpText` CPs by clamping each piece to the remaining budget. -**Self-describing FIB read.** `read(ParsedFib&)` trusts the on-disk counts, -reads what the module models and ignores the surplus. Version dispatch picks -the `FibRgFcLcb*` layout by `nFib` (`nFib97` 0x00C1, `nFib2000` 0x00D9, -`nFib2002` 0x0101, `nFib2003` 0x010C, `nFib2007` 0x0112). A newer `nFib` uses -the 2007 layout. The `FcLcb` block is copied clamped to -`min(sizeof(layout), cbRgFcLcb·8)`, so extra entries are ignored and a shorter -block leaves the rest zero. `clx` sits in the `FibRgFcLcb97` base, so it is -always covered. +**Self-describing FIB read.** Counts bound each FIB array. The parser keeps the +`FibRgFcLcb97` prefix by value; later versions append entries that it does not +use. A shorter block zero-fills missing entries. `cswNew` bounds the version +extension, whose unused fields are skipped. Known versions are validated; +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 @@ -101,8 +99,8 @@ are dropped. - `html_output_test` compares the real `.doc` fixtures against the reference output. There is no assertion-based test over a real fixture. -Not tested: FIB robustness (negative `ccpText`, newer-than-2007 fallback) and -`page_break` emission. +FIB versions, counted extensions, and negative `ccpText` have synthetic coverage. +`page_break` emission still lacks a focused test. ## Binary format reference (FIB) diff --git a/src/odr/internal/oldms/text/doc_io.cpp b/src/odr/internal/oldms/text/doc_io.cpp index 4c3b28028..a87fa7b49 100644 --- a/src/odr/internal/oldms/text/doc_io.cpp +++ b/src/odr/internal/oldms/text/doc_io.cpp @@ -1,8 +1,8 @@ #include -#include "odr/internal/util/string_util.hpp" - #include +#include +#include #include #include @@ -12,34 +12,20 @@ namespace odr::internal::oldms::text { namespace { -template struct TypeTag { - using type = T; -}; - -template -auto type_dispatch_FibRgFcLcb(const std::uint16_t nFib, const F &f) { - switch (nFib) { - case nFib97: { - return f(TypeTag{}); - } - case nFib2000: { - return f(TypeTag{}); - } - case nFib2002: { - return f(TypeTag{}); - } - case nFib2003: { - return f(TypeTag{}); - } - case nFib2007: { - return f(TypeTag{}); - } +void validate_version(const std::uint16_t version, const bool allow_future) { + switch (version) { + case nFib97: + case nFib2000: + case nFib2002: + case nFib2003: + case nFib2007: + return; default: - // Newer FIBs only append entries, so the newest modelled layout fits. - if (nFib > nFib2007) { - return f(TypeTag{}); + if (allow_future && version > nFib2007) { + return; } - throw std::runtime_error("Unknown nFib value: " + std::to_string(nFib)); + throw std::runtime_error("doc: unsupported FIB version " + + std::to_string(version)); } } @@ -55,6 +41,9 @@ void text::read(std::istream &in, FibBase &out) { void text::read(std::istream &in, ParsedFib &out) { read(in, out.base); + if (out.base.wIdent != fib_wIdent) { + throw std::runtime_error("doc: invalid FIB signature"); + } util::byte_stream::read(in, out.csw); if (static_cast(out.csw) * 2 < sizeof(out.fibRgW)) { @@ -62,8 +51,7 @@ void text::read(std::istream &in, ParsedFib &out) { std::to_string(out.csw)); } util::byte_stream::read(in, out.fibRgW); - in.ignore(static_cast(static_cast(out.csw) * 2 - - sizeof(out.fibRgW))); + util::byte_stream::skip(in, std::uint64_t{out.csw} * 2 - sizeof(out.fibRgW)); util::byte_stream::read(in, out.cslw); if (static_cast(out.cslw) * 4 < sizeof(out.fibRgLw)) { @@ -71,8 +59,8 @@ void text::read(std::istream &in, ParsedFib &out) { std::to_string(out.cslw)); } util::byte_stream::read(in, out.fibRgLw); - in.ignore(static_cast( - static_cast(out.cslw) * 4 - sizeof(out.fibRgLw))); + util::byte_stream::skip(in, + std::uint64_t{out.cslw} * 4 - sizeof(out.fibRgLw)); // ccpText MUST be >= 0 ([MS-DOC] 2.5.5). if (out.ccpText() < 0) { @@ -81,53 +69,26 @@ void text::read(std::istream &in, ParsedFib &out) { } util::byte_stream::read(in, out.cbRgFcLcb); - const auto fibRgFcLcb = - std::make_unique(static_cast(out.cbRgFcLcb) * 8); - in.read(fibRgFcLcb.get(), static_cast(out.cbRgFcLcb) * 8); + const std::string offsets = util::byte_stream::read_u8s( + in, std::uint32_t{out.cbRgFcLcb} * sizeof(FcLcb)); + out.fibRgFcLcb = {}; + std::memcpy(&out.fibRgFcLcb, offsets.data(), + std::min(sizeof(out.fibRgFcLcb), offsets.size())); util::byte_stream::read(in, out.cswNew); + out.nFibNew.reset(); if (out.cswNew > 0) { - out.fibRgCswNew.emplace(); - read(in, *out.fibRgCswNew); - } - const std::uint16_t nFib = - out.fibRgCswNew.has_value() ? out.fibRgCswNew->nFibNew : out.base.nFib; - - out.fibRgFcLcb = type_dispatch_FibRgFcLcb( - nFib, [&](const T) -> std::unique_ptr { - using FibRgFcLcbType = T::type; - auto result = std::make_unique(); - // Clamped: surplus entries are dropped, a short block stays zeroed. - const std::size_t copy = - std::min(sizeof(FibRgFcLcbType), - static_cast(out.cbRgFcLcb) * 8); - std::memcpy(result.get(), fibRgFcLcb.get(), copy); - return result; - }); -} - -void text::read(std::istream &in, ParsedFibRgCswNew &out) { - util::byte_stream::read(in, out.nFibNew); - - switch (out.nFibNew) { - case nFib97: - break; - case nFib2000: - case nFib2002: - case nFib2003: { - auto rgCswNewData = std::make_unique(); - util::byte_stream::read(in, *rgCswNewData); - out.rgCswNewData = std::move(rgCswNewData); - } break; - case nFib2007: { - auto rgCswNewData = std::make_unique(); - util::byte_stream::read(in, *rgCswNewData); - out.rgCswNewData = std::move(rgCswNewData); - } break; - default: - throw std::runtime_error("Unsupported nFibNew value: " + - std::to_string(out.nFibNew)); - } + const std::string extension = util::byte_stream::read_u8s( + in, std::uint32_t{out.cswNew} * sizeof(std::uint16_t)); + util::byte_string::Reader cursor(extension); + out.nFibNew = cursor.read(); + validate_version(*out.nFibNew, false); + // [MS-DOC] 2.5.11–13: only the version affects the fields we use. + cursor.skip(*out.nFibNew == nFib97 ? 0 + : (*out.nFibNew == nFib2007 ? 8 : 2)); + } + validate_version(out.nFibNew.value_or(out.base.nFib), + !out.nFibNew.has_value()); } void text::read_Clx(std::istream &in, const HandlePrc &handle_Prc, @@ -151,7 +112,7 @@ void text::skip_Prc(std::istream &in) { } const auto cbGrpprl = util::byte_stream::read(in); - in.ignore(cbGrpprl); + util::byte_stream::skip(in, cbGrpprl); } std::string text::read_string(std::istream &in, const std::size_t length_cp, diff --git a/src/odr/internal/oldms/text/doc_io.hpp b/src/odr/internal/oldms/text/doc_io.hpp index 8259d02b8..9b09068ec 100644 --- a/src/odr/internal/oldms/text/doc_io.hpp +++ b/src/odr/internal/oldms/text/doc_io.hpp @@ -11,7 +11,6 @@ namespace odr::internal::oldms::text { void read(std::istream &in, FibBase &out); -void read(std::istream &in, ParsedFibRgCswNew &out); void read(std::istream &in, ParsedFib &out); using HandlePrc = std::function; diff --git a/src/odr/internal/oldms/text/doc_parser.cpp b/src/odr/internal/oldms/text/doc_parser.cpp index 83236c063..5a669852b 100644 --- a/src/odr/internal/oldms/text/doc_parser.cpp +++ b/src/odr/internal/oldms/text/doc_parser.cpp @@ -171,16 +171,16 @@ ElementIdentifier text::parse_tree(ElementRegistry ®istry, const auto table_stream = table_file->stream(); const std::vector font_names = - read_font_names(*table_stream, fib.fibRgFcLcb->sttbfFfn); + read_font_names(*table_stream, fib.fibRgFcLcb.sttbfFfn); // Direct character formatting only ([MS-DOC] 2.4.6.2). std::vector styles{default_character_style()}; // index 0 const CharacterRuns character_runs = read_character_runs(*document_stream, *table_stream, - fib.fibRgFcLcb->plcfBteChpx, styles, font_names); + fib.fibRgFcLcb.plcfBteChpx, styles, font_names); style_registry = StyleRegistry(std::move(styles)); - table_stream->seekg(fib.fibRgFcLcb->clx.fc); + table_stream->seekg(fib.fibRgFcLcb.clx.fc); const CharacterIndex character_index = read_character_index(*table_stream); const auto ccp_text = static_cast(fib.ccpText()); diff --git a/src/odr/internal/oldms/text/doc_structs.hpp b/src/odr/internal/oldms/text/doc_structs.hpp index 4fc086c76..a7326f29b 100644 --- a/src/odr/internal/oldms/text/doc_structs.hpp +++ b/src/odr/internal/oldms/text/doc_structs.hpp @@ -2,7 +2,6 @@ #include #include -#include #include #include #include @@ -64,7 +63,7 @@ struct FibBase { std::uint32_t reserved5; std::uint32_t reserved6; }; -static_assert(sizeof(FibBase) == 32, "FibBase should be 8 bytes"); +static_assert(sizeof(FibBase) == 32, "FibBase should be 32 bytes"); struct FibRgFcLcb97 { FcLcb stshfOrig; @@ -165,130 +164,6 @@ struct FibRgFcLcb97 { static_assert(sizeof(FibRgFcLcb97) == 744, "FibRgFcLcb97 should be 744 bytes in size"); -struct FibRgFcLcb2000 : FibRgFcLcb97 { - FcLcb plcfTch; - FcLcb rmdThreading; - FcLcb mid; - FcLcb sttbRgtplc; - FcLcb msoEnvelope; - FcLcb plcfLad; - FcLcb rgDofr; - FcLcb plcosl; - FcLcb plcfCookieOld; - FcLcb pgdMotherOld; - FcLcb bkdMotherOld; - FcLcb pgdFtnOld; - FcLcb bkdFtnOld; - FcLcb pgdEdnOld; - FcLcb bkdEdnOld; -}; -static_assert(sizeof(FibRgFcLcb2000) == 864, - "FibRgFcLcb2003 should be 864 bytes in size"); - -struct FibRgFcLcb2002 : FibRgFcLcb2000 { - FcLcb unused1; - FcLcb plcfPgp; - FcLcb plcfuim; - FcLcb plfguidUim; - FcLcb atrdExtra; - FcLcb plrsid; - FcLcb sttbfBkmkFactoid; - FcLcb plcfBkfFactoid; - FcLcb plcfcookie; - FcLcb plcfBklFactoid; - FcLcb factoidData; - FcLcb docUndo; - FcLcb sttbfBkmkFcc; - FcLcb plcfBkfFcc; - FcLcb plcfBklFcc; - FcLcb sttbfbkmkBPRepairs; - FcLcb plcfbkfBPRepairs; - FcLcb plcfbklBPRepairs; - FcLcb pmsNew; - FcLcb odso; - FcLcb plcfpmiOldXP; - FcLcb plcfpmiNewXP; - FcLcb plcfpmiMixedXP; - FcLcb unused2; - FcLcb plcffactoid; - FcLcb plcflvcOldXP; - FcLcb plcflvcNewXP; - FcLcb plcflvcMixedXP; -}; -static_assert(sizeof(FibRgFcLcb2002) == 1088, - "FibRgFcLcb2003 should be 1088 bytes in size"); - -struct FibRgFcLcb2003 : FibRgFcLcb2002 { - FcLcb hplxsdr; - FcLcb sttbfBkmkSdt; - FcLcb plcfBkfSdt; - FcLcb plcfBklSdt; - FcLcb customXForm; - FcLcb sttbfBkmkProt; - FcLcb plcfBkfProt; - FcLcb plcfBklProt; - FcLcb sttbProtUser; - FcLcb unused; - FcLcb plcfpmiOld; - FcLcb plcfpmiOldInline; - FcLcb plcfpmiNew; - FcLcb plcfpmiNewInline; - FcLcb plcflvcOld; - FcLcb plcflvcOldInline; - FcLcb plcflvcNew; - FcLcb plcflvcNewInline; - FcLcb pgdMother; - FcLcb bkdMother; - FcLcb afdMother; - FcLcb pgdFtn; - FcLcb bkdFtn; - FcLcb afdFtn; - FcLcb pgdEdn; - FcLcb bkdEdn; - FcLcb afdEdn; - FcLcb afd; -}; -static_assert(sizeof(FibRgFcLcb2003) == 1312, - "FibRgFcLcb2003 should be 1312 bytes in size"); - -struct FibRgFcLcb2007 : FibRgFcLcb2003 { - FcLcb plcfmthd; - FcLcb sttbfBkmkMoveFrom; - FcLcb plcfBkfMoveFrom; - FcLcb plcfBklMoveFrom; - FcLcb sttbfBkmkMoveTo; - FcLcb plcfBkfMoveTo; - FcLcb plcfBklMoveTo; - FcLcb unused1; - FcLcb unused2; - FcLcb unused3; - FcLcb sttbfBkmkArto; - FcLcb plcfBkfArto; - FcLcb plcfBklArto; - FcLcb artoData; - FcLcb unused4; - FcLcb unused5; - FcLcb unused6; - FcLcb ossTheme; - FcLcb colorSchemeMapping; -}; -static_assert(sizeof(FibRgFcLcb2007) == 1464, - "FibRgFcLcb2007 should be 1464 bytes in size"); - -struct FibRgCswNewData2000 { - std::uint16_t cQuickSavesNew; -}; -static_assert(sizeof(FibRgCswNewData2000) == 2, - "FibRgCswNewData2000 should be 2 bytes"); - -struct FibRgCswNewData2007 : FibRgCswNewData2000 { - std::uint16_t lidThemeOther; - std::uint16_t lidThemeFE; - std::uint16_t lidThemeCS; -}; -static_assert(sizeof(FibRgCswNewData2007) == 8, - "FibRgCswNewData2007 should be 8 bytes"); - /// The character SPRMs mapped to TextStyle ([MS-DOC] 2.6.1); everything else /// in a Chpx is skipped via Sprm::operand_size. enum CharacterSprms : std::uint16_t { @@ -370,11 +245,6 @@ static_assert(sizeof(FfnFixed) == 39, "FfnFixed should be 39 bytes"); #pragma pack(pop) -struct ParsedFibRgCswNew { - std::uint16_t nFibNew; - std::unique_ptr rgCswNewData; -}; - struct ParsedFib { FibBase base; std::uint16_t csw; @@ -382,9 +252,9 @@ struct ParsedFib { std::uint16_t cslw; std::array fibRgLw; std::uint16_t cbRgFcLcb; - std::unique_ptr fibRgFcLcb; + FibRgFcLcb97 fibRgFcLcb{}; std::uint16_t cswNew; - std::optional fibRgCswNew; + std::optional nFibNew; /// FibRgLw97.ccpText ([MS-DOC] 2.5.5), the 4th 32-bit field of fibRgLw. [[nodiscard]] std::int32_t ccpText() const { diff --git a/test/src/internal/oldms/doc_test.cpp b/test/src/internal/oldms/doc_test.cpp index 8c8c0c321..e437df687 100644 --- a/test/src/internal/oldms/doc_test.cpp +++ b/test/src/internal/oldms/doc_test.cpp @@ -329,3 +329,47 @@ TEST(OldMs, doc_character_formatting) { EXPECT_EQ(paragraphs[0].as_paragraph().text_style().font_size, Measure("10pt")); } + +TEST(OldMs, doc_fib_versions_respect_counted_arrays) { + namespace word = internal::oldms::text; + const auto parse = [](const std::string &bytes) { + std::istringstream in(bytes + "NEXT"); + word::ParsedFib fib{}; + word::read(in, fib); + EXPECT_EQ(in.peek(), 'N'); + EXPECT_EQ(fib.fibRgFcLcb.clx.fc, 123); + EXPECT_EQ(fib.fibRgFcLcb.clx.lcb, 456); + return fib; + }; + const std::string base = make_fib(7, {123, 456}, {}, {}); + EXPECT_EQ(parse(base).ccpText(), 7); + std::string extended = base; + extended[152] = + 95; // cbRgFcLcb: two unused FcLcb entries after the common prefix. + extended.insert(extended.size() - 2, 16, '\0'); + EXPECT_EQ(parse(extended).ccpText(), 7); + for (const std::uint16_t version : + {word::nFib2000, word::nFib2002, word::nFib2003, word::nFib2007}) { + std::string bytes = base.substr(0, base.size() - 2); + const std::uint16_t words = version == word::nFib2007 ? 5 : 2; + append_u16(bytes, words); + append_u16(bytes, version); + bytes.append((words - 1) * 2, '\0'); + EXPECT_EQ(parse(bytes).nFibNew, version); + } + std::string future = base; + future[2] = '\x13'; + future[3] = '\x01'; + EXPECT_FALSE(parse(future).nFibNew.has_value()); + + std::string undersized = base.substr(0, base.size() - 2); + append_u16(undersized, 1); + append_u16(undersized, word::nFib2007); + undersized.append(8, + '\0'); // Bytes after the declared array cannot satisfy it. + EXPECT_THROW(parse(undersized), std::runtime_error); + std::string invalid = base; + invalid[0] = 0; + EXPECT_THROW(parse(invalid), std::runtime_error); + EXPECT_THROW(parse(make_fib(0x80000000u, {}, {}, {})), std::runtime_error); +}