This is an automated email from the ASF dual-hosted git repository.
kou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new 39125bac39 GH-50355: [C++][Gandiva] fix out-of-bounds read in
utf8_length_ignore_invalid (#50356)
39125bac39 is described below
commit 39125bac3960fb0a0625d924b5b3fe174974f530
Author: Abdul Rawoof Khan <[email protected]>
AuthorDate: Wed Jul 8 14:06:13 2026 +0530
GH-50355: [C++][Gandiva] fix out-of-bounds read in
utf8_length_ignore_invalid (#50356)
### Rationale for this change
`utf8_length_ignore_invalid` extends `char_len` while scanning the bytes
after a lead byte and never rechecks the buffer end, so an input ending in a
truncated multi-byte utf8 sequence (a `0xF0` lead byte followed by
non-continuation bytes) reads past `data_len`. It is reached from untrusted
string data through `lpad`/`rpad`, which count the input glyphs before padding.
Reproduced against a verbatim copy of the function under AddressSanitizer with
the 4-byte input `{0xF0, 'a', 'a', 'a [...]
### What changes are included in this PR?
Stop the inner scan with a `break` when a byte after the lead byte is not a
continuation byte, instead of incrementing `char_len`. Growing `char_len` on
each stray byte kept extending the loop past `data_len`; breaking leaves
`char_len` bounded so the outer `i + char_len <= data_len` check keeps every
read in range. Valid input counts the same, because a well-formed glyph has
only continuation bytes after its lead byte and never hits the `break`.
### Are these changes tested?
Yes. Added `TestStringOps.TestPadMalformedUtf8NoOverread`, which runs
`lpad`/`rpad` on the truncated multi-byte input placed in an exactly-sized heap
buffer so the over-read trips ASAN, and asserts the full padded output. The
existing pad tests still pass.
### Are there any user-facing changes?
No.
**This PR contains a "Critical Fix".** It fixes an out-of-bounds read in
the Gandiva utf8 length helper reachable from `lpad`/`rpad` on crafted string
data.
* GitHub Issue: #50355
Authored-by: abdul rawoof <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
---
cpp/src/gandiva/precompiled/string_ops.cc | 7 +++-
cpp/src/gandiva/precompiled/string_ops_test.cc | 56 ++++++++++++++++++++++++++
2 files changed, 62 insertions(+), 1 deletion(-)
diff --git a/cpp/src/gandiva/precompiled/string_ops.cc
b/cpp/src/gandiva/precompiled/string_ops.cc
index 70cad7c626..9d771758bb 100644
--- a/cpp/src/gandiva/precompiled/string_ops.cc
+++ b/cpp/src/gandiva/precompiled/string_ops.cc
@@ -208,7 +208,12 @@ gdv_int32 utf8_length_ignore_invalid(const char* data,
gdv_int32 data_len) {
}
for (int j = 1; j < char_len; ++j) {
if ((data[i + j] & 0xC0) != 0x80) { // bytes following head-byte of
glyph
- char_len += 1;
+ // Only the bytes up to the mismatch belong to this (invalid) glyph, so
+ // advance past them and let the outer loop re-parse the rest. Keeping
+ // char_len at its declared width would swallow valid characters that
+ // fall inside the truncated sequence's window.
+ char_len = j;
+ break;
}
}
++count;
diff --git a/cpp/src/gandiva/precompiled/string_ops_test.cc
b/cpp/src/gandiva/precompiled/string_ops_test.cc
index c866b88540..3a16e4076b 100644
--- a/cpp/src/gandiva/precompiled/string_ops_test.cc
+++ b/cpp/src/gandiva/precompiled/string_ops_test.cc
@@ -18,7 +18,9 @@
#include <gmock/gmock.h>
#include <gtest/gtest.h>
+#include <cstring>
#include <limits>
+#include <memory>
#include "gandiva/execution_context.h"
#include "gandiva/precompiled/types.h"
@@ -1608,6 +1610,60 @@ TEST(TestStringOps, TestRpadString) {
EXPECT_EQ(std::string(out_str + 5000, 2), "α");
}
+TEST(TestStringOps, TestPadMalformedUtf8NoOverread) {
+ gandiva::ExecutionContext ctx;
+ uint64_t ctx_ptr = reinterpret_cast<gdv_int64>(&ctx);
+ gdv_int32 out_len = 0;
+
+ // A 4-byte utf8 lead byte followed by non-continuation bytes and no trailing
+ // space. utf8_length_ignore_invalid() used to extend the glyph length past
+ // the end of the buffer while scanning the continuation bytes. The input is
+ // held in an exactly-sized heap buffer so any over-read trips
AddressSanitizer.
+ const char bytes[] = {'\xF0', 'a', 'a', 'a'};
+ const auto text_len = static_cast<gdv_int32>(sizeof(bytes));
+ std::unique_ptr<char[]> text(new char[text_len]);
+ std::memcpy(text.get(), bytes, text_len);
+ const std::string text_str(text.get(), text_len);
+
+ // The lone lead byte counts as one invalid glyph and the three 'a's as one
+ // each, so the length is 4 and padding to width 6 adds two fill characters.
+ const char* out_str =
+ lpad_utf8_int32_utf8(ctx_ptr, text.get(), text_len, 6, " ", 1, &out_len);
+ EXPECT_EQ(out_len, 6);
+ EXPECT_EQ(std::string(out_str, out_len), " " + text_str);
+
+ out_str = rpad_utf8_int32_utf8(ctx_ptr, text.get(), text_len, 6, " ", 1,
&out_len);
+ EXPECT_EQ(out_len, 6);
+ EXPECT_EQ(std::string(out_str, out_len), text_str + " ");
+}
+
+TEST(TestStringOps, TestPadMalformedUtf8KeepsValidGlyph) {
+ gandiva::ExecutionContext ctx;
+ uint64_t ctx_ptr = reinterpret_cast<gdv_int64>(&ctx);
+ gdv_int32 out_len = 0;
+
+ // {0xF0, 'a', 0xE2, 0x82, 0xAC}: malformed 4-byte lead + ASCII 'a' + U+20AC
€.
+ // 0xF0 alone counts as one invalid glyph, then 'a' and € follow on their
own,
+ // so the count is 3. If the inner scan kept char_len at 4 it would advance
the
+ // outer loop past 'a', 0xE2, 0x82 and only see the orphaned 0xAC, giving 2.
+ // The input sits in an exactly-sized heap buffer so any over-read trips
ASAN.
+ const char bytes[] = {'\xF0', 'a', '\xE2', '\x82', '\xAC'};
+ const auto text_len = static_cast<gdv_int32>(sizeof(bytes));
+ std::unique_ptr<char[]> text(new char[text_len]);
+ std::memcpy(text.get(), bytes, text_len);
+ const std::string text_str(text.get(), text_len);
+
+ // 3 glyphs padded to width 5 adds two fill characters, out_len = 2 + 5 = 7.
+ const char* out_str =
+ lpad_utf8_int32_utf8(ctx_ptr, text.get(), text_len, 5, " ", 1, &out_len);
+ EXPECT_EQ(out_len, 7);
+ EXPECT_EQ(std::string(out_str, out_len), " " + text_str);
+
+ out_str = rpad_utf8_int32_utf8(ctx_ptr, text.get(), text_len, 5, " ", 1,
&out_len);
+ EXPECT_EQ(out_len, 7);
+ EXPECT_EQ(std::string(out_str, out_len), text_str + " ");
+}
+
TEST(TestStringOps, TestRtrim) {
gandiva::ExecutionContext ctx;
uint64_t ctx_ptr = reinterpret_cast<gdv_int64>(&ctx);
