From c68ab4b52b898f5c168cf662137b3dce922c29d9 Mon Sep 17 00:00:00 2001 From: Ebrahim Byagowi Date: Thu, 21 May 2020 00:25:17 +0430 Subject: [PATCH] Fix _get_ligature_caret's oob read issue AAT::Lookup has no other way to detect whether it is returned from a real and sanitized font data or from a null pool, this checks if the table has been recognized valid by sanitizer by checking table's major version which is zero if returned from a null pool and non-zero if is from a sanitized font data, it is expected the other calls of the table (unlikely to have more calls however) also do a similar version check before calling the lookups used on the table. --- src/hb-aat-layout-lcar-table.hh | 9 +++++++++ test/api/test-ot-face.c | 2 ++ test/api/test-ot-ligature-carets.c | 14 ++++---------- test/fuzzing/hb-draw-fuzzer.cc | 3 +++ 4 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/hb-aat-layout-lcar-table.hh b/src/hb-aat-layout-lcar-table.hh index 7063b386c..90cfd9292 100644 --- a/src/hb-aat-layout-lcar-table.hh +++ b/src/hb-aat-layout-lcar-table.hh @@ -116,6 +116,8 @@ struct lcar { static constexpr hb_tag_t tableTag = HB_AAT_TAG_lcar; + bool has_data () const { return version.major; } + unsigned int get_lig_carets (hb_font_t *font, hb_direction_t direction, hb_codepoint_t glyph, @@ -123,6 +125,13 @@ struct lcar unsigned int *caret_count /* IN/OUT */, hb_position_t *caret_array /* OUT */) const { + if (!has_data ()) + { + if (caret_count) + *caret_count = 0; + return 0; + } + switch (format) { case 0: return u.format0.get_lig_carets (font, direction, glyph, start_offset, diff --git a/test/api/test-ot-face.c b/test/api/test-ot-face.c index 5eac868c5..e3d64f627 100644 --- a/test/api/test-ot-face.c +++ b/test/api/test-ot-face.c @@ -95,6 +95,8 @@ test_font (hb_font_t *font, hb_codepoint_t cp) hb_ot_layout_has_substitution (face); hb_ot_layout_has_positioning (face); + hb_ot_layout_get_ligature_carets (font, HB_DIRECTION_LTR, cp, 0, NULL, NULL); + hb_ot_math_has_data (face); hb_ot_math_get_constant (font, HB_OT_MATH_CONSTANT_MATH_LEADING); hb_ot_math_get_glyph_italics_correction (font, cp); diff --git a/test/api/test-ot-ligature-carets.c b/test/api/test-ot-ligature-carets.c index adfa871bc..39f568914 100644 --- a/test/api/test-ot-ligature-carets.c +++ b/test/api/test-ot-ligature-carets.c @@ -96,16 +96,12 @@ test_ot_layout_get_ligature_carets_ot_gsub (void) hb_position_t caret_array[16]; { -/* unsigned caret_count = 16; - g_assert_cmpuint (210, ==, hb_ot_layout_get_ligature_carets (font, HB_DIRECTION_LTR, + g_assert_cmpuint (0, ==, hb_ot_layout_get_ligature_carets (font, HB_DIRECTION_LTR, 188, 0, &caret_count, caret_array)); - g_assert_cmpuint (3, ==, caret_count); - g_assert_cmpuint (2718, ==, caret_array[0]); - g_assert_cmpuint (5438, ==, caret_array[1]); - g_assert_cmpuint (5438, ==, caret_array[1]); -*/ + + g_assert_cmpuint (0, ==, caret_count); } { @@ -118,17 +114,15 @@ test_ot_layout_get_ligature_carets_ot_gsub (void) g_assert_cmpuint (2718, ==, caret_array[0]); g_assert_cmpuint (5438, ==, caret_array[1]); g_assert_cmpuint (5438, ==, caret_array[1]); - } { -/* unsigned caret_count = 16; g_assert_cmpuint (0, ==, hb_ot_layout_get_ligature_carets (font, HB_DIRECTION_LTR, 1021, 0, &caret_count, caret_array)); + g_assert_cmpuint (0, ==, caret_count); -*/ } { diff --git a/test/fuzzing/hb-draw-fuzzer.cc b/test/fuzzing/hb-draw-fuzzer.cc index 61f2740df..32287c8ff 100644 --- a/test/fuzzing/hb-draw-fuzzer.cc +++ b/test/fuzzing/hb-draw-fuzzer.cc @@ -121,6 +121,9 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) counter += !!extents.width + !!extents.height + !!extents.x_bearing + !!extents.y_bearing; if (!counter) counter += 1; + + /* misc calls, TODO: test more of calls working with some specific gid */ + hb_ot_layout_get_ligature_carets (font, HB_DIRECTION_LTR, gid, 0, nullptr, nullptr); } assert (counter); #ifdef HB_EXPERIMENTAL_API