0

Verify resource order in data pack files

This CL adds a resource order check when loading a data pack or calling DataPack::GetStringPiece to make sure the resources are ordered sequentially in memory.

Bug: 1504936
Change-Id: Ie3bf1d9dbac937407355935a859a5daa9ce84350
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5059113
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Reviewed-by: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1238675}
This commit is contained in:
Lyra Rebane
2023-12-18 19:53:17 +00:00
committed by Chromium LUCI CQ
parent 28c2299e42
commit c4b2e6246a
5 changed files with 40 additions and 1 deletions

@ -841,6 +841,7 @@ Luke Warlow <luke@warlow.dev>
Luke Zarko <lukezarko@gmail.com>
Luoxi Pan <l.panpax@gmail.com>
Lu Yahan <yahan@iscas.ac.cn>
Lyra Rebane <rebane2001@gmail.com>
Ma Aiguo <imaiguo@gmail.com>
Maarten Lankhorst <m.b.lankhorst@gmail.com>
Maciej Pawlowski <m.pawlowski@eyeo.com>

@ -279,7 +279,16 @@ bool DataPack::SanityCheckFileAndRegisterResources(size_t margin_to_skip,
}
}
// 3) Verify the aliases are within the appropriate bounds.
// 3) Verify the entries are ordered correctly.
for (size_t i = 0; i < resource_count_; ++i) {
if (resource_table_[i].file_offset > resource_table_[i + 1].file_offset) {
LOG(ERROR) << "Data pack file corruption: " << "Entry #" << i + 1
<< " before Entry #" << i << ".";
return false;
}
}
// 4) Verify the aliases are within the appropriate bounds.
for (size_t i = 0; i < alias_count_; ++i) {
if (alias_table_[i].entry_index >= resource_count_) {
LOG(ERROR) << "Data pack file corruption: "
@ -393,6 +402,14 @@ absl::optional<base::StringPiece> DataPack::GetStringPiece(
<< "file modified?";
return absl::nullopt;
}
if (target->file_offset > next_entry->file_offset) {
size_t entry_index = target - resource_table_;
size_t next_index = next_entry - resource_table_;
LOG(ERROR) << "Entry #" << next_index << " in data pack is before Entry #"
<< entry_index << ". This should have been caught when loading. "
<< "Was the file modified?";
return absl::nullopt;
}
MaybePrintResourceId(resource_id);
return GetStringPieceFromOffset(target->file_offset, next_entry->file_offset,

@ -89,6 +89,18 @@ const uint8_t kSampleCorruptPakContents[] = {
const size_t kSampleCorruptPakSize = sizeof(kSampleCorruptPakContents);
const uint8_t kSampleMisorderedPakContents[] = {
0x05, 0x00, 0x00, 0x00, // version
0x01, 0x00, 0x00, 0x00, // encoding + padding
0x02, 0x00, 0x00, 0x00, // num_resources, num_aliases
0x06, 0x00, 0x2a, 0x00, 0x00, 0x00, // index entry 6 (wrong order)
0x04, 0x00, 0x1e, 0x00, 0x00, 0x00, // index entry 4
0x00, 0x00, 0x36, 0x00, 0x00, 0x00, // extra entry for the size of last
't', 'h', 'i', 's', ' ', 'i', 's', ' ', 'i', 'd', ' ', '4',
't', 'h', 'i', 's', ' ', 'i', 's', ' ', 'i', 'd', ' ', '6'};
const size_t kSampleMisorderedPakSize = sizeof(kSampleMisorderedPakContents);
const uint8_t kSamplePakContents2x[] = {
0x04, 0x00, 0x00, 0x00, // header(version
0x01, 0x00, 0x00, 0x00, // no. entries

@ -22,6 +22,8 @@ extern const uint8_t kEmptyPakContents[];
extern const size_t kEmptyPakSize;
extern const uint8_t kSampleCorruptPakContents[];
extern const size_t kSampleCorruptPakSize;
extern const uint8_t kSampleMisorderedPakContents[];
extern const size_t kSampleMisorderedPakSize;
} // namespace ui

@ -341,4 +341,11 @@ TEST(DataPackTest, ModifiedWhileUsed) {
}
#endif
TEST(DataPackTest, Misordered) {
DataPack pack(k100Percent);
ASSERT_FALSE(pack.LoadFromBuffer(
{kSampleMisorderedPakContents, kSampleMisorderedPakSize}));
}
} // namespace ui