[fontations] Fix OOB write on glyph-name callback with size == 0 (#5946) (#5947)
* [fontations] Fix OOB write on glyph-name callback with size == 0
The Rust `_hb_fontations_glyph_name()` callback computed
`glyph_name.len().min(size as usize - 1)`. When a caller passed
`size == 0`, `size as usize - 1` underflowed a `usize` to
`usize::MAX`, the `min()` clamp collapsed to `glyph_name.len()`,
and the subsequent `copy_nonoverlapping` / trailing NUL wrote
`glyph_name.len() + 1` bytes into a zero-capacity caller buffer.
This is an out-of-bounds write reachable from the public API
whenever the build was configured with `-Dfontations=enabled`
and the application had called `hb_fontations_font_set_funcs()`.
Short-circuit the `size == 0` case before the subtraction.
Return `true` in that case to match the existing post and CFF1
backends (`hb-ot-post-table.hh`, `hb-ot-cff1-table.hh`), which
both early-out with `return true` on `!buf_len` — the glyph
does have a name; we simply had no buffer to write it into.
Regression tests added in `test/api/test-glyph-names.c` cover
the zero-size path on post and CFF1 backends, a sub-font, a
NULL buffer, and `size == 1` / small-buffer / exact-fit cases
so the truncation path does not silently regress.
Fixes: https://github.com/harfbuzz/harfbuzz/issues/5946
Assisted-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* [font] Reject glyph-name lookups with size == 0 at the wrapper
`hb_font_t::get_glyph_name()` previously pre-wrote the trailing
NUL only when `size != 0` but still dispatched to the backend
callback with `size == 0`. Every current glyph-name backend has
to re-validate that argument: `post` and `cff1` short-circuit
with `return true`, the new Rust `fontations` callback now
short-circuits too (prior commit), and the CoreText callback in
`hb-coretext-font.cc` has a structurally similar `(CFIndex)size
- 1` pattern.
Normalize this at the wrapper layer: return `false` immediately
on `size == 0`, before any backend dispatch and before the
`*name = '\0'` write. This makes the zero-size case a single,
auditable no-op for every glyph-name provider (native or Rust,
current or future) and removes the CoreText underflow from the
reachable surface through the same chokepoint.
Observable change: callers that invoked
`hb_font_get_glyph_name(font, gid, buf, 0)` on post/cff1 backed
fonts used to get `true` back; they now get `false`. `size == 0`
queries carry no useful information for the caller, so this is
considered a safe hardening rather than a contract break.
Assisted-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* [font] Preserve glyph-name zero-size probe semantics
Keep `hb_font_get_glyph_name(..., NULL, 0)` as a valid
return-value-only probe instead of rejecting `size == 0` in the
wrapper.
Fix the remaining backend-specific zero-size paths in hb-ft and
hb-coretext-font so they answer the probe without dereferencing the
caller buffer, and update the API tests to assert the preserved
behavior.
Testing: CCACHE_DISABLE=1 meson test -C build test-glyph-names
Testing: CCACHE_DISABLE=1 meson test -C build test-ft
Assisted-by: OpenAI Codex <codex@openai.com>
---------
Co-authored-by: jeffhuang <jeff@docker.xydrsucermoubd24xgo33yhsgd.bx.internal.cloudapp.net>
diff --git a/src/hb-coretext-font.cc b/src/hb-coretext-font.cc
index 939e620..9f46928 100644
--- a/src/hb-coretext-font.cc
+++ b/src/hb-coretext-font.cc
@@ -409,6 +409,12 @@
if (!cf_name)
return false;
+ if (!size)
+ {
+ CFRelease (cf_name);
+ return true;
+ }
+
CFIndex len = CFStringGetLength (cf_name);
if (len > (CFIndex)size - 1)
len = (CFIndex)size - 1;
diff --git a/src/hb-ft.cc b/src/hb-ft.cc
index 5fef168..8ac5db7 100644
--- a/src/hb-ft.cc
+++ b/src/hb-ft.cc
@@ -766,6 +766,12 @@
hb_lock_t lock (ft_font->lock);
FT_Face ft_face = ft_font->ft_face;
+ if (!size)
+ {
+ char buf[128];
+ return !FT_Get_Glyph_Name (ft_face, glyph, buf, sizeof (buf)) && *buf;
+ }
+
hb_bool_t ret = !FT_Get_Glyph_Name (ft_face, glyph, name, size);
if (ret && (size && !*name))
ret = false;
diff --git a/src/rust/font.rs b/src/rust/font.rs
index 8e6b526..efe0c7c 100644
--- a/src/rust/font.rs
+++ b/src/rust/font.rs
@@ -981,9 +981,12 @@
let data = unsafe { &mut *(font_data as *mut FontationsData) };
if let Some(glyph_name) = data.glyph_names.get(GlyphId::new(glyph)) {
+ if size == 0 {
+ return true as hb_bool_t;
+ }
let glyph_name = glyph_name.as_str();
// Copy the glyph name into the buffer, up to size-1 bytes
- let len = glyph_name.len().min(size as usize - 1);
+ let len = glyph_name.len().min((size as usize) - 1);
unsafe {
std::slice::from_raw_parts_mut(name as *mut u8, len)
.copy_from_slice(&glyph_name.as_bytes()[..len]);
diff --git a/test/api/test-ft.c b/test/api/test-ft.c
index ae792cf..e32e0b6 100644
--- a/test/api/test-ft.c
+++ b/test/api/test-ft.c
@@ -80,7 +80,7 @@
init_freetype ();
- ft_face = get_ft_face ("fonts/Cantarell.A.otf");
+ ft_face = get_ft_face ("fonts/adwaita.ttf");
g_assert_nonnull (ft_face);
g_assert_nonnull (FT_Get_Font_Format (ft_face));
@@ -135,6 +135,77 @@
cleanup_freetype ();
}
+static void
+test_native_ft_glyph_name_zero_size_probe (void)
+{
+ static const char *files[] = {
+ "fonts/adwaita.ttf",
+ "fonts/SourceSansPro-Regular.otf",
+ "fonts/Cantarell.A.otf",
+ };
+ FT_Face ft_face = NULL;
+ hb_font_t *font = NULL;
+ hb_bool_t ret;
+ hb_codepoint_t glyph = 0;
+ char name[64];
+ char guard[4] = { 0x7f, 0x7f, 0x7f, 0x7f };
+ unsigned int i;
+
+ init_freetype ();
+
+ for (i = 0; i < G_N_ELEMENTS (files); i++)
+ {
+ unsigned int gid;
+
+ ft_face = get_ft_face (files[i]);
+ g_assert_nonnull (ft_face);
+
+ font = hb_ft_font_create_referenced (ft_face);
+ g_assert_nonnull (font);
+ hb_ft_font_set_funcs (font);
+
+ for (gid = 0; gid < (unsigned int) ft_face->num_glyphs; gid++)
+ if (hb_font_get_glyph_name (font, gid, name, sizeof (name)))
+ {
+ glyph = gid;
+ goto found;
+ }
+
+ hb_font_destroy (font);
+ font = NULL;
+ FT_Done_Face (ft_face);
+ ft_face = NULL;
+ }
+
+ g_test_skip ("No FreeType glyph-name test font available");
+ cleanup_freetype ();
+ return;
+
+found:
+
+ ret = hb_font_get_glyph_name (font, glyph, name, sizeof (name));
+ g_assert_true (ret);
+ g_assert_true (*name);
+
+ ret = hb_font_get_glyph_name (font, glyph, NULL, 0);
+ g_assert_true (ret);
+
+ ret = hb_font_get_glyph_name (font, glyph, guard, 0);
+ g_assert_true (ret);
+ g_assert_cmpint (guard[0], ==, 0x7f);
+ g_assert_cmpint (guard[1], ==, 0x7f);
+ g_assert_cmpint (guard[2], ==, 0x7f);
+ g_assert_cmpint (guard[3], ==, 0x7f);
+
+ ret = hb_font_get_glyph_name (font, 0xFFFFFF, NULL, 0);
+ g_assert_false (ret);
+
+ hb_font_destroy (font);
+ FT_Done_Face (ft_face);
+
+ cleanup_freetype ();
+}
+
int
main (int argc, char **argv)
{
@@ -142,6 +213,7 @@
hb_test_add (test_native_ft_basic);
hb_test_add (test_native_ft_set_funcs_preserves_load_flags);
+ hb_test_add (test_native_ft_glyph_name_zero_size_probe);
return hb_test_run ();
}
diff --git a/test/api/test-glyph-names.c b/test/api/test-glyph-names.c
index 48ca06b..50dd104 100644
--- a/test/api/test-glyph-names.c
+++ b/test/api/test-glyph-names.c
@@ -100,6 +100,277 @@
hb_face_destroy (face);
}
+/* Regression tests for issue #5946: hb_font_get_glyph_name (..., size=0)
+ * must not underflow and write past a zero-capacity buffer, while preserving
+ * the long-standing "return value only" probe behavior. */
+
+static void
+test_glyph_names_zero_size (void)
+{
+ hb_face_t *face;
+ hb_font_t *font;
+ char guard[4] = { 0x7f, 0x7f, 0x7f, 0x7f };
+
+ face = hb_test_open_font_file ("fonts/adwaita.ttf");
+ font = hb_font_create (face);
+
+ /* size == 0 must not write to the buffer. */
+ g_assert_true (hb_font_get_glyph_name (font, 1, guard, 0));
+ g_assert_cmpint (guard[0], ==, 0x7f);
+ g_assert_cmpint (guard[1], ==, 0x7f);
+ g_assert_cmpint (guard[2], ==, 0x7f);
+ g_assert_cmpint (guard[3], ==, 0x7f);
+
+ hb_font_destroy (font);
+ hb_face_destroy (face);
+}
+
+static void
+test_glyph_names_zero_size_post (void)
+{
+ hb_face_t *face;
+ hb_font_t *font;
+ hb_bool_t ret;
+ char guard[8] = { 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f };
+ unsigned int i;
+
+ face = hb_test_open_font_file ("fonts/adwaita.ttf");
+ font = hb_font_create (face);
+
+ /* Named glyph, size == 0: keep the old return-value-only probe behavior
+ * and leave the buffer untouched. */
+ ret = hb_font_get_glyph_name (font, 1, guard, 0);
+ g_assert_true (ret);
+ for (i = 0; i < 8; i++)
+ g_assert_cmpint (guard[i], ==, 0x7f);
+
+ /* .notdef, size == 0: same invariant. Confirms it is not glyph-specific. */
+ ret = hb_font_get_glyph_name (font, 0, guard, 0);
+ g_assert_true (ret);
+ for (i = 0; i < 8; i++)
+ g_assert_cmpint (guard[i], ==, 0x7f);
+
+ /* Perform a successful sized lookup first, then retry with size == 0.
+ * Confirms no residual state corrupts the zero-size path. */
+ {
+ char name[64];
+ ret = hb_font_get_glyph_name (font, 2, name, 64);
+ g_assert_true (ret);
+ g_assert_cmpstr (name, ==, "icon0");
+ }
+ ret = hb_font_get_glyph_name (font, 2, guard, 0);
+ g_assert_true (ret);
+ for (i = 0; i < 8; i++)
+ g_assert_cmpint (guard[i], ==, 0x7f);
+
+ hb_font_destroy (font);
+ hb_face_destroy (face);
+}
+
+static void
+test_glyph_names_zero_size_cff (void)
+{
+ hb_face_t *face;
+ hb_font_t *font;
+ hb_bool_t ret;
+ char guard[8] = { 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f };
+ unsigned int i;
+
+ face = hb_test_open_font_file ("fonts/SourceSansPro-Regular.otf");
+ font = hb_font_create (face);
+
+ /* CFF-backed named glyph, size == 0: preserve the return-value-only probe
+ * behavior without writing into the buffer. */
+ ret = hb_font_get_glyph_name (font, 2, guard, 0);
+ g_assert_true (ret);
+ for (i = 0; i < 8; i++)
+ g_assert_cmpint (guard[i], ==, 0x7f);
+
+ /* .notdef on CFF. */
+ ret = hb_font_get_glyph_name (font, 0, guard, 0);
+ g_assert_true (ret);
+ for (i = 0; i < 8; i++)
+ g_assert_cmpint (guard[i], ==, 0x7f);
+
+ hb_font_destroy (font);
+ hb_face_destroy (face);
+}
+
+static void
+test_glyph_names_zero_size_unnamed_glyph (void)
+{
+ /* size == 0 on a glyph beyond the font's range must also return false
+ * and must not write. */
+ hb_face_t *face;
+ hb_font_t *font;
+ hb_bool_t ret;
+ char guard[4] = { 0x7f, 0x7f, 0x7f, 0x7f };
+ unsigned int i;
+
+ face = hb_test_open_font_file ("fonts/adwaita.ttf");
+ font = hb_font_create (face);
+
+ ret = hb_font_get_glyph_name (font, 100, guard, 0);
+ g_assert_false (ret);
+ for (i = 0; i < 4; i++)
+ g_assert_cmpint (guard[i], ==, 0x7f);
+
+ hb_font_destroy (font);
+ hb_face_destroy (face);
+}
+
+static void
+test_glyph_names_zero_size_null_buffer (void)
+{
+ /* name == NULL with size == 0 is a valid return-value-only probe. */
+ hb_face_t *face;
+ hb_font_t *font;
+ hb_bool_t ret;
+
+ face = hb_test_open_font_file ("fonts/adwaita.ttf");
+ font = hb_font_create (face);
+
+ /* Glyph with a name. */
+ ret = hb_font_get_glyph_name (font, 1, NULL, 0);
+ g_assert_true (ret);
+
+ /* Glyph beyond the font's range. */
+ ret = hb_font_get_glyph_name (font, 100, NULL, 0);
+ g_assert_false (ret);
+
+ hb_font_destroy (font);
+ hb_face_destroy (face);
+}
+
+static void
+test_glyph_names_size_one (void)
+{
+ /* Boundary: size == 1. The backend must produce exactly a trailing NUL
+ * at name[0] (len = min(|name|, size-1) = 0 bytes of name copied) and
+ * must not touch any byte past name[0]. */
+ hb_face_t *face;
+ hb_font_t *font;
+ hb_bool_t ret;
+ char buf[4] = { 0x7f, 0x7f, 0x7f, 0x7f };
+
+ face = hb_test_open_font_file ("fonts/adwaita.ttf");
+ font = hb_font_create (face);
+
+ ret = hb_font_get_glyph_name (font, 1, buf, 1);
+ g_assert_true (ret);
+ g_assert_cmpint (buf[0], ==, 0);
+ /* Bytes beyond the caller-declared capacity must remain intact. */
+ g_assert_cmpint (buf[1], ==, 0x7f);
+ g_assert_cmpint (buf[2], ==, 0x7f);
+ g_assert_cmpint (buf[3], ==, 0x7f);
+
+ hb_font_destroy (font);
+ hb_face_destroy (face);
+}
+
+static void
+test_glyph_names_small_buffer_truncation (void)
+{
+ /* Non-regression sanity: a small-but-nonzero buffer must truncate to
+ * size-1 bytes of the name plus a trailing NUL. */
+ hb_face_t *face;
+ hb_font_t *font;
+ hb_bool_t ret;
+ char buf[4] = { 0x7f, 0x7f, 0x7f, 0x7f };
+
+ face = hb_test_open_font_file ("fonts/adwaita.ttf");
+ font = hb_font_create (face);
+
+ /* ".notdef" truncated to capacity 4 -> ".no\0". */
+ ret = hb_font_get_glyph_name (font, 0, buf, 4);
+ g_assert_true (ret);
+ g_assert_cmpint (buf[0], ==, '.');
+ g_assert_cmpint (buf[1], ==, 'n');
+ g_assert_cmpint (buf[2], ==, 'o');
+ g_assert_cmpint (buf[3], ==, 0);
+
+ hb_font_destroy (font);
+ hb_face_destroy (face);
+}
+
+static void
+test_glyph_names_exact_fit_buffer (void)
+{
+ /* Non-regression sanity: a buffer exactly sized to fit the full name
+ * (including the trailing NUL) must hold the full name. For ".notdef"
+ * that is 8 bytes. */
+ hb_face_t *face;
+ hb_font_t *font;
+ hb_bool_t ret;
+ char buf[8] = { 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f };
+
+ face = hb_test_open_font_file ("fonts/adwaita.ttf");
+ font = hb_font_create (face);
+
+ ret = hb_font_get_glyph_name (font, 0, buf, 8);
+ g_assert_true (ret);
+ g_assert_cmpstr (buf, ==, ".notdef");
+ g_assert_cmpint (buf[7], ==, 0);
+
+ hb_font_destroy (font);
+ hb_face_destroy (face);
+}
+
+static void
+test_glyph_names_nonzero_sizes_unchanged (void)
+{
+ /* Non-regression sanity: the existing behaviour at typical sizes must
+ * be preserved by the fix. */
+ hb_face_t *face;
+ hb_font_t *font;
+ hb_bool_t ret;
+ char name[64];
+
+ face = hb_test_open_font_file ("fonts/adwaita.ttf");
+ font = hb_font_create (face);
+
+ ret = hb_font_get_glyph_name (font, 0, name, 64);
+ g_assert_true (ret);
+ g_assert_cmpstr (name, ==, ".notdef");
+
+ ret = hb_font_get_glyph_name (font, 1, name, 64);
+ g_assert_true (ret);
+ g_assert_cmpstr (name, ==, ".space");
+
+ /* Beyond last glyph still returns false. */
+ ret = hb_font_get_glyph_name (font, 100, name, 64);
+ g_assert_false (ret);
+
+ hb_font_destroy (font);
+ hb_face_destroy (face);
+}
+
+static void
+test_glyph_names_zero_size_sub_font (void)
+{
+ /* Sub-fonts delegate to the parent's glyph-name funcs. Preserve the
+ * return-value-only probe behavior through the parent chain as well. */
+ hb_face_t *face;
+ hb_font_t *parent;
+ hb_font_t *font;
+ hb_bool_t ret;
+ char guard[4] = { 0x7f, 0x7f, 0x7f, 0x7f };
+ unsigned int i;
+
+ face = hb_test_open_font_file ("fonts/adwaita.ttf");
+ parent = hb_font_create (face);
+ font = hb_font_create_sub_font (parent);
+
+ ret = hb_font_get_glyph_name (font, 1, guard, 0);
+ g_assert_true (ret);
+ for (i = 0; i < 4; i++)
+ g_assert_cmpint (guard[i], ==, 0x7f);
+
+ hb_font_destroy (font);
+ hb_font_destroy (parent);
+ hb_face_destroy (face);
+}
+
int
main (int argc, char **argv)
{
@@ -107,6 +378,16 @@
hb_test_add (test_glyph_names_post);
hb_test_add (test_glyph_names_cff);
+ hb_test_add (test_glyph_names_zero_size);
+ hb_test_add (test_glyph_names_zero_size_post);
+ hb_test_add (test_glyph_names_zero_size_cff);
+ hb_test_add (test_glyph_names_zero_size_unnamed_glyph);
+ hb_test_add (test_glyph_names_zero_size_null_buffer);
+ hb_test_add (test_glyph_names_size_one);
+ hb_test_add (test_glyph_names_small_buffer_truncation);
+ hb_test_add (test_glyph_names_exact_fit_buffer);
+ hb_test_add (test_glyph_names_nonzero_sizes_unchanged);
+ hb_test_add (test_glyph_names_zero_size_sub_font);
return hb_test_run();
}
