aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--patch_read_write_unittest.cc100
-rw-r--r--patch_reader.cc44
-rw-r--r--patch_reader.h11
-rw-r--r--zucchini_apply.cc13
4 files changed, 117 insertions, 51 deletions
diff --git a/patch_read_write_unittest.cc b/patch_read_write_unittest.cc
index f6396d6..ed0a6ed 100644
--- a/patch_read_write_unittest.cc
+++ b/patch_read_write_unittest.cc
@@ -55,6 +55,52 @@ void TestSerialize(const ByteVector& expected, const T& value) {
EXPECT_EQ(expected, buffer);
}
+ByteVector CreatePatchElement() {
+ return {
+ 0x01, 0, 0, 0, // old_offset
+ 0x03, 0, 0, 0, // new_offset
+ 0x51, 0, 0, 0, // old_length
+ 0x50, 0, 0, 0, // new_length
+ 'P', 'x', '8', '6', // EXE_TYPE_WIN32_X86
+
+ 1, 0, 0, 0, // src_skip size
+ 0x10, // src_skip content
+ 1, 0, 0, 0, // dst_skip size
+ 0x11, // dst_skip content
+ 1, 0, 0, 0, // copy_count size
+ 0x12, // copy_count content
+
+ 1, 0, 0, 0, // extra_data size
+ 0x13, // extra_data content
+
+ 1, 0, 0, 0, // raw_delta_skip size
+ 0x14, // raw_delta_skip content
+ 1, 0, 0, 0, // raw_delta_diff size
+ 0x15, // raw_delta_diff content
+
+ 1, 0, 0, 0, // reference_delta size
+ 0x16, // reference_delta content
+
+ 2, 0, 0, 0, // pool count
+ 0, // pool_tag
+ 1, 0, 0, 0, // extra_targets size
+ 0x17, // extra_targets content
+ 2, // pool_tag
+ 1, 0, 0, 0, // extra_targets size
+ 0x18, // extra_targets content
+ };
+}
+
+// Helper to mutate test |data| (e.g., from CreatePatchElement()) at |idx| from |from_val| (as
+// sanity check) to |to_val|.
+void ModifyByte(size_t idx,
+ uint8_t from_val,
+ uint8_t to_val,
+ std::vector<uint8_t>* data) {
+ ASSERT_EQ(from_val, (*data)[idx]);
+ (*data)[idx] = to_val;
+}
+
} // namespace
bool operator==(const ByteVector& a, ConstBufferView b) {
@@ -391,39 +437,7 @@ TEST(TargetSourceSinkTest, Normal) {
}
TEST(PatchElementTest, Normal) {
- ByteVector data = {
- 0x01, 0, 0, 0, // old_offset
- 0x03, 0, 0, 0, // new_offset
- 0x02, 0, 0, 0, // old_length
- 0x04, 0, 0, 0, // new_length
- 'P', 'x', '8', '6', // EXE_TYPE_WIN32_X86
-
- 1, 0, 0, 0, // src_skip size
- 0x10, // src_skip content
- 1, 0, 0, 0, // dst_skip size
- 0x11, // dst_skip content
- 1, 0, 0, 0, // copy_count size
- 0x12, // copy_count content
-
- 1, 0, 0, 0, // extra_data size
- 0x13, // extra_data content
-
- 1, 0, 0, 0, // raw_delta_skip size
- 0x14, // raw_delta_skip content
- 1, 0, 0, 0, // raw_delta_diff size
- 0x15, // raw_delta_diff content
-
- 1, 0, 0, 0, // reference_delta size
- 0x16, // reference_delta content
-
- 2, 0, 0, 0, // pool count
- 0, // pool_tag
- 1, 0, 0, 0, // extra_targets size
- 0x17, // extra_targets content
- 2, // pool_tag
- 1, 0, 0, 0, // extra_targets size
- 0x18, // extra_targets content
- };
+ ByteVector data = CreatePatchElement();
PatchElementReader patch_element_reader =
TestInitialize<PatchElementReader>(&data);
@@ -433,9 +447,9 @@ TEST(PatchElementTest, Normal) {
EXPECT_EQ(kExeTypeWin32X86, element_match.old_element.exe_type);
EXPECT_EQ(kExeTypeWin32X86, element_match.new_element.exe_type);
EXPECT_EQ(0x1U, element_match.old_element.offset);
- EXPECT_EQ(0x2U, element_match.old_element.size);
+ EXPECT_EQ(0x51U, element_match.old_element.size);
EXPECT_EQ(0x3U, element_match.new_element.offset);
- EXPECT_EQ(0x4U, element_match.new_element.size);
+ EXPECT_EQ(0x50U, element_match.new_element.size);
EquivalenceSource equivalence_source =
patch_element_reader.GetEquivalenceSource();
@@ -476,6 +490,22 @@ TEST(PatchElementTest, Normal) {
TestSerialize(data, patch_element_writer);
}
+TEST(PatchElementTest, BadEquivalence) {
+ // If the "old" element is too small the test should fail.
+ {
+ ByteVector data = CreatePatchElement();
+ ModifyByte(8, 0x51, 0x04, &data); // old_length (too small)
+ TestInvalidInitialize<PatchElementReader>(&data);
+ }
+
+ // If the "new" element is too small the test should fail.
+ {
+ ByteVector data = CreatePatchElement();
+ ModifyByte(12, 0x50, 0x05, &data); // new_length (too small)
+ TestInvalidInitialize<PatchElementReader>(&data);
+ }
+}
+
TEST(EnsemblePatchTest, RawPatch) {
ByteVector data = {
0x5A, 0x75, 0x63, 0x00, // magic
diff --git a/patch_reader.cc b/patch_reader.cc
index 8215405..90f35f4 100644
--- a/patch_reader.cc
+++ b/patch_reader.cc
@@ -13,6 +13,19 @@
namespace zucchini {
+namespace {
+
+bool RangeIsInRegion(offset_t begin, uint32_t size, BufferRegion region) {
+ // Return false if |begin + size > kOffsetBound|, without overflowing.
+ if (begin >= kOffsetBound || kOffsetBound - begin < size)
+ return false;
+ offset_t end = begin + size;
+ // Return false if |[begin, end]| leaks outside |region|.
+ return begin >= region.lo() && end <= region.hi();
+}
+
+} // namespace
+
namespace patch {
bool ParseElementMatch(BufferSource* source, ElementMatch* element_match) {
@@ -217,7 +230,7 @@ PatchElementReader::~PatchElementReader() = default;
bool PatchElementReader::Initialize(BufferSource* source) {
bool ok = patch::ParseElementMatch(source, &element_match_) &&
- equivalences_.Initialize(source) &&
+ equivalences_.Initialize(source) && ValidateEquivalences() &&
extra_data_.Initialize(source) && raw_delta_.Initialize(source) &&
reference_delta_.Initialize(source);
if (!ok)
@@ -240,7 +253,7 @@ bool PatchElementReader::Initialize(BufferSource* source) {
}
auto insert_result = extra_targets_.insert({pool_tag, {}});
if (!insert_result.second) { // Element already present.
- LOG(ERROR) << "Multiple ExtraTargetList found for the same pool_tag";
+ LOG(ERROR) << "Multiple ExtraTargetList found for the same pool_tag.";
return false;
}
if (!insert_result.first->second.Initialize(source))
@@ -249,6 +262,33 @@ bool PatchElementReader::Initialize(BufferSource* source) {
return true;
}
+bool PatchElementReader::ValidateEquivalences() {
+ EquivalenceSource equivalences_copy = equivalences_;
+
+ BufferRegion old_region = element_match_.old_element.region();
+ BufferRegion new_region = element_match_.new_element.region();
+
+ // Validate that each |equivalence| falls within the bounds of the
+ // |element_match_| and are in order.
+ offset_t prev_dst_end = 0;
+ for (auto equivalence = equivalences_copy.GetNext(); equivalence.has_value();
+ equivalence = equivalences_copy.GetNext()) {
+ if (!RangeIsInRegion(equivalence->src_offset, equivalence->length,
+ old_region) ||
+ !RangeIsInRegion(equivalence->dst_offset, equivalence->length,
+ new_region)) {
+ LOG(ERROR) << "Out of bounds equivalence detected.";
+ return false;
+ }
+ if (prev_dst_end > equivalence->dst_end()) {
+ LOG(ERROR) << "Out of order equivalence detected.";
+ return false;
+ }
+ prev_dst_end = equivalence->dst_end();
+ }
+ return true;
+}
+
/******** EnsemblePatchReader ********/
base::Optional<EnsemblePatchReader> EnsemblePatchReader::Create(
diff --git a/patch_reader.h b/patch_reader.h
index ad517f5..5d3c6dd 100644
--- a/patch_reader.h
+++ b/patch_reader.h
@@ -218,7 +218,11 @@ class PatchElementReader {
const Element& old_element() const { return element_match_.old_element; }
const Element& new_element() const { return element_match_.new_element; }
- // The Get*() functions below return copies of cached sources.
+ // The Get*() functions below return copies of cached sources. Callers may
+ // assume the following:
+ // - Equivalences satisfy basic boundary constraints
+ // - "Old" / "new" blocks lie entirely in "old" / "new" images.
+ // - "New" blocks are sorted.
EquivalenceSource GetEquivalenceSource() const { return equivalences_; }
ExtraDataSource GetExtraDataSource() const { return extra_data_; }
RawDeltaSource GetRawDeltaSource() const { return raw_delta_; }
@@ -231,6 +235,11 @@ class PatchElementReader {
}
private:
+ // Checks that "old" and "new" blocks of each item in |equivalences_| satisfy
+ // basic order and image bound constraints (using |element_match_| data).
+ // Returns true if successful.
+ bool ValidateEquivalences();
+
ElementMatch element_match_;
// Cached sources.
diff --git a/zucchini_apply.cc b/zucchini_apply.cc
index 63b8ce0..b67bf63 100644
--- a/zucchini_apply.cc
+++ b/zucchini_apply.cc
@@ -26,13 +26,6 @@ bool ApplyEquivalenceAndExtraData(ConstBufferView old_image,
for (auto equivalence = equiv_source.GetNext(); equivalence.has_value();
equivalence = equiv_source.GetNext()) {
- // TODO(ckitagawa): Ensure guards don't overflow. Move these validation
- // check to the patch reader.
- // Validate that the |equivalence| is within the |new_image|.
- if (equivalence->dst_end() > new_image.size()) {
- LOG(ERROR) << "Out of bounds equivalence";
- return false;
- }
MutableBufferView::iterator next_dst_it =
new_image.begin() + equivalence->dst_offset;
CHECK(next_dst_it >= dst_it);
@@ -47,12 +40,6 @@ bool ApplyEquivalenceAndExtraData(ConstBufferView old_image,
// copy should be valid.
dst_it = std::copy(extra_data->begin(), extra_data->end(), dst_it);
CHECK_EQ(dst_it, next_dst_it);
-
- // Validate that the |equivalence| is within the |old_image|.
- if (equivalence->src_end() > old_image.size()) {
- LOG(ERROR) << "Out of bounds equivalence";
- return false;
- }
dst_it = std::copy_n(old_image.begin() + equivalence->src_offset,
equivalence->length, dst_it);
CHECK_EQ(dst_it, next_dst_it + equivalence->length);
generated by cgit v1.2.3 (git 2.25.1) at 2024-07-10 09:58:09 +0000