diff --git a/CHANGELOG.md b/CHANGELOG.md index 4eabd4a2e..f3c05471b 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 PowerPoint validates nested picture bounds, record lengths, and formatting + counts, limits text-container nesting, and avoids overflow in frame dimensions. + - 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. diff --git a/src/odr/internal/oldms/presentation/AGENTS.md b/src/odr/internal/oldms/presentation/AGENTS.md index fa83b5708..d34fb39de 100644 --- a/src/odr/internal/oldms/presentation/AGENTS.md +++ b/src/odr/internal/oldms/presentation/AGENTS.md @@ -62,7 +62,10 @@ record type (`read_header`); a record that overruns its container; a missing mandatory child (`DrawingContainer`, `OfficeArtDgContainer`, `OfficeArtSpgrContainer`, via `require_child`); an `OfficeArtClientAnchor` whose `recLen` is neither 8 nor 16; a looping or empty `UserEditAtom` chain; -an unresolved `docPersistIdRef`; a slide `persistIdRef` not in the directory. +an unresolved `docPersistIdRef`; a slide `persistIdRef` not in the directory; +truncated skipped bytes, properties or persist-directory entries; an embedded +BLIP outside its store entry; formatting counts that do not cover the text. +Text-container recursion stops at 1024 levels. Pass through: an absent slide list, a shape with no anchor (unpositioned frame), nested groups and non-`Sp` records in a group, any unrecognised child. diff --git a/src/odr/internal/oldms/presentation/ppt_document.cpp b/src/odr/internal/oldms/presentation/ppt_document.cpp index bab6045ce..4a003dcc6 100644 --- a/src/odr/internal/oldms/presentation/ppt_document.cpp +++ b/src/odr/internal/oldms/presentation/ppt_document.cpp @@ -107,13 +107,15 @@ class ElementAdapter final : public AdapterBase { } [[nodiscard]] std::optional frame_width(const ElementIdentifier element_id) const override { - return anchor_measure(element_id, - [](const Anchor &a) { return a.right - a.left; }); + return anchor_measure(element_id, [](const Anchor &a) { + return std::int64_t{a.right} - a.left; + }); } [[nodiscard]] std::optional frame_height(const ElementIdentifier element_id) const override { - return anchor_measure(element_id, - [](const Anchor &a) { return a.bottom - a.top; }); + return anchor_measure(element_id, [](const Anchor &a) { + return std::int64_t{a.bottom} - a.top; + }); } [[nodiscard]] std::optional frame_z_index(const ElementIdentifier /*element_id*/) const override { diff --git a/src/odr/internal/oldms/presentation/ppt_parser.cpp b/src/odr/internal/oldms/presentation/ppt_parser.cpp index f6c6f1e3e..e3728b2ed 100644 --- a/src/odr/internal/oldms/presentation/ppt_parser.cpp +++ b/src/odr/internal/oldms/presentation/ppt_parser.cpp @@ -8,7 +8,6 @@ #include #include #include -#include #include #include @@ -69,7 +68,7 @@ class ChildCursor { void skip_body() { if (m_body > 0) { - m_in->ignore(m_body); + util::byte_stream::skip(*m_in, m_body); m_remaining -= static_cast(m_body); m_body = 0; } @@ -205,7 +204,7 @@ StyledText style_pending(const PendingText &pending, if (at >= count) { break; // the runs also cover the implicit final paragraph mark } - const std::size_t end = std::min(count, at + run.count); + const std::size_t end = at + std::min(count - at, run.count); if (std::string text = pending.decode(at, end); !text.empty()) { result.push_back({std::move(text), resolve_style(run, context)}); } @@ -322,7 +321,7 @@ void read_pending_text(std::istream &in, const RecordHeader &header, std::vector read_style_atom(std::istream &in, const RecordHeader &header, const PendingText &pending) { - const std::string body = util::stream::read(in, header.recLen); + const std::string body = util::byte_stream::read_u8s(in, header.recLen); return parse_style_text_prop_atom(body, pending.char_count() + 1); } @@ -334,7 +333,10 @@ std::vector read_style_atom(std::istream &in, void gather_text(std::istream &in, const RecordHeader &container, StyledText &slide_text, const std::vector &outline_texts, - StyleContext &context) { + StyleContext &context, const std::size_t depth = 0) { + if (depth >= 1024) { + throw std::runtime_error("ppt: text container nesting limit exceeded"); + } PendingText pending; const auto flush = [&](const std::vector &runs) { if (pending.has_value()) { @@ -367,7 +369,7 @@ void gather_text(std::istream &in, const RecordHeader &container, } } else if (child->is_container()) { flush({}); - gather_text(in, *child, slide_text, outline_texts, context); + gather_text(in, *child, slide_text, outline_texts, context, depth + 1); children.consume(child->recLen); } // Other atoms: left unconsumed; the cursor skips their bodies. @@ -400,9 +402,10 @@ Shape read_shape(std::istream &in, const RecordHeader &header, // rh.recInstance is the property count; complex data (skipped by the // cursor) follows the array ([MS-ODRAW] 2.2.9). const std::uint16_t count = child->recInstance; - for (std::uint16_t i = 0; - i < count && (i + 1) * sizeof(OfficeArtFopte) <= child->recLen; - ++i) { + if (count > child->recLen / sizeof(OfficeArtFopte)) { + throw std::runtime_error("ppt: truncated property array"); + } + for (std::size_t i = 0; i < count; ++i) { const auto property = util::byte_stream::read(in); children.consume(sizeof(OfficeArtFopte)); if (property.fComplex != 0) { @@ -479,12 +482,18 @@ void read_persist_directory(std::istream &in, const RecordHeader &header, constexpr std::uint32_t field_size = sizeof(std::uint32_t); std::uint32_t remaining = header.recLen; - while (remaining >= field_size) { + while (remaining > 0) { + if (remaining < field_size) { + throw std::runtime_error("ppt: truncated persist directory entry"); + } const std::uint32_t entry = read_u32(in); // persistId:20 | cPersist:12 remaining -= field_size; const std::uint32_t persist_id = entry & 0x000FFFFF; const std::uint32_t count = entry >> 20; - for (std::uint32_t i = 0; i < count && remaining >= field_size; ++i) { + if (count == 0 || count > remaining / field_size || persist_id > 0xFFFFE) { + throw std::runtime_error("ppt: invalid persist directory entry"); + } + for (std::uint32_t i = 0; i < count; ++i) { const std::uint32_t persist_offset = read_u32(in); remaining -= field_size; directory.emplace(persist_id + i, persist_offset); @@ -569,23 +578,19 @@ std::uint32_t blip_prefix_size(const RecordHeader &header) { return uid_size + (two_uids ? uid_size : 0) + 1; } -/// Reads an OfficeArt BLIP record ([MS-ODRAW] 2.2.23) at the stream position -/// and returns its image file bytes; empty for BLIP types not modelled -/// (WMF/EMF/PICT/DIB/TIFF). -std::string read_blip_record(std::istream &in) { - const RecordHeader header = read_record_header(in); - +/// Reads a BLIP body ([MS-ODRAW] 2.2.23); unsupported image types return empty. +std::string read_blip_body(std::istream &in, const RecordHeader &header) { if (header.recType != RT_OfficeArtBlipJPEG && header.recType != RT_OfficeArtBlipPNG) { - in.ignore(header.recLen); + util::byte_stream::skip(in, header.recLen); return {}; } const std::uint32_t prefix = blip_prefix_size(header); if (header.recLen < prefix) { throw std::runtime_error("ppt: truncated BLIP record"); } - in.ignore(prefix); - return util::stream::read(in, header.recLen - prefix); + util::byte_stream::skip(in, prefix); + return util::byte_stream::read_u8s(in, header.recLen - prefix); } /// One slot of the BLIP store: the offset of the BLIP in the "Pictures" @@ -622,32 +627,31 @@ std::vector read_blip_store(std::istream &in, while (const std::optional child = entries.next()) { BlipSlot &slot = slots.emplace_back(); if (child->recType == RT_OfficeArtFBSE) { - const auto fbse = util::byte_stream::read(in); entries.consume(sizeof(OfficeArtFbseFixed)); - in.ignore(fbse.cbName); + const auto fbse = util::byte_stream::read(in); entries.consume(fbse.cbName); + util::byte_stream::skip(in, fbse.cbName); // An embedded BLIP follows the name ([MS-ODRAW] 2.2.32); otherwise the // BLIP lives in the delay ("Pictures") stream at foDelay. if (child->recLen > sizeof(OfficeArtFbseFixed) + std::uint32_t{fbse.cbName}) { const std::uint32_t remaining = child->recLen - sizeof(OfficeArtFbseFixed) - fbse.cbName; - slot.data = read_blip_record(in); - entries.consume(remaining); + if (remaining < sizeof(RecordHeader)) { + throw std::runtime_error("ppt: truncated embedded BLIP header"); + } + const RecordHeader blip = read_record_header(in); + if (blip.recLen > remaining - sizeof(RecordHeader)) { + throw std::runtime_error("ppt: embedded BLIP exceeds store entry"); + } + slot.data = read_blip_body(in, blip); + entries.consume(sizeof(RecordHeader) + blip.recLen); } else { slot.fo_delay = fbse.foDelay; } } else if (child->recType == RT_OfficeArtBlipJPEG || child->recType == RT_OfficeArtBlipPNG) { - // A BLIP directly in the store occupies a slot of its own; its header is - // already consumed, so the body is read here rather than via - // read_blip_record. - const std::uint32_t prefix = blip_prefix_size(*child); - if (child->recLen < prefix) { - throw std::runtime_error("ppt: truncated BLIP record"); - } - in.ignore(prefix); - slot.data = util::stream::read(in, child->recLen - prefix); + slot.data = read_blip_body(in, *child); entries.consume(child->recLen); } // Other record types leave an empty slot; the cursor skips them. @@ -706,7 +710,8 @@ collect_slides(std::istream ¤t_user, std::istream &document, ElementRegistry ®istry, StyleContext &context) { // Newest user edit offset, from the Current User stream. const CurrentUserAtomHead head = read_current_user_atom_head(current_user); - if (head.rh.recType != RT_CurrentUserAtom) { + if (head.rh.recType != RT_CurrentUserAtom || + head.rh.recLen < sizeof(CurrentUserAtomHead) - sizeof(RecordHeader)) { throw std::runtime_error( "ppt: invalid CurrentUserAtom in \"Current User\" stream"); } @@ -720,7 +725,10 @@ collect_slides(std::istream ¤t_user, std::istream &document, for (bool first = true; edit_offset != 0; first = false) { document.clear(); document.seekg(edit_offset); - read_header(document, RT_UserEditAtom); + const RecordHeader edit_header = read_header(document, RT_UserEditAtom); + if (edit_header.recLen < sizeof(UserEditAtomBody)) { + throw std::runtime_error("ppt: truncated UserEditAtom"); + } const UserEditAtomBody edit = read_user_edit_atom_body(document); if (first) { doc_persist_id = edit.docPersistIdRef; @@ -836,7 +844,8 @@ collect_slides(std::istream ¤t_user, std::istream &document, pictures_stream != nullptr) { pictures_stream->clear(); pictures_stream->seekg(slot.fo_delay); - slot.data = read_blip_record(*pictures_stream); + slot.data = read_blip_body(*pictures_stream, + read_record_header(*pictures_stream)); slot.fo_delay = 0xFFFFFFFF; // resolved (possibly to unsupported/empty) } shape.image = slot.data; diff --git a/src/odr/internal/oldms/presentation/ppt_style.cpp b/src/odr/internal/oldms/presentation/ppt_style.cpp index 2bdf95266..c1e68a647 100644 --- a/src/odr/internal/oldms/presentation/ppt_style.cpp +++ b/src/odr/internal/oldms/presentation/ppt_style.cpp @@ -2,6 +2,8 @@ #include +#include +#include #include namespace odr::internal::oldms::presentation { @@ -197,22 +199,30 @@ presentation::parse_style_text_prop_atom(const std::string_view body, const std::size_t char_count) { BodyCursor cursor(body); - // rgTextPFRun: skipped; the counts cover the whole corresponding text. - for (std::size_t covered = 0; - covered < char_count && cursor.remaining() > 0;) { - covered += cursor.read(); // count - cursor.skip(2); // indentLevel + // [MS-PPT] 2.9.44: both run arrays cover the text. A run past its end is + // cut, as LibreOffice does, so a slightly wrong count does not refuse the + // file; `style_pending` gives characters after the last run the default. + const auto read_count = [&](std::size_t &remaining) { + const auto count = + std::min(cursor.read(), remaining); + remaining -= count; + return static_cast(count); + }; + + for (std::size_t remaining = char_count; + remaining > 0 && cursor.remaining() > 0;) { + read_count(remaining); + cursor.skip(2); // indentLevel skip_text_pf_exception(cursor); } // rgTextCFRun. std::vector runs; - for (std::size_t covered = 0; - covered < char_count && cursor.remaining() > 0;) { + for (std::size_t remaining = char_count; + remaining > 0 && cursor.remaining() > 0;) { TextCFRun run; - run.count = cursor.read(); + run.count = read_count(remaining); read_text_cf_exception(cursor, run); - covered += run.count; runs.push_back(run); } return runs; @@ -250,8 +260,12 @@ std::uint32_t presentation::resolve_style(const TextCFRun &run, } style.font_name = context.fonts[*run.font_ref]; } - context.styles.push_back(style); - return static_cast(context.styles.size() - 1); + if (!std::in_range(context.styles.size())) { + throw std::length_error("ppt: too many character styles"); + } + const auto index = static_cast(context.styles.size()); + context.styles.push_back(std::move(style)); + return index; } } // namespace odr::internal::oldms diff --git a/src/odr/internal/oldms/presentation/ppt_style.hpp b/src/odr/internal/oldms/presentation/ppt_style.hpp index 0d76c34bb..9c87e03b0 100644 --- a/src/odr/internal/oldms/presentation/ppt_style.hpp +++ b/src/odr/internal/oldms/presentation/ppt_style.hpp @@ -23,9 +23,8 @@ struct TextCFRun final { }; /// Parses a StyleTextPropAtom body ([MS-PPT] 2.9.44): the paragraph-level runs -/// are skipped, the character-level runs returned. `char_count` is the -/// corresponding text's length in characters plus one (the runs also cover an -/// implicit final paragraph mark); fewer covered characters end the parse. +/// are skipped, the character runs returned. `char_count` includes the final +/// paragraph mark. std::vector parse_style_text_prop_atom(std::string_view body, std::size_t char_count); diff --git a/src/odr/internal/util/byte_stream_util.cpp b/src/odr/internal/util/byte_stream_util.cpp index 504064494..f60ea711b 100644 --- a/src/odr/internal/util/byte_stream_util.cpp +++ b/src/odr/internal/util/byte_stream_util.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include namespace odr::internal::util { @@ -37,6 +38,18 @@ void byte_stream::read(std::istream &in, char *out, std::size_t count) { } } +void byte_stream::skip(std::istream &in, std::uint64_t count) { + while (count > 0) { + const auto step = static_cast(std::min( + count, std::numeric_limits::max())); + in.ignore(step); + if (in.gcount() != step) { + throw_exhausted(); + } + count -= static_cast(step); + } +} + std::uint8_t byte_stream::read_u8(std::istream &in) { const auto c = in.rdbuf()->sbumpc(); if (c == eof) { diff --git a/src/odr/internal/util/byte_stream_util.hpp b/src/odr/internal/util/byte_stream_util.hpp index a49db9305..4b5840585 100644 --- a/src/odr/internal/util/byte_stream_util.hpp +++ b/src/odr/internal/util/byte_stream_util.hpp @@ -31,6 +31,8 @@ template std::optional try_read(std::istream &in) { } void read(std::istream &in, char *out, std::size_t count); +/// Skips exactly @p count bytes; throws like `read` where the stream ends. +void skip(std::istream &in, std::uint64_t count); template void read(std::istream &in, T &out) { read(in, reinterpret_cast(&out), sizeof(T)); diff --git a/test/src/internal/oldms/ppt_test.cpp b/test/src/internal/oldms/ppt_test.cpp index 9a03f5895..e0c7f5cb1 100644 --- a/test/src/internal/oldms/ppt_test.cpp +++ b/test/src/internal/oldms/ppt_test.cpp @@ -9,6 +9,10 @@ #include #include +#include +#include +#include +#include #include #include #include @@ -26,6 +30,55 @@ using odr::test::oldms::append_u16; using odr::test::oldms::append_u32; using odr::test::oldms::collect_text; +namespace { +namespace ppt = internal::oldms::presentation; + +std::string ppt_record(const std::uint16_t type, const std::string &body, + const std::uint16_t flags = 0) { + std::string record; + append_u16(record, flags); + append_u16(record, type); + append_u32(record, static_cast(body.size())); + return record + body; +} + +std::shared_ptr +ppt_files(const std::string &document_body, const std::string &slide_body = {}, + const std::string &directory_tail = {}) { + std::string document = + ppt_record(ppt::RT_DocumentContainer, document_body, 15); + const auto slide_offset = static_cast(document.size()); + document += ppt_record(ppt::RT_SlideContainer, slide_body, 15); + const auto directory_offset = static_cast(document.size()); + std::string directory; + append_u32(directory, (2u << 20) | 1u); + append_u32(directory, 0); + append_u32(directory, slide_offset); + document += + ppt_record(ppt::RT_PersistDirectoryAtom, directory + directory_tail); + const auto edit_offset = static_cast(document.size()); + std::string edit(12, '\0'); + append_u32(edit, directory_offset); + append_u32(edit, 1); + append_u32(edit, 3); + edit.resize(28, '\0'); + document += ppt_record(ppt::RT_UserEditAtom, edit); + std::string user; + append_u32(user, 20); + append_u32(user, ppt::current_user_token_plain); + append_u32(user, edit_offset); + user.resize(20, '\0'); + auto files = std::make_shared(); + files->copy(std::make_shared(document), + internal::AbsPath("/PowerPoint Document")); + files->copy(std::make_shared( + ppt_record(ppt::RT_CurrentUserAtom, user)), + internal::AbsPath("/Current User")); + return files; +} + +} // namespace + // A StyleTextPropAtom body ([MS-PPT] 2.9.44): the paragraph-level runs are // skipped (including their mask-dependent fields), the character-level runs // map masks/CFStyle/fontRef/size/color onto TextCFRun. @@ -209,3 +262,83 @@ TEST(OldMs, ppt_text_bytes_require_complete_record) { throwing.exceptions(std::ios::failbit | std::ios::badbit); EXPECT_THROW(read_raw_text_bytes(throwing, 3), std::ios_base::failure); } + +TEST(OldMs, ppt_rejects_invalid_record_boundaries) { + EXPECT_NO_THROW(ppt::Document{ppt_files({})}); + EXPECT_THROW(ppt::Document{ppt_files({}, {}, "x")}, std::runtime_error); + std::string missing_offsets; + append_u32(missing_offsets, (2u << 20) | 3u); + EXPECT_THROW(ppt::Document{ppt_files({}, {}, missing_offsets)}, + std::runtime_error); + + std::string blip = + ppt_record(ppt::RT_OfficeArtBlipPNG, std::string(17, '\0')); + blip.pop_back(); + const std::string fbse = + ppt_record(ppt::RT_OfficeArtFBSE, std::string(36, '\0') + blip); + const std::string store = + ppt_record(ppt::RT_OfficeArtBStoreContainer, fbse, 15); + const std::string drawing = + ppt_record(ppt::RT_DrawingGroup, + ppt_record(ppt::RT_OfficeArtDggContainer, store, 15), 15); + EXPECT_THROW(ppt::Document{ppt_files(drawing)}, std::runtime_error); +} + +TEST(OldMs, ppt_text_nesting_is_bounded) { + const auto files = [](const std::size_t depth) { + std::string text = ppt_record(ppt::RT_TextBytesAtom, "visible"); + for (std::size_t i = 0; i < depth; ++i) { + text = ppt_record(0x7777, text, 15); + } + const std::string shape = + ppt_record(ppt::RT_OfficeArtSpContainer, + ppt_record(ppt::RT_OfficeArtClientTextbox, text), 15); + const std::string slide = ppt_record( + ppt::RT_Drawing, + ppt_record(ppt::RT_OfficeArtDgContainer, + ppt_record(ppt::RT_OfficeArtSpgrContainer, shape, 15), 15), + 15); + std::string persist; + append_u32(persist, 2); + return ppt_files(ppt_record(ppt::RT_SlideListWithText, + ppt_record(ppt::RT_SlidePersistAtom, persist), + 15), + slide); + }; + const Document shallow(std::make_shared(files(4))); + EXPECT_EQ(collect_text(shallow.root_element()), "visible"); + EXPECT_THROW(ppt::Document{files(1024)}, std::runtime_error); +} + +TEST(OldMs, ppt_style_runs_are_bounded_by_the_text) { + std::string body; + append_u32(body, 3); + append_u16(body, 0); + append_u32(body, 0); + append_u32(body, 3); + append_u32(body, 0); + EXPECT_EQ(ppt::parse_style_text_prop_atom(body, 3).size(), 1); + // a run past the end of the text is cut, not refused + const auto cut = ppt::parse_style_text_prop_atom(body, 2); + ASSERT_EQ(cut.size(), 1); + EXPECT_EQ(cut[0].count, 2); + // too short a run array reads into the next one and runs out of bytes + EXPECT_THROW(ppt::parse_style_text_prop_atom(body, 4), std::runtime_error); + body.pop_back(); + EXPECT_THROW(ppt::parse_style_text_prop_atom(body, 3), std::runtime_error); +} + +TEST(OldMs, ppt_anchor_differences_do_not_overflow) { + ppt::Document document(ppt_files({})); + auto [id, element, frame] = + document.element_registry().create_frame_element(); + frame.anchor = ppt::Anchor{std::numeric_limits::min(), + std::numeric_limits::min(), + std::numeric_limits::max(), + std::numeric_limits::max()}; + const Frame handle = Element(document.element_adapter(), id).as_frame(); + const Measure extent(4294967295.0 / ppt::master_units_per_inch, + DynamicUnit("in")); + EXPECT_EQ(handle.width(), extent); + EXPECT_EQ(handle.height(), extent); +} diff --git a/test/src/internal/util/stream_util_test.cpp b/test/src/internal/util/stream_util_test.cpp index 699fbeab7..1f9920ec6 100644 --- a/test/src/internal/util/stream_util_test.cpp +++ b/test/src/internal/util/stream_util_test.cpp @@ -108,6 +108,14 @@ TEST(Stream, copying_reports_input_and_output_failures) { EXPECT_THROW(stream::read(broken), std::ios_base::failure); } +TEST(ByteStream, skip_moves_exactly_and_refuses_a_short_stream) { + std::istringstream in("abcdef"); + byte_stream::skip(in, 0); + byte_stream::skip(in, 2); + EXPECT_EQ(byte_stream::read_u8(in), 'c'); + EXPECT_THROW(byte_stream::skip(in, 4), std::runtime_error); +} + TEST(ByteStream, length_prefixed_reads_handle_chunks_and_stream_exceptions) { const std::string data(4097, 'x'); std::istringstream in(data);