Skip to content

Commit 0ecb603

Browse files
robert-ancellloic-sharma
authored andcommitted
Fix bounds checking in FlAccessibleTextField (flutter#188137)
Add bounds validation to get_substring and get_string_at_offset to prevent out-of-bounds memory access when ATK clients pass offsets beyond the text length. - get_substring: clamp start and end to [0, length] and ensure start <= end before calling g_utf8_substring. - get_string_at_offset: clamp start and end to [0, n_attrs-1] before accessing the PangoLogAttr array. Add tests for offset-beyond-end, empty text, and offset-at-end boundary conditions.
1 parent 07b642a commit 0ecb603

2 files changed

Lines changed: 149 additions & 4 deletions

File tree

engine/src/flutter/shell/platform/linux/fl_accessible_text_field.cc

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,13 @@ static gchar* get_substring(FlAccessibleTextField* self,
3939
glong start,
4040
glong end) {
4141
const gchar* value = gtk_entry_buffer_get_text(self->buffer);
42+
glong length = g_utf8_strlen(value, -1);
4243
if (end == -1) {
4344
// g_utf8_substring() accepts -1 since 2.72
44-
end = g_utf8_strlen(value, -1);
45+
end = length;
4546
}
47+
start = CLAMP(start, 0, length);
48+
end = CLAMP(end, start, length);
4649
return g_utf8_substring(value, start, end);
4750
}
4851

@@ -76,16 +79,20 @@ static gchar* get_string_at_offset(FlAccessibleTextField* self,
7679
const PangoLogAttr* attrs =
7780
pango_layout_get_log_attrs_readonly(layout, &n_attrs);
7881

82+
start = CLAMP(start, 0, MAX(n_attrs - 1, 0));
83+
end = CLAMP(end, 0, MAX(n_attrs - 1, 0));
84+
7985
while (start > 0 && !is_start(&attrs[start])) {
8086
--start;
8187
}
82-
if (start_offset != nullptr) {
83-
*start_offset = start;
84-
}
8588

8689
while (end < n_attrs && !is_end(&attrs[end])) {
8790
++end;
8891
}
92+
93+
if (start_offset != nullptr) {
94+
*start_offset = start;
95+
}
8996
if (end_offset != nullptr) {
9097
*end_offset = end;
9198
}

engine/src/flutter/shell/platform/linux/fl_accessible_text_field_test.cc

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,33 @@ TEST(FlAccessibleTextFieldTest, GetText) {
218218
EXPECT_STREQ(tt, "tt");
219219
}
220220

221+
// Tests AtkText::get_text with out-of-bounds offsets.
222+
TEST(FlAccessibleTextFieldTest, GetTextBoundsChecking) {
223+
g_autoptr(FlDartProject) project = fl_dart_project_new();
224+
g_autoptr(FlEngine) engine = fl_engine_new(project);
225+
g_autoptr(FlAccessibleNode) node =
226+
fl_accessible_text_field_new(engine, 123, 1);
227+
228+
fl_accessible_node_set_value(node, "Hello");
229+
230+
// start beyond end of text
231+
g_autofree gchar* beyond = atk_text_get_text(ATK_TEXT(node), 100, -1);
232+
EXPECT_STREQ(beyond, "");
233+
234+
// end beyond text length
235+
g_autofree gchar* end_beyond = atk_text_get_text(ATK_TEXT(node), 2, 100);
236+
EXPECT_STREQ(end_beyond, "llo");
237+
238+
// both beyond text length
239+
g_autofree gchar* both_beyond = atk_text_get_text(ATK_TEXT(node), 50, 100);
240+
EXPECT_STREQ(both_beyond, "");
241+
242+
// empty buffer
243+
fl_accessible_node_set_value(node, "");
244+
g_autofree gchar* empty_beyond = atk_text_get_text(ATK_TEXT(node), 5, 10);
245+
EXPECT_STREQ(empty_beyond, "");
246+
}
247+
221248
// Tests AtkText::get_caret_offset.
222249
TEST(FlAccessibleTextFieldTest, GetCaretOffset) {
223250
g_autoptr(FlDartProject) project = fl_dart_project_new();
@@ -696,4 +723,115 @@ TEST(FlAccessibleTextFieldTest, TextBoundary) {
696723
EXPECT_EQ(end_offset, 70);
697724
}
698725

726+
// Tests that get_string_at_offset handles offset beyond text length.
727+
TEST(FlAccessibleTextFieldTest, TextBoundaryOffsetBeyondEnd) {
728+
g_autoptr(FlDartProject) project = fl_dart_project_new();
729+
g_autoptr(FlEngine) engine = fl_engine_new(project);
730+
g_autoptr(FlAccessibleNode) node =
731+
fl_accessible_text_field_new(engine, 123, 1);
732+
733+
fl_accessible_node_set_value(node, "Hello");
734+
735+
// Offset well beyond the text length should not crash and should return
736+
// a valid result clamped to the text boundaries.
737+
gint start_offset = -1, end_offset = -1;
738+
g_autofree gchar* char_result = atk_text_get_string_at_offset(
739+
ATK_TEXT(node), 100, ATK_TEXT_GRANULARITY_CHAR, &start_offset,
740+
&end_offset);
741+
EXPECT_NE(char_result, nullptr);
742+
EXPECT_GE(start_offset, 0);
743+
EXPECT_LE(end_offset, 5);
744+
745+
g_autofree gchar* word_result = atk_text_get_string_at_offset(
746+
ATK_TEXT(node), 100, ATK_TEXT_GRANULARITY_WORD, &start_offset,
747+
&end_offset);
748+
EXPECT_NE(word_result, nullptr);
749+
EXPECT_GE(start_offset, 0);
750+
EXPECT_LE(end_offset, 5);
751+
752+
g_autofree gchar* sentence_result = atk_text_get_string_at_offset(
753+
ATK_TEXT(node), 100, ATK_TEXT_GRANULARITY_SENTENCE, &start_offset,
754+
&end_offset);
755+
EXPECT_NE(sentence_result, nullptr);
756+
EXPECT_GE(start_offset, 0);
757+
EXPECT_LE(end_offset, 5);
758+
}
759+
760+
// Tests that get_string_at_offset handles offset at position zero.
761+
TEST(FlAccessibleTextFieldTest, TextBoundaryOffsetAtStart) {
762+
g_autoptr(FlDartProject) project = fl_dart_project_new();
763+
g_autoptr(FlEngine) engine = fl_engine_new(project);
764+
g_autoptr(FlAccessibleNode) node =
765+
fl_accessible_text_field_new(engine, 123, 1);
766+
767+
fl_accessible_node_set_value(node, "Hello");
768+
769+
// Offset at zero should return the first character/word.
770+
gint start_offset = -1, end_offset = -1;
771+
g_autofree gchar* char_result = atk_text_get_string_at_offset(
772+
ATK_TEXT(node), 0, ATK_TEXT_GRANULARITY_CHAR, &start_offset, &end_offset);
773+
EXPECT_NE(char_result, nullptr);
774+
EXPECT_EQ(start_offset, 0);
775+
EXPECT_EQ(end_offset, 1);
776+
EXPECT_STREQ(char_result, "H");
777+
778+
g_autofree gchar* word_result = atk_text_get_string_at_offset(
779+
ATK_TEXT(node), 0, ATK_TEXT_GRANULARITY_WORD, &start_offset, &end_offset);
780+
EXPECT_NE(word_result, nullptr);
781+
EXPECT_EQ(start_offset, 0);
782+
EXPECT_EQ(end_offset, 5);
783+
EXPECT_STREQ(word_result, "Hello");
784+
}
785+
786+
// Tests that get_string_at_offset handles empty text.
787+
TEST(FlAccessibleTextFieldTest, TextBoundaryEmptyText) {
788+
g_autoptr(FlDartProject) project = fl_dart_project_new();
789+
g_autoptr(FlEngine) engine = fl_engine_new(project);
790+
g_autoptr(FlAccessibleNode) node =
791+
fl_accessible_text_field_new(engine, 123, 1);
792+
793+
// Empty text - should not crash.
794+
gint start_offset = -1, end_offset = -1;
795+
g_autofree gchar* char_result = atk_text_get_string_at_offset(
796+
ATK_TEXT(node), 0, ATK_TEXT_GRANULARITY_CHAR, &start_offset, &end_offset);
797+
EXPECT_NE(char_result, nullptr);
798+
EXPECT_EQ(start_offset, 0);
799+
EXPECT_STREQ(char_result, "");
800+
801+
g_autofree gchar* word_result = atk_text_get_string_at_offset(
802+
ATK_TEXT(node), 0, ATK_TEXT_GRANULARITY_WORD, &start_offset, &end_offset);
803+
EXPECT_NE(word_result, nullptr);
804+
EXPECT_EQ(start_offset, 0);
805+
EXPECT_STREQ(word_result, "");
806+
}
807+
808+
// Tests that get_string_at_offset handles offset at exact text length.
809+
TEST(FlAccessibleTextFieldTest, TextBoundaryOffsetAtEnd) {
810+
g_autoptr(FlDartProject) project = fl_dart_project_new();
811+
g_autoptr(FlEngine) engine = fl_engine_new(project);
812+
g_autoptr(FlAccessibleNode) node =
813+
fl_accessible_text_field_new(engine, 123, 1);
814+
815+
fl_accessible_node_set_value(node, "Hello world");
816+
817+
// Offset at exactly the character count (one past last char).
818+
gint char_count = atk_text_get_character_count(ATK_TEXT(node));
819+
EXPECT_EQ(char_count, 11);
820+
821+
gint start_offset = -1, end_offset = -1;
822+
g_autofree gchar* char_result = atk_text_get_string_at_offset(
823+
ATK_TEXT(node), char_count, ATK_TEXT_GRANULARITY_CHAR, &start_offset,
824+
&end_offset);
825+
EXPECT_NE(char_result, nullptr);
826+
EXPECT_GE(start_offset, 0);
827+
EXPECT_LE(end_offset, char_count);
828+
829+
g_autofree gchar* word_result = atk_text_get_string_at_offset(
830+
ATK_TEXT(node), char_count, ATK_TEXT_GRANULARITY_WORD, &start_offset,
831+
&end_offset);
832+
EXPECT_NE(word_result, nullptr);
833+
EXPECT_GE(start_offset, 0);
834+
EXPECT_LE(end_offset, char_count);
835+
}
836+
699837
// NOLINTEND(clang-analyzer-core.StackAddressEscape)

0 commit comments

Comments
 (0)