Skip to content

Commit 73a0f34

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: 914914973
1 parent aac3cce commit 73a0f34

8 files changed

Lines changed: 22 additions & 64 deletions

tensorflow_text/core/kernels/BUILD

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,6 @@ 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",
218217
"@icu//:common",
219218
# lite/kernels/shim:status_macros tensorflow dep,
220219
],
@@ -351,10 +350,7 @@ cc_library(
351350
"darts_clone_trie_wrapper.h",
352351
],
353352
deps = [
354-
"@com_google_absl//absl/base:core_headers",
355-
"@com_google_absl//absl/status",
356353
"@com_google_absl//absl/status:statusor",
357-
"@com_google_absl//absl/types:span",
358354
],
359355
)
360356

@@ -366,7 +362,6 @@ cc_test(
366362
":darts_clone_trie_builder",
367363
":darts_clone_trie_wrapper",
368364
"@com_google_absl//absl/status",
369-
"@com_google_absl//absl/types:span",
370365
"@com_google_googletest//:gtest_main",
371366
],
372367
)
@@ -403,7 +398,6 @@ tf_cc_library(
403398
"@com_google_absl//absl/status",
404399
"@com_google_absl//absl/status:statusor",
405400
"@com_google_absl//absl/strings",
406-
"@com_google_absl//absl/types:span",
407401
"@icu//:nfkc",
408402
# lite/kernels/shim:status_macros tensorflow dep,
409403
],

tensorflow_text/core/kernels/darts_clone_trie_test.cc

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

1515
#include <gmock/gmock.h>
1616
#include <gtest/gtest.h>
17-
#include "absl/types/span.h"
1817
#include "tensorflow_text/core/kernels/darts_clone_trie_builder.h"
1918
#include "tensorflow_text/core/kernels/darts_clone_trie_wrapper.h"
2019

@@ -32,7 +31,7 @@ TEST(DartsCloneTrieTest, CreateCursorPointToRootAndTryTraverseOneStep) {
3231
ASSERT_OK_AND_ASSIGN(std::vector<uint32_t> trie_array,
3332
BuildDartsCloneTrie(vocab_tokens));
3433
ASSERT_OK_AND_ASSIGN(DartsCloneTrieWrapper trie,
35-
DartsCloneTrieWrapper::Create(trie_array));
34+
DartsCloneTrieWrapper::Create(trie_array.data()));
3635

3736
DartsCloneTrieWrapper::TraversalCursor cursor;
3837
int data;
@@ -57,7 +56,7 @@ TEST(DartsCloneTrieTest, CreateCursorAndTryTraverseSeveralSteps) {
5756
ASSERT_OK_AND_ASSIGN(std::vector<uint32_t> trie_array,
5857
BuildDartsCloneTrie(vocab_tokens));
5958
ASSERT_OK_AND_ASSIGN(DartsCloneTrieWrapper trie,
60-
DartsCloneTrieWrapper::Create(trie_array));
59+
DartsCloneTrieWrapper::Create(trie_array.data()));
6160

6261
DartsCloneTrieWrapper::TraversalCursor cursor;
6362
int data;
@@ -77,7 +76,7 @@ TEST(DartsCloneTrieTest, TraversePathNotExisted) {
7776
ASSERT_OK_AND_ASSIGN(std::vector<uint32_t> trie_array,
7877
BuildDartsCloneTrie(vocab_tokens));
7978
ASSERT_OK_AND_ASSIGN(DartsCloneTrieWrapper trie,
80-
DartsCloneTrieWrapper::Create(trie_array));
79+
DartsCloneTrieWrapper::Create(trie_array.data()));
8180

8281
DartsCloneTrieWrapper::TraversalCursor cursor;
8382

@@ -95,7 +94,7 @@ TEST(DartsCloneTrieTest, TraverseOnUtf8Path) {
9594
ASSERT_OK_AND_ASSIGN(std::vector<uint32_t> trie_array,
9695
BuildDartsCloneTrie(vocab_tokens));
9796
ASSERT_OK_AND_ASSIGN(DartsCloneTrieWrapper trie,
98-
DartsCloneTrieWrapper::Create(trie_array));
97+
DartsCloneTrieWrapper::Create(trie_array.data()));
9998

10099
DartsCloneTrieWrapper::TraversalCursor cursor;
101100
int data;
@@ -116,7 +115,7 @@ TEST(DartsCloneTrieTest, TraverseOnPartialUtf8Path) {
116115
ASSERT_OK_AND_ASSIGN(std::vector<uint32_t> trie_array,
117116
BuildDartsCloneTrie(vocab_tokens));
118117
ASSERT_OK_AND_ASSIGN(DartsCloneTrieWrapper trie,
119-
DartsCloneTrieWrapper::Create(trie_array));
118+
DartsCloneTrieWrapper::Create(trie_array.data()));
120119

121120
DartsCloneTrieWrapper::TraversalCursor cursor;
122121
int data;
@@ -136,7 +135,7 @@ TEST(DartsCloneTrieTest, TraverseOnUtf8PathNotExisted) {
136135
ASSERT_OK_AND_ASSIGN(std::vector<uint32_t> trie_array,
137136
BuildDartsCloneTrie(vocab_tokens));
138137
ASSERT_OK_AND_ASSIGN(DartsCloneTrieWrapper trie,
139-
DartsCloneTrieWrapper::Create(trie_array));
138+
DartsCloneTrieWrapper::Create(trie_array.data()));
140139

141140
DartsCloneTrieWrapper::TraversalCursor cursor;
142141

@@ -184,20 +183,6 @@ TEST(DartsCloneTrieBuildError, NegativeValues) {
184183
StatusIs(util::error::INVALID_ARGUMENT));
185184
}
186185

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-
201186
} // namespace trie_utils
202187
} // namespace text
203188
} // namespace tensorflow

tensorflow_text/core/kernels/darts_clone_trie_wrapper.h

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

33-
#include "absl/status/status.h"
3433
#include "absl/status/statusor.h"
35-
#include "absl/types/span.h"
3634

3735
namespace tensorflow {
3836
namespace text {
@@ -53,14 +51,14 @@ class DartsCloneTrieWrapper {
5351
uint32_t unit = 0;
5452
};
5553

56-
// Constructs an instance by passing in the span of the trie array data.
54+
// Constructs an instance by passing in the pointer to the trie array data.
5755
// The caller needs to make sure that 'trie_array' points to a valid structure
5856
// returned by darts_clone trie builder. The caller also needs to maintain the
5957
// availability of 'trie_array' throughout the lifetime of this instance.
6058
static absl::StatusOr<DartsCloneTrieWrapper> Create(
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.");
59+
const uint32_t* trie_array) {
60+
if (trie_array == nullptr) {
61+
return absl::InvalidArgumentError("trie_array is nullptr.");
6462
}
6563
return DartsCloneTrieWrapper(trie_array);
6664
}
@@ -72,28 +70,20 @@ class DartsCloneTrieWrapper {
7270

7371
// Creates a cursor pointing to the 'node_id'.
7472
TraversalCursor CreateTraversalCursor(uint32_t node_id) {
75-
if (node_id >= trie_array_.size()) {
76-
return {0, 0};
77-
}
7873
return {node_id, trie_array_[node_id]};
7974
}
8075

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

8982
// Traverses one step from 'cursor' following 'ch'. If successful (i.e., there
9083
// exists such an edge), moves 'cursor' to the new node and returns true.
9184
// Otherwise, does nothing (i.e., 'cursor' is not changed) and returns false.
9285
bool TryTraverseOneStep(TraversalCursor& cursor, unsigned char ch) const {
9386
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-
}
9787
const uint32_t next_node_unit = trie_array_[next_node_id];
9888
if (label(next_node_unit) != ch) {
9989
return false;
@@ -118,18 +108,15 @@ class DartsCloneTrieWrapper {
118108
if (!has_leaf(cursor.unit)) {
119109
return false;
120110
}
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];
111+
const uint32_t value_unit =
112+
trie_array_[cursor.node_id ^ offset(cursor.unit)];
126113
out_data = value(value_unit);
127114
return true;
128115
}
129116

130117
private:
131118
// Use Create() instead of the constructor.
132-
explicit DartsCloneTrieWrapper(absl::Span<const uint32_t> trie_array)
119+
explicit DartsCloneTrieWrapper(const uint32_t* trie_array)
133120
: trie_array_(trie_array) {}
134121

135122
// The actual implementation of TryTraverseSeveralSteps.
@@ -140,9 +127,6 @@ class DartsCloneTrieWrapper {
140127
for (; size > 0; --size, ++ptr) {
141128
const unsigned char ch = static_cast<const unsigned char>(*ptr);
142129
cur_id ^= offset(cur_unit) ^ ch;
143-
if (cur_id >= trie_array_.size()) {
144-
return false;
145-
}
146130
cur_unit = trie_array_[cur_id];
147131
if (label(cur_unit) != ch) {
148132
return false;
@@ -173,8 +157,8 @@ class DartsCloneTrieWrapper {
173157
return static_cast<int>(unit & 0x7fffffff);
174158
}
175159

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

180164
} // namespace trie_utils

tensorflow_text/core/kernels/fast_bert_normalizer.h

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

2121
#include "absl/strings/string_view.h"
22-
#include "absl/types/span.h"
2322
#include "icu4c/source/common/unicode/utf8.h"
2423
#include "tensorflow/lite/kernels/shim/status_macros.h"
2524
#include "tensorflow_text/core/kernels/darts_clone_trie_wrapper.h"
@@ -83,7 +82,7 @@ class FastBertNormalizer {
8382
// which is not owned by this instance and should be kept alive through the
8483
// lifetime of the instance.
8584
static absl::StatusOr<FastBertNormalizer> Create(
86-
absl::Span<const uint32_t> trie_data, int data_for_codepoint_zero,
85+
const uint32_t* trie_data, int data_for_codepoint_zero,
8786
const char* normalized_string_pool) {
8887
FastBertNormalizer result;
8988
SH_ASSIGN_OR_RETURN(auto trie,
@@ -107,9 +106,7 @@ class FastBertNormalizer {
107106
// `GetFastBertNormalizerModel()` is autogenerated by flatbuffer.
108107
auto model = GetFastBertNormalizerModel(model_flatbuffer);
109108
return Create(
110-
absl::MakeSpan(model->trie_array()->data(),
111-
model->trie_array()->size()),
112-
model->data_for_codepoint_zero(),
109+
model->trie_array()->data(), model->data_for_codepoint_zero(),
113110
reinterpret_cast<const char*>(model->normalized_string_pool()->data()));
114111
}
115112

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_for_codepoint_zero_, mapped_value_pool_.data());
232+
trie_data_.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: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
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"
2625
#include "icu4c/source/common/unicode/uchar.h"
2726
#include "icu4c/source/common/unicode/utf8.h"
2827
#include "tensorflow/lite/kernels/shim/status_macros.h"
@@ -50,8 +49,7 @@ FastWordpieceTokenizer::Create(const void* config_flatbuffer) {
5049
// `GetFastWordpieceTokenizerConfig()` is autogenerated by flatbuffer.
5150
tokenizer.config_ = GetFastWordpieceTokenizerConfig(config_flatbuffer);
5251
auto trie_or = trie_utils::DartsCloneTrieWrapper::Create(
53-
absl::MakeSpan(tokenizer.config_->trie_array()->data(),
54-
tokenizer.config_->trie_array()->size()));
52+
tokenizer.config_->trie_array()->data());
5553
if (!trie_or.ok()) {
5654
return absl::InvalidArgumentError(
5755
"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_));
437+
trie_utils::DartsCloneTrieWrapper::Create(trie_array_.data()));
438438
trie_.emplace(std::move(trie));
439439

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

0 commit comments

Comments
 (0)