Skip to content

Commit 59ce5ba

Browse files
Mitigates a critical Out-of-Bounds (OOB) read vulnerability by transitioning internal trie references to absl::Span and validating array lengths before dynamic offset queries. Added explicit boundary safety unit tests to verify mitigation.
PiperOrigin-RevId: 914999991
1 parent 72a28e1 commit 59ce5ba

8 files changed

Lines changed: 64 additions & 22 deletions

tensorflow_text/core/kernels/BUILD

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,7 @@ tf_cc_library(
214214
"@com_google_absl//absl/memory",
215215
"@com_google_absl//absl/status",
216216
"@com_google_absl//absl/strings",
217+
"@com_google_absl//absl/types:span",
217218
"@icu//:common",
218219
# lite/kernels/shim:status_macros tensorflow dep,
219220
],
@@ -350,7 +351,10 @@ cc_library(
350351
"darts_clone_trie_wrapper.h",
351352
],
352353
deps = [
354+
"@com_google_absl//absl/base:core_headers",
355+
"@com_google_absl//absl/status",
353356
"@com_google_absl//absl/status:statusor",
357+
"@com_google_absl//absl/types:span",
354358
],
355359
)
356360

@@ -362,6 +366,7 @@ cc_test(
362366
":darts_clone_trie_builder",
363367
":darts_clone_trie_wrapper",
364368
"@com_google_absl//absl/status",
369+
"@com_google_absl//absl/types:span",
365370
"@com_google_googletest//:gtest_main",
366371
],
367372
)
@@ -398,6 +403,7 @@ tf_cc_library(
398403
"@com_google_absl//absl/status",
399404
"@com_google_absl//absl/status:statusor",
400405
"@com_google_absl//absl/strings",
406+
"@com_google_absl//absl/types:span",
401407
"@icu//:nfkc",
402408
# lite/kernels/shim:status_macros tensorflow dep,
403409
],

tensorflow_text/core/kernels/darts_clone_trie_test.cc

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
#include <gmock/gmock.h>
1616
#include <gtest/gtest.h>
17+
#include "absl/types/span.h"
1718
#include "tensorflow_text/core/kernels/darts_clone_trie_builder.h"
1819
#include "tensorflow_text/core/kernels/darts_clone_trie_wrapper.h"
1920

@@ -31,7 +32,7 @@ TEST(DartsCloneTrieTest, CreateCursorPointToRootAndTryTraverseOneStep) {
3132
ASSERT_OK_AND_ASSIGN(std::vector<uint32_t> trie_array,
3233
BuildDartsCloneTrie(vocab_tokens));
3334
ASSERT_OK_AND_ASSIGN(DartsCloneTrieWrapper trie,
34-
DartsCloneTrieWrapper::Create(trie_array.data()));
35+
DartsCloneTrieWrapper::Create(trie_array));
3536

3637
DartsCloneTrieWrapper::TraversalCursor cursor;
3738
int data;
@@ -56,7 +57,7 @@ TEST(DartsCloneTrieTest, CreateCursorAndTryTraverseSeveralSteps) {
5657
ASSERT_OK_AND_ASSIGN(std::vector<uint32_t> trie_array,
5758
BuildDartsCloneTrie(vocab_tokens));
5859
ASSERT_OK_AND_ASSIGN(DartsCloneTrieWrapper trie,
59-
DartsCloneTrieWrapper::Create(trie_array.data()));
60+
DartsCloneTrieWrapper::Create(trie_array));
6061

6162
DartsCloneTrieWrapper::TraversalCursor cursor;
6263
int data;
@@ -76,7 +77,7 @@ TEST(DartsCloneTrieTest, TraversePathNotExisted) {
7677
ASSERT_OK_AND_ASSIGN(std::vector<uint32_t> trie_array,
7778
BuildDartsCloneTrie(vocab_tokens));
7879
ASSERT_OK_AND_ASSIGN(DartsCloneTrieWrapper trie,
79-
DartsCloneTrieWrapper::Create(trie_array.data()));
80+
DartsCloneTrieWrapper::Create(trie_array));
8081

8182
DartsCloneTrieWrapper::TraversalCursor cursor;
8283

@@ -94,7 +95,7 @@ TEST(DartsCloneTrieTest, TraverseOnUtf8Path) {
9495
ASSERT_OK_AND_ASSIGN(std::vector<uint32_t> trie_array,
9596
BuildDartsCloneTrie(vocab_tokens));
9697
ASSERT_OK_AND_ASSIGN(DartsCloneTrieWrapper trie,
97-
DartsCloneTrieWrapper::Create(trie_array.data()));
98+
DartsCloneTrieWrapper::Create(trie_array));
9899

99100
DartsCloneTrieWrapper::TraversalCursor cursor;
100101
int data;
@@ -115,7 +116,7 @@ TEST(DartsCloneTrieTest, TraverseOnPartialUtf8Path) {
115116
ASSERT_OK_AND_ASSIGN(std::vector<uint32_t> trie_array,
116117
BuildDartsCloneTrie(vocab_tokens));
117118
ASSERT_OK_AND_ASSIGN(DartsCloneTrieWrapper trie,
118-
DartsCloneTrieWrapper::Create(trie_array.data()));
119+
DartsCloneTrieWrapper::Create(trie_array));
119120

120121
DartsCloneTrieWrapper::TraversalCursor cursor;
121122
int data;
@@ -135,7 +136,7 @@ TEST(DartsCloneTrieTest, TraverseOnUtf8PathNotExisted) {
135136
ASSERT_OK_AND_ASSIGN(std::vector<uint32_t> trie_array,
136137
BuildDartsCloneTrie(vocab_tokens));
137138
ASSERT_OK_AND_ASSIGN(DartsCloneTrieWrapper trie,
138-
DartsCloneTrieWrapper::Create(trie_array.data()));
139+
DartsCloneTrieWrapper::Create(trie_array));
139140

140141
DartsCloneTrieWrapper::TraversalCursor cursor;
141142

@@ -183,6 +184,20 @@ TEST(DartsCloneTrieBuildError, NegativeValues) {
183184
StatusIs(util::error::INVALID_ARGUMENT));
184185
}
185186

187+
TEST(DartsCloneTrieTest, OutOfBoundsAccessIsRejected) {
188+
std::vector<std::string> vocab_tokens{"def", "\xe1\xb8\x8aZZ", "Abc"};
189+
ASSERT_OK_AND_ASSIGN(std::vector<uint32_t> trie_array,
190+
BuildDartsCloneTrie(vocab_tokens));
191+
// Wrap using a constrained span to emulate an out-of-bounds access attempts.
192+
auto span = absl::MakeSpan(trie_array.data(), 1);
193+
ASSERT_OK_AND_ASSIGN(DartsCloneTrieWrapper trie,
194+
DartsCloneTrieWrapper::Create(span));
195+
196+
DartsCloneTrieWrapper::TraversalCursor cursor =
197+
trie.CreateTraversalCursorPointToRoot();
198+
EXPECT_FALSE(trie.TryTraverseOneStep(cursor, 'd'));
199+
}
200+
186201
} // namespace trie_utils
187202
} // namespace text
188203
} // namespace tensorflow

tensorflow_text/core/kernels/darts_clone_trie_wrapper.h

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@
3030
#include <stdint.h>
3131
#include <string.h>
3232

33+
#include "absl/status/status.h"
3334
#include "absl/status/statusor.h"
35+
#include "absl/types/span.h"
3436

3537
namespace tensorflow {
3638
namespace text {
@@ -51,14 +53,14 @@ class DartsCloneTrieWrapper {
5153
uint32_t unit = 0;
5254
};
5355

54-
// Constructs an instance by passing in the pointer to the trie array data.
56+
// Constructs an instance by passing in the span of the trie array data.
5557
// The caller needs to make sure that 'trie_array' points to a valid structure
5658
// returned by darts_clone trie builder. The caller also needs to maintain the
5759
// availability of 'trie_array' throughout the lifetime of this instance.
5860
static absl::StatusOr<DartsCloneTrieWrapper> Create(
59-
const uint32_t* trie_array) {
60-
if (trie_array == nullptr) {
61-
return absl::InvalidArgumentError("trie_array is nullptr.");
61+
absl::Span<const uint32_t> trie_array) {
62+
if (trie_array.empty() || trie_array.data() == nullptr) {
63+
return absl::InvalidArgumentError("trie_array is empty or nullptr.");
6264
}
6365
return DartsCloneTrieWrapper(trie_array);
6466
}
@@ -70,20 +72,28 @@ class DartsCloneTrieWrapper {
7072

7173
// Creates a cursor pointing to the 'node_id'.
7274
TraversalCursor CreateTraversalCursor(uint32_t node_id) {
75+
if (node_id >= trie_array_.size()) {
76+
return {0, 0};
77+
}
7378
return {node_id, trie_array_[node_id]};
7479
}
7580

7681
// Sets the cursor to point to 'node_id'.
7782
void SetTraversalCursor(TraversalCursor& cursor, uint32_t node_id) {
78-
cursor.node_id = node_id;
79-
cursor.unit = trie_array_[node_id];
83+
if (node_id < trie_array_.size()) {
84+
cursor.node_id = node_id;
85+
cursor.unit = trie_array_[node_id];
86+
}
8087
}
8188

8289
// Traverses one step from 'cursor' following 'ch'. If successful (i.e., there
8390
// exists such an edge), moves 'cursor' to the new node and returns true.
8491
// Otherwise, does nothing (i.e., 'cursor' is not changed) and returns false.
8592
bool TryTraverseOneStep(TraversalCursor& cursor, unsigned char ch) const {
8693
const uint32_t next_node_id = cursor.node_id ^ offset(cursor.unit) ^ ch;
94+
if (next_node_id >= trie_array_.size()) {
95+
return false;
96+
}
8797
const uint32_t next_node_unit = trie_array_[next_node_id];
8898
if (label(next_node_unit) != ch) {
8999
return false;
@@ -108,15 +118,18 @@ class DartsCloneTrieWrapper {
108118
if (!has_leaf(cursor.unit)) {
109119
return false;
110120
}
111-
const uint32_t value_unit =
112-
trie_array_[cursor.node_id ^ offset(cursor.unit)];
121+
const uint32_t value_node_id = cursor.node_id ^ offset(cursor.unit);
122+
if (value_node_id >= trie_array_.size()) {
123+
return false;
124+
}
125+
const uint32_t value_unit = trie_array_[value_node_id];
113126
out_data = value(value_unit);
114127
return true;
115128
}
116129

117130
private:
118131
// Use Create() instead of the constructor.
119-
explicit DartsCloneTrieWrapper(const uint32_t* trie_array)
132+
explicit DartsCloneTrieWrapper(absl::Span<const uint32_t> trie_array)
120133
: trie_array_(trie_array) {}
121134

122135
// The actual implementation of TryTraverseSeveralSteps.
@@ -127,6 +140,9 @@ class DartsCloneTrieWrapper {
127140
for (; size > 0; --size, ++ptr) {
128141
const unsigned char ch = static_cast<const unsigned char>(*ptr);
129142
cur_id ^= offset(cur_unit) ^ ch;
143+
if (cur_id >= trie_array_.size()) {
144+
return false;
145+
}
130146
cur_unit = trie_array_[cur_id];
131147
if (label(cur_unit) != ch) {
132148
return false;
@@ -157,8 +173,8 @@ class DartsCloneTrieWrapper {
157173
return static_cast<int>(unit & 0x7fffffff);
158174
}
159175

160-
// The pointer to the darts trie array.
161-
const uint32_t* trie_array_;
176+
// The dart trie array represented as a span for bounds awareness.
177+
absl::Span<const uint32_t> trie_array_;
162178
};
163179

164180
} // namespace trie_utils

tensorflow_text/core/kernels/fast_bert_normalizer.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include <vector>
2020

2121
#include "absl/strings/string_view.h"
22+
#include "absl/types/span.h"
2223
#include "icu4c/source/common/unicode/utf8.h"
2324
#include "tensorflow/lite/kernels/shim/status_macros.h"
2425
#include "tensorflow_text/core/kernels/darts_clone_trie_wrapper.h"
@@ -82,7 +83,7 @@ class FastBertNormalizer {
8283
// which is not owned by this instance and should be kept alive through the
8384
// lifetime of the instance.
8485
static absl::StatusOr<FastBertNormalizer> Create(
85-
const uint32_t* trie_data, int data_for_codepoint_zero,
86+
absl::Span<const uint32_t> trie_data, int data_for_codepoint_zero,
8687
const char* normalized_string_pool) {
8788
FastBertNormalizer result;
8889
SH_ASSIGN_OR_RETURN(auto trie,
@@ -106,7 +107,9 @@ class FastBertNormalizer {
106107
// `GetFastBertNormalizerModel()` is autogenerated by flatbuffer.
107108
auto model = GetFastBertNormalizerModel(model_flatbuffer);
108109
return Create(
109-
model->trie_array()->data(), model->data_for_codepoint_zero(),
110+
absl::MakeSpan(model->trie_array()->data(),
111+
model->trie_array()->size()),
112+
model->data_for_codepoint_zero(),
110113
reinterpret_cast<const char*>(model->normalized_string_pool()->data()));
111114
}
112115

tensorflow_text/core/kernels/fast_bert_normalizer_model_builder.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ FastBertNormalizerFactory::FastBertNormalizerFactory(
229229
return;
230230
}
231231
auto char_set_recognizer_mapper = FastBertNormalizer::Create(
232-
trie_data_.data(), data_for_codepoint_zero_, mapped_value_pool_.data());
232+
trie_data_, data_for_codepoint_zero_, mapped_value_pool_.data());
233233
if (!char_set_recognizer_mapper.ok()) {
234234
// Should never happen since the same code must have passed the unit tests.
235235
LOG(ERROR) << "Unexpected error: Failed to initialize "

tensorflow_text/core/kernels/fast_wordpiece_tokenizer.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "absl/strings/match.h"
2323
#include "absl/strings/str_join.h"
2424
#include "absl/strings/string_view.h"
25+
#include "absl/types/span.h"
2526
#include "icu4c/source/common/unicode/uchar.h"
2627
#include "icu4c/source/common/unicode/utf8.h"
2728
#include "tensorflow/lite/kernels/shim/status_macros.h"
@@ -49,7 +50,8 @@ FastWordpieceTokenizer::Create(const void* config_flatbuffer) {
4950
// `GetFastWordpieceTokenizerConfig()` is autogenerated by flatbuffer.
5051
tokenizer.config_ = GetFastWordpieceTokenizerConfig(config_flatbuffer);
5152
auto trie_or = trie_utils::DartsCloneTrieWrapper::Create(
52-
tokenizer.config_->trie_array()->data());
53+
absl::MakeSpan(tokenizer.config_->trie_array()->data(),
54+
tokenizer.config_->trie_array()->size()));
5355
if (!trie_or.ok()) {
5456
return absl::InvalidArgumentError(
5557
"Failed to create DartsCloneTrieWrapper from "

tensorflow_text/core/kernels/fast_wordpiece_tokenizer_model_builder.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,7 @@ absl::Status FastWordpieceBuilder::ConstructTrie(
434434
trie_utils::BuildDartsCloneTrie(keys, values));
435435
SH_ASSIGN_OR_RETURN(
436436
trie_utils::DartsCloneTrieWrapper trie,
437-
trie_utils::DartsCloneTrieWrapper::Create(trie_array_.data()));
437+
trie_utils::DartsCloneTrieWrapper::Create(trie_array_));
438438
trie_.emplace(std::move(trie));
439439

440440
if (trie_array_.size() >
Binary file not shown.

0 commit comments

Comments
 (0)