From 72d5984facc6da24719b219cf81ee58fef8deec0 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 5 Oct 2026 00:20:26 +0200 Subject: [PATCH 1/3] fix(font): validate SFNT character map ranges --- CHANGELOG.md | 4 ++ src/odr/internal/font/sfnt_font.cpp | 97 ++++++++++++++++++++++------ test/src/internal/font/sfnt_font.cpp | 71 ++++++++++++++++++++ 3 files changed, 153 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a97581dd..3c7b09fed 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 +- SFNT fonts validate table and character-map ranges before expanding them, + preventing overflow and unbounded loops. Unsupported character-map formats no + longer hide supported alternatives; truncated names and zero em sizes refuse. + - Legacy Office reports unencrypted documents as decodable and derives metadata from the shared file-type table. diff --git a/src/odr/internal/font/sfnt_font.cpp b/src/odr/internal/font/sfnt_font.cpp index 84ab1618f..f4f5afb7a 100644 --- a/src/odr/internal/font/sfnt_font.cpp +++ b/src/odr/internal/font/sfnt_font.cpp @@ -19,6 +19,15 @@ namespace bs = util::byte_string; namespace { +std::string_view checked_slice(const std::string_view bytes, + const std::size_t offset, + const std::size_t length) { + if (offset > bytes.size() || length > bytes.size() - offset) { + throw std::runtime_error("sfnt: table range exceeds its container"); + } + return bytes.substr(offset, length); +} + // The enumerators below are the on-disk codes (OpenType spec). /// `cmap`/`name` platform IDs. @@ -106,6 +115,9 @@ struct NameEntry { /// included, into UTF-8. @p d is exactly the string's bytes. [[nodiscard]] std::string read_utf16be(const std::string_view d) { const std::size_t len = d.size(); + if (len % 2 != 0) { + throw std::runtime_error("sfnt: odd UTF-16 name length"); + } std::string out; for (std::size_t i = 0; i + 1 < len; i += 2) { char32_t cp = bs::read_u16_be(d.substr(i)); @@ -133,8 +145,8 @@ struct NameEntry { /// Preference among `cmap` subtables: Unicode full > Unicode BMP > symbol > /// Mac. -[[nodiscard]] int cmap_score(const PlatformId platform, - const std::uint16_t encoding) { +[[nodiscard]] std::uint32_t cmap_score(const PlatformId platform, + const std::uint16_t encoding) { switch (platform) { case PlatformId::windows: switch (static_cast(encoding)) { @@ -197,6 +209,10 @@ void SfntFont::parse() { std::uint32_t sfnt_offset = 0; if (d.substr(0, 4) == "ttcf") { + const std::uint32_t count = bs::read_u32_be(d.substr(8)); + if (count == 0 || count > (d.size() - 12) / 4) { + throw std::runtime_error("sfnt: invalid collection directory"); + } // TrueType Collection: read the first member's offset table. sfnt_offset = bs::read_u32_be(d.substr(12)); } @@ -210,13 +226,19 @@ void SfntFont::parse() { } std::string_view SfntFont::table_data(const Table table) const { - return std::string_view{m_data}.substr(table.offset, table.length); + return checked_slice(m_data, table.offset, table.length); } void SfntFont::read_directory(const std::string_view sfnt) { + if (sfnt.size() < 12 || !is_sfnt(sfnt) || sfnt.substr(0, 4) == "ttcf") { + throw std::runtime_error("sfnt: invalid offset table"); + } m_format = sfnt.substr(0, 4) == "OTTO" ? FontFormat::opentype_cff : FontFormat::truetype; const std::uint16_t num_tables = bs::read_u16_be(sfnt.substr(4)); + if (num_tables > (sfnt.size() - 12) / 16) { + throw std::runtime_error("sfnt: truncated table directory"); + } // Past the 12 byte offset table, each directory entry is 16 bytes: tag(4), // checkSum(4), offset(4), length(4). @@ -225,7 +247,9 @@ void SfntFont::read_directory(const std::string_view sfnt) { std::string tag{sfnt.substr(entry, 4)}; const std::uint32_t offset = bs::read_u32_be(sfnt.substr(entry + 8)); const std::uint32_t length = bs::read_u32_be(sfnt.substr(entry + 12)); - m_tables.emplace(std::move(tag), Table{offset, length}); + if (!m_tables.emplace(std::move(tag), Table{offset, length}).second) { + throw std::runtime_error("sfnt: duplicate table tag"); + } } } @@ -236,6 +260,9 @@ void SfntFont::read_head() { } const std::string_view t = table_data(*head); m_units_per_em = bs::read_u16_be(t.substr(18)); + if (m_units_per_em == 0) { + throw std::runtime_error("sfnt: zero units per em"); + } m_bbox = read_bbox(t.substr(36)); } @@ -291,13 +318,20 @@ void SfntFont::read_cmap() { WindowsEncoding::symbol; }); - const auto best_entry = - std::ranges::max_element(entries, {}, [](const CmapEntry &entry) { - return cmap_score(entry.platform, entry.encoding); - }); + const auto score = [t](const CmapEntry &entry) -> std::uint32_t { + switch (static_cast(bs::read_u16_be(t.substr(entry.offset)))) { + case CmapFormat::byte_encoding: + case CmapFormat::segment_mapping: + case CmapFormat::trimmed_mapping: + case CmapFormat::segmented_coverage: + return cmap_score(entry.platform, entry.encoding); + default: + return 0; + } + }; + const auto best_entry = std::ranges::max_element(entries, {}, score); - if (best_entry != entries.end() && - cmap_score(best_entry->platform, best_entry->encoding) > 0) { + if (best_entry != entries.end() && score(*best_entry) > 0) { // The encoding record's offset is relative to the start of the `cmap` // table. read_cmap_subtable(t.substr(best_entry->offset)); @@ -315,7 +349,12 @@ void SfntFont::update_reverse() { } } -void SfntFont::read_cmap_subtable(const std::string_view s) { +void SfntFont::read_cmap_subtable(std::string_view s) { + const auto format = static_cast(bs::read_u16_be(s)); + const std::size_t length = format == CmapFormat::segmented_coverage + ? bs::read_u32_be(s.substr(4)) + : bs::read_u16_be(s.substr(2)); + s = checked_slice(s, 0, length); const auto map = [this](const char32_t code, const std::uint16_t glyph) { if (glyph == 0) { return; @@ -333,7 +372,7 @@ void SfntFont::read_cmap_subtable(const std::string_view s) { return out; }; - switch (static_cast(bs::read_u16_be(s))) { + switch (format) { case CmapFormat::byte_encoding: { // format(0), length(2), language(4), then a 256-byte glyphIdArray(6). for (std::uint32_t code = 0; code < 256; ++code) { @@ -345,8 +384,11 @@ void SfntFont::read_cmap_subtable(const std::string_view s) { // format(0), length(2), language(4), segCountX2(6), 3 search hints(8..12), // then the parallel segs-sized arrays endCode(14), reservedPad, startCode, // idDelta, idRangeOffset. - const std::uint16_t length = bs::read_u16_be(s.substr(2)); - const std::size_t segs = bs::read_u16_be(s.substr(6)) / 2U; + const std::uint16_t seg_count_x2 = bs::read_u16_be(s.substr(6)); + const std::size_t segs = seg_count_x2 / 2U; + if (segs == 0 || seg_count_x2 % 2 != 0) { + throw std::runtime_error("sfnt: invalid cmap segment count"); + } const std::vector end_codes = read_u16_vector(s.substr(14), segs); const std::vector start_codes = @@ -368,6 +410,10 @@ void SfntFont::read_cmap_subtable(const std::string_view s) { const std::uint16_t end = end_codes.at(seg); const std::uint16_t delta = id_deltas.at(seg); const std::uint16_t range = id_range_offsets.at(seg); + if (start > end || (seg > 0 && start <= end_codes[seg - 1]) || + (start != 0xFFFF && range % 2 != 0)) { + throw std::runtime_error("sfnt: invalid cmap segment"); + } for (std::uint32_t code = start; code <= end && code != 0xffff; ++code) { std::uint16_t glyph = 0; if (range == 0) { @@ -392,6 +438,9 @@ void SfntFont::read_cmap_subtable(const std::string_view s) { // glyphIdArray(10). const std::uint16_t first = bs::read_u16_be(s.substr(6)); const std::uint16_t entries = bs::read_u16_be(s.substr(8)); + if (std::uint32_t{first} + entries > 0x10000) { + throw std::runtime_error("sfnt: cmap format 6 exceeds the BMP"); + } for (std::uint16_t i = 0; i < entries; ++i) { map(first + i, bs::read_u16_be(s.substr(10 + static_cast(i) * 2))); @@ -402,11 +451,20 @@ void SfntFont::read_cmap_subtable(const std::string_view s) { // format(0), reserved(2), length(4), language(8), numGroups(12), then the // groups(16), each a 12-byte startCharCode/endCharCode/startGlyphID. const std::uint32_t groups = bs::read_u32_be(s.substr(12)); + if (groups > (s.size() - 16) / 12) { + throw std::runtime_error("sfnt: truncated cmap groups"); + } + std::uint32_t previous_end = 0; for (std::uint32_t g = 0; g < groups; ++g) { const std::size_t base = 16 + static_cast(g) * 12; const std::uint32_t start = bs::read_u32_be(s.substr(base)); const std::uint32_t end = bs::read_u32_be(s.substr(base + 4)); const std::uint32_t start_glyph = bs::read_u32_be(s.substr(base + 8)); + if (start > end || end > 0x10FFFF || (g > 0 && start <= previous_end) || + start_glyph > 0xFFFF || end - start > 0xFFFF - start_glyph) { + throw std::runtime_error("sfnt: invalid cmap format 12 group"); + } + previous_end = end; for (std::uint32_t code = start; code <= end; ++code) { map(code, static_cast(start_glyph + (code - start))); } @@ -435,14 +493,14 @@ void SfntFont::read_name() { read_name_entry(t.substr(6 + static_cast(i) * 12))); } - const auto score_entry = [](const NameEntry &entry) { + const auto score_entry = [](const NameEntry &entry) -> std::uint32_t { // Only the full (4) and PostScript (6) names are usable; everything else // scores below the empty default so it is never selected. if (entry.name_id != NameId::full && entry.name_id != NameId::postscript) { return 0; } - const int score = (entry.name_id == NameId::postscript ? 10 : 0) + - (entry.utf16() ? 2 : 1); + const std::uint32_t score = (entry.name_id == NameId::postscript ? 10 : 0) + + (entry.utf16() ? 2 : 1); return score; }; @@ -450,7 +508,8 @@ void SfntFont::read_name() { if (best_entry != entries.end() && score_entry(*best_entry) > 0) { // name_local_offset is relative to the string storage (string_offset). - const std::string_view value = t.substr( + const std::string_view value = checked_slice( + t, static_cast(string_offset) + best_entry->name_local_offset, best_entry->name_length); m_name = best_entry->utf16() ? read_utf16be(value) : read_latin1(value); @@ -511,7 +570,7 @@ std::string SfntFont::write() const { if (tag == "cmap" || !is_kept_table(tag)) { continue; } - std::string data = m_data.substr(location.offset, location.length); + std::string data(table_data(location)); if (tag == "hmtx") { // OTS rejects an `hmtx` shorter than `hhea` and `maxp` say. const std::size_t side_bearings = diff --git a/test/src/internal/font/sfnt_font.cpp b/test/src/internal/font/sfnt_font.cpp index a0bc2d417..0062c6a38 100644 --- a/test/src/internal/font/sfnt_font.cpp +++ b/test/src/internal/font/sfnt_font.cpp @@ -5,7 +5,9 @@ #include +#include #include +#include #include #include #include @@ -312,3 +314,72 @@ TEST(SfntFont, write_pads_a_short_hmtx) { EXPECT_EQ(hmtx_length, 2 * 4 + 3 * 2); EXPECT_EQ(SfntFont(written).advance_width(4), 600); } + +TEST(SfntFont, rejects_table_ranges_and_invalid_headers) { + EXPECT_THROW(SfntFont(std::string(12, '\0')), std::runtime_error); + std::string collection("ttcf"); + collection.resize(16, '\0'); + EXPECT_THROW(SfntFont{collection}, std::runtime_error); + + std::string bytes = sample_font(cmap_table(3, 1, cmap_format4('A', 3))); + bs::write_u32_be(bytes, 24, static_cast(bytes.size())); + EXPECT_THROW(SfntFont{bytes}, std::runtime_error); + std::string head = head_table(); + bs::write_u16_be(head, 18, 0); + EXPECT_THROW(SfntFont(build_sfnt({{"head", head}})), std::runtime_error); + std::string name = name_table("A"); + bs::write_u16_be(name, 14, 4); + EXPECT_THROW(SfntFont(build_sfnt({{"name", name}})), std::runtime_error); + bs::write_u16_be(name, 14, 1); + EXPECT_THROW(SfntFont(build_sfnt({{"name", name}})), std::runtime_error); +} + +TEST(SfntFont, rejects_invalid_cmap_groups_before_expanding_them) { + const std::string valid = cmap_format12(0x1f600, 2); + const std::array, 5> patches{ + {{20, std::numeric_limits::max()}, + {20, 0x1f5ff}, + {24, 0xFFFF}, + {12, 2}, + {4, 16}}}; + for (const auto &[offset, value] : patches) { + std::string subtable = valid; + bs::write_u32_be(subtable, offset, value); + EXPECT_THROW(SfntFont(sample_font(cmap_table(3, 10, subtable))), + std::exception); + } + std::string overlapping = valid; + overlapping += valid.substr(16); + bs::write_u32_be(overlapping, 4, + static_cast(overlapping.size())); + bs::write_u32_be(overlapping, 12, 2); + EXPECT_THROW(SfntFont(sample_font(cmap_table(3, 10, overlapping))), + std::runtime_error); + + std::string short_subtable = cmap_format4('A', 3); + bs::write_u16_be(short_subtable, 2, 16); + EXPECT_THROW(SfntFont(sample_font(cmap_table(3, 1, short_subtable))), + std::exception); + short_subtable = cmap_format4('A', 3); + bs::write_u16_be(short_subtable, 6, 3); + EXPECT_THROW(SfntFont(sample_font(cmap_table(3, 1, short_subtable))), + std::runtime_error); +} + +TEST(SfntFont, unsupported_cmap_does_not_hide_a_supported_one) { + std::string cmap; + bs::put_u16_be(cmap, 0); + bs::put_u16_be(cmap, 2); + bs::put_u16_be(cmap, 0); + bs::put_u16_be(cmap, 6); // Unicode full repertoire, unsupported format 13. + bs::put_u32_be(cmap, 20); + bs::put_u16_be(cmap, 3); + bs::put_u16_be(cmap, 1); + bs::put_u32_be(cmap, 22); + bs::put_u16_be(cmap, 13); + std::string supported = cmap_format4('A', 3); + bs::write_u16_be(supported, 30, 0xFFFF); // The terminator offset is unused. + cmap += supported; + const SfntFont font(sample_font(cmap)); + EXPECT_EQ(font.glyph_for_code_point('B'), 2); +} From f470f387f704d813fef3c53236453d2471d2e736 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 5 Oct 2026 13:00:42 +0200 Subject: [PATCH 2/3] fix(font): drop only the broken part of a quirky SFNT font A PDF addresses the glyphs of an embedded font by ID, so a refused font falls back to a substitute and shows the wrong glyphs. Like FreeType, a broken cmap subtable now drops only itself, and the next supported one is used. A bad format-4 segment is skipped. A table or name that runs past its end is clipped, an odd UTF-16 name length and a duplicate table tag are accepted. A table offset past the end and a zero unitsPerEm still throw. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Gb1fLafqqzehpqPuU6uBfn --- CHANGELOG.md | 5 +- src/odr/internal/font/sfnt_font.cpp | 67 +++++++++++++++------------ test/src/internal/font/sfnt_font.cpp | 69 ++++++++++++++++++++-------- 3 files changed, 90 insertions(+), 51 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c7b09fed..6e680b690 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,9 +16,8 @@ The release run heads these entries with the version and opens a fresh ## Unreleased -- SFNT fonts validate table and character-map ranges before expanding them, - preventing overflow and unbounded loops. Unsupported character-map formats no - longer hide supported alternatives; truncated names and zero em sizes refuse. +- SFNT fonts check character-map ranges before they expand them. A broken or + unsupported character map no longer hides the next supported one. - Legacy Office reports unencrypted documents as decodable and derives metadata from the shared file-type table. diff --git a/src/odr/internal/font/sfnt_font.cpp b/src/odr/internal/font/sfnt_font.cpp index f4f5afb7a..79a6cc066 100644 --- a/src/odr/internal/font/sfnt_font.cpp +++ b/src/odr/internal/font/sfnt_font.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -19,15 +20,6 @@ namespace bs = util::byte_string; namespace { -std::string_view checked_slice(const std::string_view bytes, - const std::size_t offset, - const std::size_t length) { - if (offset > bytes.size() || length > bytes.size() - offset) { - throw std::runtime_error("sfnt: table range exceeds its container"); - } - return bytes.substr(offset, length); -} - // The enumerators below are the on-disk codes (OpenType spec). /// `cmap`/`name` platform IDs. @@ -115,9 +107,6 @@ struct NameEntry { /// included, into UTF-8. @p d is exactly the string's bytes. [[nodiscard]] std::string read_utf16be(const std::string_view d) { const std::size_t len = d.size(); - if (len % 2 != 0) { - throw std::runtime_error("sfnt: odd UTF-16 name length"); - } std::string out; for (std::size_t i = 0; i + 1 < len; i += 2) { char32_t cp = bs::read_u16_be(d.substr(i)); @@ -226,7 +215,11 @@ void SfntFont::parse() { } std::string_view SfntFont::table_data(const Table table) const { - return checked_slice(m_data, table.offset, table.length); + if (table.offset > m_data.size()) { + throw std::runtime_error("sfnt: table offset exceeds the font"); + } + // Like FreeType, clip a table that runs past the end of the font. + return std::string_view{m_data}.substr(table.offset, table.length); } void SfntFont::read_directory(const std::string_view sfnt) { @@ -247,9 +240,7 @@ void SfntFont::read_directory(const std::string_view sfnt) { std::string tag{sfnt.substr(entry, 4)}; const std::uint32_t offset = bs::read_u32_be(sfnt.substr(entry + 8)); const std::uint32_t length = bs::read_u32_be(sfnt.substr(entry + 12)); - if (!m_tables.emplace(std::move(tag), Table{offset, length}).second) { - throw std::runtime_error("sfnt: duplicate table tag"); - } + m_tables.emplace(std::move(tag), Table{offset, length}); } } @@ -319,6 +310,9 @@ void SfntFont::read_cmap() { }); const auto score = [t](const CmapEntry &entry) -> std::uint32_t { + if (t.size() < 2 || entry.offset > t.size() - 2) { + return 0; + } switch (static_cast(bs::read_u16_be(t.substr(entry.offset)))) { case CmapFormat::byte_encoding: case CmapFormat::segment_mapping: @@ -329,12 +323,22 @@ void SfntFont::read_cmap() { return 0; } }; - const auto best_entry = std::ranges::max_element(entries, {}, score); + std::ranges::stable_sort(entries, std::ranges::greater{}, score); - if (best_entry != entries.end() && score(*best_entry) > 0) { - // The encoding record's offset is relative to the start of the `cmap` - // table. - read_cmap_subtable(t.substr(best_entry->offset)); + for (const CmapEntry &entry : entries) { + if (score(entry) == 0) { + break; + } + // Like FreeType, a broken subtable drops only itself, because a PDF + // addresses the glyphs of an embedded font by ID. + try { + // The encoding record's offset is relative to the start of the `cmap` + // table. + read_cmap_subtable(t.substr(entry.offset)); + break; + } catch (const std::exception &) { + m_cmap.clear(); + } } update_reverse(); @@ -354,7 +358,7 @@ void SfntFont::read_cmap_subtable(std::string_view s) { const std::size_t length = format == CmapFormat::segmented_coverage ? bs::read_u32_be(s.substr(4)) : bs::read_u16_be(s.substr(2)); - s = checked_slice(s, 0, length); + s = s.substr(0, length); const auto map = [this](const char32_t code, const std::uint16_t glyph) { if (glyph == 0) { return; @@ -405,15 +409,19 @@ void SfntFont::read_cmap_subtable(std::string_view s) { } const std::vector glyph_ids = read_u16_vector(s.substr(header), (length - header) / 2); + std::int32_t previous_end = -1; for (std::size_t seg = 0; seg < segs; ++seg) { const std::uint16_t start = start_codes.at(seg); const std::uint16_t end = end_codes.at(seg); const std::uint16_t delta = id_deltas.at(seg); const std::uint16_t range = id_range_offsets.at(seg); - if (start > end || (seg > 0 && start <= end_codes[seg - 1]) || + // Skip a bad segment and keep the rest. In ascending order, the + // segments cover each code once. + if (start > end || start <= previous_end || (start != 0xFFFF && range % 2 != 0)) { - throw std::runtime_error("sfnt: invalid cmap segment"); + continue; } + previous_end = end; for (std::uint32_t code = start; code <= end && code != 0xffff; ++code) { std::uint16_t glyph = 0; if (range == 0) { @@ -508,11 +516,12 @@ void SfntFont::read_name() { if (best_entry != entries.end() && score_entry(*best_entry) > 0) { // name_local_offset is relative to the string storage (string_offset). - const std::string_view value = checked_slice( - t, - static_cast(string_offset) + best_entry->name_local_offset, - best_entry->name_length); - m_name = best_entry->utf16() ? read_utf16be(value) : read_latin1(value); + const std::size_t offset = + static_cast(string_offset) + best_entry->name_local_offset; + if (offset <= t.size()) { + const std::string_view value = t.substr(offset, best_entry->name_length); + m_name = best_entry->utf16() ? read_utf16be(value) : read_latin1(value); + } } } diff --git a/test/src/internal/font/sfnt_font.cpp b/test/src/internal/font/sfnt_font.cpp index 0062c6a38..d167ea362 100644 --- a/test/src/internal/font/sfnt_font.cpp +++ b/test/src/internal/font/sfnt_font.cpp @@ -315,26 +315,31 @@ TEST(SfntFont, write_pads_a_short_hmtx) { EXPECT_EQ(SfntFont(written).advance_width(4), 600); } -TEST(SfntFont, rejects_table_ranges_and_invalid_headers) { +TEST(SfntFont, rejects_invalid_headers_and_clips_quirky_ranges) { EXPECT_THROW(SfntFont(std::string(12, '\0')), std::runtime_error); std::string collection("ttcf"); collection.resize(16, '\0'); EXPECT_THROW(SfntFont{collection}, std::runtime_error); std::string bytes = sample_font(cmap_table(3, 1, cmap_format4('A', 3))); - bs::write_u32_be(bytes, 24, static_cast(bytes.size())); + std::string clipped = bytes; + bs::write_u32_be(clipped, 24, static_cast(bytes.size())); + EXPECT_EQ(SfntFont(clipped).glyph_for_code_point('B'), 2); + bs::write_u32_be(bytes, 20, static_cast(bytes.size() + 1)); EXPECT_THROW(SfntFont{bytes}, std::runtime_error); + std::string head = head_table(); bs::write_u16_be(head, 18, 0); EXPECT_THROW(SfntFont(build_sfnt({{"head", head}})), std::runtime_error); + std::string name = name_table("A"); bs::write_u16_be(name, 14, 4); - EXPECT_THROW(SfntFont(build_sfnt({{"name", name}})), std::runtime_error); + EXPECT_EQ(SfntFont(build_sfnt({{"name", name}})).name(), "A"); bs::write_u16_be(name, 14, 1); - EXPECT_THROW(SfntFont(build_sfnt({{"name", name}})), std::runtime_error); + EXPECT_EQ(SfntFont(build_sfnt({{"name", name}})).name(), ""); } -TEST(SfntFont, rejects_invalid_cmap_groups_before_expanding_them) { +TEST(SfntFont, drops_an_invalid_cmap_and_keeps_the_font) { const std::string valid = cmap_format12(0x1f600, 2); const std::array, 5> patches{ {{20, std::numeric_limits::max()}, @@ -345,41 +350,67 @@ TEST(SfntFont, rejects_invalid_cmap_groups_before_expanding_them) { for (const auto &[offset, value] : patches) { std::string subtable = valid; bs::write_u32_be(subtable, offset, value); - EXPECT_THROW(SfntFont(sample_font(cmap_table(3, 10, subtable))), - std::exception); + const SfntFont font(sample_font(cmap_table(3, 10, subtable))); + EXPECT_TRUE(font.cmap().empty()); + EXPECT_EQ(font.advance_width(1), 600); } std::string overlapping = valid; overlapping += valid.substr(16); bs::write_u32_be(overlapping, 4, static_cast(overlapping.size())); bs::write_u32_be(overlapping, 12, 2); - EXPECT_THROW(SfntFont(sample_font(cmap_table(3, 10, overlapping))), - std::runtime_error); + EXPECT_TRUE( + SfntFont(sample_font(cmap_table(3, 10, overlapping))).cmap().empty()); std::string short_subtable = cmap_format4('A', 3); bs::write_u16_be(short_subtable, 2, 16); - EXPECT_THROW(SfntFont(sample_font(cmap_table(3, 1, short_subtable))), - std::exception); + EXPECT_TRUE( + SfntFont(sample_font(cmap_table(3, 1, short_subtable))).cmap().empty()); short_subtable = cmap_format4('A', 3); bs::write_u16_be(short_subtable, 6, 3); - EXPECT_THROW(SfntFont(sample_font(cmap_table(3, 1, short_subtable))), - std::runtime_error); + EXPECT_TRUE( + SfntFont(sample_font(cmap_table(3, 1, short_subtable))).cmap().empty()); } -TEST(SfntFont, unsupported_cmap_does_not_hide_a_supported_one) { +TEST(SfntFont, skips_a_format4_segment_out_of_order) { + std::string subtable = cmap_format4('A', 3); + bs::write_u16_be(subtable, 14, 'A' - 1); // endCode[0] below startCode[0] + const SfntFont font(sample_font(cmap_table(3, 1, subtable))); + EXPECT_EQ(font.glyph_for_code_point('A'), 0); +} + +namespace { + +/// One cmap with a (0, 6) record for @p first and a (3, 1) record for a valid +/// format-4 subtable after it. +std::string cmap_with_fallback(const std::string &first) { std::string cmap; bs::put_u16_be(cmap, 0); bs::put_u16_be(cmap, 2); bs::put_u16_be(cmap, 0); - bs::put_u16_be(cmap, 6); // Unicode full repertoire, unsupported format 13. + bs::put_u16_be(cmap, 6); // Unicode full repertoire. bs::put_u32_be(cmap, 20); bs::put_u16_be(cmap, 3); bs::put_u16_be(cmap, 1); - bs::put_u32_be(cmap, 22); - bs::put_u16_be(cmap, 13); + bs::put_u32_be(cmap, static_cast(20 + first.size())); + cmap += first; std::string supported = cmap_format4('A', 3); bs::write_u16_be(supported, 30, 0xFFFF); // The terminator offset is unused. cmap += supported; - const SfntFont font(sample_font(cmap)); - EXPECT_EQ(font.glyph_for_code_point('B'), 2); + return cmap; +} + +} // namespace + +TEST(SfntFont, unsupported_cmap_does_not_hide_a_supported_one) { + std::string unsupported; + bs::put_u16_be(unsupported, 13); + EXPECT_EQ(SfntFont(sample_font(cmap_with_fallback(unsupported))) + .glyph_for_code_point('B'), + 2); + std::string broken = cmap_format12(0x1f600, 2); + bs::write_u32_be(broken, 20, 0x1f5ff); + EXPECT_EQ(SfntFont(sample_font(cmap_with_fallback(broken))) + .glyph_for_code_point('B'), + 2); } From 117118ec99be3dca9cbc4b7ecf57f98753e1939e Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 5 Oct 2026 13:13:29 +0200 Subject: [PATCH 3/3] Apply suggestion from @andiwand --- src/odr/internal/font/sfnt_font.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/odr/internal/font/sfnt_font.cpp b/src/odr/internal/font/sfnt_font.cpp index 79a6cc066..3a612be83 100644 --- a/src/odr/internal/font/sfnt_font.cpp +++ b/src/odr/internal/font/sfnt_font.cpp @@ -579,7 +579,7 @@ std::string SfntFont::write() const { if (tag == "cmap" || !is_kept_table(tag)) { continue; } - std::string data(table_data(location)); + std::string data = table_data(location); if (tag == "hmtx") { // OTS rejects an `hmtx` shorter than `hhea` and `maxp` say. const std::size_t side_bearings =