Skip to content

Commit 66740b1

Browse files
Mitigates a critical Out-of-Bounds (OOB) read vulnerability by transitioning internal trie references to absl::Span and validating arra...
PiperOrigin-RevId: 914999991
1 parent eb6b2c0 commit 66740b1

8 files changed

Lines changed: 60 additions & 20 deletions

tensorflow_text/core/kernels/BUILD

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,7 @@ tf_cc_library(
215215
"@com_google_absl//absl/memory",
216216
"@com_google_absl//absl/status",
217217
"@com_google_absl//absl/strings",
218+
"@com_google_absl//absl/types:span",
218219
"@icu//:common",
219220
# lite/kernels/shim:status_macros tensorflow dep,
220221
],
@@ -351,7 +352,10 @@ cc_library(
351352
"darts_clone_trie_wrapper.h",
352353
],
353354
deps = [
355+
"@com_google_absl//absl/base:core_headers",
356+
"@com_google_absl//absl/status",
354357
"@com_google_absl//absl/status:statusor",
358+
"@com_google_absl//absl/types:span",
355359
],
356360
)
357361

@@ -363,6 +367,7 @@ cc_test(
363367
":darts_clone_trie_builder",
364368
":darts_clone_trie_wrapper",
365369
"@com_google_absl//absl/status",
370+
"@com_google_absl//absl/types:span",
366371
"@com_google_googletest//:gtest_main",
367372
],
368373
)
@@ -399,6 +404,7 @@ tf_cc_library(
399404
"@com_google_absl//absl/status",
400405
"@com_google_absl//absl/status:statusor",
401406
"@com_google_absl//absl/strings",
407+
"@com_google_absl//absl/types:span",
402408
"@icu//:nfkc",
403409
# lite/kernels/shim:status_macros tensorflow dep,
404410
],

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: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "absl/base/optimization.h"
2323
#include "absl/status/status.h"
2424
#include "absl/strings/string_view.h"
25+
#include "absl/types/span.h"
2526
#include "icu4c/source/common/unicode/utf8.h"
2627
#include "tensorflow/lite/kernels/shim/status_macros.h"
2728
#include "tensorflow_text/core/kernels/darts_clone_trie_wrapper.h"

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
@@ -24,6 +24,7 @@
2424
#include "absl/strings/str_cat.h"
2525
#include "absl/strings/str_join.h"
2626
#include "absl/strings/string_view.h"
27+
#include "absl/types/span.h"
2728
#include "icu4c/source/common/unicode/uchar.h"
2829
#include "icu4c/source/common/unicode/utf8.h"
2930
#include "tensorflow/lite/kernels/shim/status_macros.h"
@@ -56,7 +57,8 @@ FastWordpieceTokenizer::Create(const void* config_flatbuffer) {
5657
"FastWordpieceTokenizerConfig or its trie_array is null.");
5758
}
5859
auto trie_or = trie_utils::DartsCloneTrieWrapper::Create(
59-
tokenizer.config_->trie_array()->data());
60+
absl::MakeSpan(tokenizer.config_->trie_array()->data(),
61+
tokenizer.config_->trie_array()->size()));
6062
if (!trie_or.ok()) {
6163
return absl::InvalidArgumentError(
6264
"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)