[automerger skipped] Skip ab/6749736 in stage. am: 99f9472c3f -s ours am: 6b5a5a0e22 -s ours am: e6b29cbc26 -s ours
am skip reason: Change-Id I5ddd6573d86a554f9f0eee041490e748affd2b4d with SHA-1 23f525ff9d is in history
Original change: https://googleplex-android-review.googlesource.com/c/platform/system/update_engine/+/12797272
Change-Id: I84f890906eca277786e3e0374d1af3cf8637a436
diff --git a/payload_consumer/delta_performer.cc b/payload_consumer/delta_performer.cc
index 87fc4cf..626c881 100644
--- a/payload_consumer/delta_performer.cc
+++ b/payload_consumer/delta_performer.cc
@@ -519,27 +519,24 @@
if (!CanPerformInstallOperation(op))
return true;
- // Validate the operation only if the metadata signature is present.
- // Otherwise, keep the old behavior. This serves as a knob to disable
- // the validation logic in case we find some regression after rollout.
- // NOTE: If hash checks are mandatory and if metadata_signature is empty,
- // we would have already failed in ParsePayloadMetadata method and thus not
- // even be here. So no need to handle that case again here.
- if (!payload_->metadata_signature.empty()) {
- // Note: Validate must be called only if CanPerformInstallOperation is
- // called. Otherwise, we might be failing operations before even if there
- // isn't sufficient data to compute the proper hash.
- *error = ValidateOperationHash(op);
- if (*error != ErrorCode::kSuccess) {
- if (install_plan_->hash_checks_mandatory) {
- LOG(ERROR) << "Mandatory operation hash check failed";
- return false;
- }
-
- // For non-mandatory cases, just send a UMA stat.
- LOG(WARNING) << "Ignoring operation validation errors";
- *error = ErrorCode::kSuccess;
+ // Validate the operation unconditionally. This helps prevent the
+ // exploitation of vulnerabilities in the patching libraries, e.g. bspatch.
+ // The hash of the patch data for a given operation is embedded in the
+ // payload metadata; and thus has been verified against the public key on
+ // device.
+ // Note: Validate must be called only if CanPerformInstallOperation is
+ // called. Otherwise, we might be failing operations before even if there
+ // isn't sufficient data to compute the proper hash.
+ *error = ValidateOperationHash(op);
+ if (*error != ErrorCode::kSuccess) {
+ if (install_plan_->hash_checks_mandatory) {
+ LOG(ERROR) << "Mandatory operation hash check failed";
+ return false;
}
+
+ // For non-mandatory cases, just send a UMA stat.
+ LOG(WARNING) << "Ignoring operation validation errors";
+ *error = ErrorCode::kSuccess;
}
// Makes sure we unblock exit when this operation completes.