Skip to content

Commit 1f3f40f

Browse files
authored
GH-50462: [C++][Gandiva] fix out-of-bounds read in translate_utf8_utf8_utf8 (#50463)
### Rationale for this change The multi-byte branch of `translate_utf8_utf8_utf8` (taken whenever the input, FROM, or TO contains a byte > 127) reads `gdv_fn_utf8_char_length` on a lead byte and then builds a `std::string` of that width from the buffer without checking how many bytes remain. A truncated trailing glyph in the input reads past IN, a multi-byte input character missing from FROM reads one byte past FROM at the end-of-list sentinel (the `from_for == from_len` check ran after the read), and a truncated glyph in TO reads past TO. All three are reachable from the SQL `translate(in, from, to)` call on truncated column data. ### What changes are included in this PR? Clamp each utf8 character width to the bytes left in its buffer before copying, consuming a single byte on a truncated or invalid lead byte (which also stops a zero-width advance from looping forever), and move the FROM end-of-list check ahead of the read so the sentinel iteration no longer touches `from[from_len]`. Valid input keeps its existing grouping since the clamp only fires past the end. ### Are these changes tested? Yes. Added two cases to `TestGdvFnStubs.TestTranslate`: a truncated trailing glyph in the input, and a valid multi-byte input char absent from FROM. Both use exact-sized buffers so ASAN trips on the pre-patch over-read and passes after the fix. ### Are there any user-facing changes? No. **This PR contains a "Critical Fix".** It fixes a heap out-of-bounds read (crash) in `translate` reachable from user-supplied string data. * GitHub Issue: #50462 Authored-by: abdul rawoof <abdulr@bugqore.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
1 parent af1c67e commit 1f3f40f

2 files changed

Lines changed: 41 additions & 4 deletions

File tree

cpp/src/gandiva/gdv_function_stubs_test.cc

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1202,6 +1202,29 @@ TEST(TestGdvFnStubs, TestTranslate) {
12021202
EXPECT_STREQ(result, "");
12031203
EXPECT_THAT(ctx.get_error(),
12041204
::testing::HasSubstr("Would overflow maximum output size"));
1205+
1206+
// A byte > 127 selects the multi-byte path. A truncated trailing glyph (0xE2
1207+
// claims a 3-byte character but only one byte is present) must not be read past
1208+
// the end of the input; it is passed through as a single byte. Exact-sized
1209+
// buffers let ASAN catch any over-read here.
1210+
const char truncated_in[] = {'a', static_cast<char>(0xE2)};
1211+
result = translate_utf8_utf8_utf8(ctx_ptr, truncated_in, 2, "x", 1, "y", 1, &out_len);
1212+
EXPECT_EQ(std::string(truncated_in, 2), std::string(result, out_len));
1213+
1214+
// A valid multi-byte input character absent from FROM previously over-read FROM
1215+
// by one byte at the end-of-list sentinel.
1216+
const char euro[] = {static_cast<char>(0xE2), static_cast<char>(0x82),
1217+
static_cast<char>(0xAC)};
1218+
const char from_one[] = {'a'};
1219+
const char to_one[] = {'b'};
1220+
result = translate_utf8_utf8_utf8(ctx_ptr, euro, 3, from_one, 1, to_one, 1, &out_len);
1221+
EXPECT_EQ(std::string(euro, 3), std::string(result, out_len));
1222+
1223+
// A truncated trailing glyph in TO (0xE2 claims a 3-byte character but only one
1224+
// byte is present) previously over-read TO when a matched input char mapped to it.
1225+
const char trunc_to[] = {static_cast<char>(0xE2)};
1226+
result = translate_utf8_utf8_utf8(ctx_ptr, "a", 1, "a", 1, trunc_to, 1, &out_len);
1227+
EXPECT_EQ(std::string(trunc_to, 1), std::string(result, out_len));
12051228
}
12061229

12071230
TEST(TestGdvFnStubs, TestToUtcTimezone) {

cpp/src/gandiva/gdv_string_function_stubs.cc

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -779,6 +779,11 @@ const char* translate_utf8_utf8_utf8(int64_t context, const char* in, int32_t in
779779
for (int32_t in_for = 0; in_for < in_len; in_for += len_char_in) {
780780
// Updating len to char in this position
781781
len_char_in = gdv_fn_utf8_char_length(in[in_for]);
782+
// A truncated or invalid lead byte at the tail would make the copy below read
783+
// past IN, and a zero width would spin the loop forever; consume one byte.
784+
if (len_char_in == 0 || in_for + len_char_in > in_len) {
785+
len_char_in = 1;
786+
}
782787
// Making copy to std::string with length for this char position
783788
std::string insert_copy_key(in + in_for, len_char_in);
784789
if (subs_list.find(insert_copy_key) != subs_list.end()) {
@@ -790,10 +795,6 @@ const char* translate_utf8_utf8_utf8(int64_t context, const char* in, int32_t in
790795
}
791796
} else {
792797
for (int32_t from_for = 0; from_for <= from_len; from_for += len_char_from) {
793-
// Updating len to char in this position
794-
len_char_from = gdv_fn_utf8_char_length(from[from_for]);
795-
// Making copy to std::string with length for this char position
796-
std::string copy_from_compare(from + from_for, len_char_from);
797798
if (from_for == from_len) {
798799
// If it's not in the FROM list, just add it to the map and the result.
799800
std::string insert_copy_value(in + in_for, len_char_in);
@@ -806,6 +807,15 @@ const char* translate_utf8_utf8_utf8(int64_t context, const char* in, int32_t in
806807
break;
807808
}
808809

810+
// Updating len to char in this position
811+
len_char_from = gdv_fn_utf8_char_length(from[from_for]);
812+
// Clamp a truncated or invalid lead byte to the remaining bytes so the copy
813+
// below never reads past FROM and the loop always advances.
814+
if (len_char_from == 0 || from_for + len_char_from > from_len) {
815+
len_char_from = 1;
816+
}
817+
// Making copy to std::string with length for this char position
818+
std::string copy_from_compare(from + from_for, len_char_from);
809819
if (insert_copy_key != copy_from_compare) {
810820
// If this character does not exist in FROM list, don't need treatment
811821
continue;
@@ -818,6 +828,10 @@ const char* translate_utf8_utf8_utf8(int64_t context, const char* in, int32_t in
818828
// If exist and the start_compare is in range, add to map with the
819829
// corresponding TO in position start_compare
820830
len_char_to = gdv_fn_utf8_char_length(to[start_compare]);
831+
// Clamp a truncated or invalid lead byte to the remaining bytes of TO.
832+
if (len_char_to == 0 || start_compare + len_char_to > to_len) {
833+
len_char_to = 1;
834+
}
821835
std::string insert_copy_value(to + start_compare, len_char_to);
822836
// Insert in map to next loops
823837
subs_list.insert(

0 commit comments

Comments
 (0)