Merge spl-2021-03-05

Change-Id: Ie78b737f2d86c1a9ca5e46ac3dd772486ef5f050

1 files changed, 17 insertions, 20 deletions

diff --git a/payload_consumer/delta_performer.cc b/payload_consumer/delta_performer.cc
index 4c4ff041..15973e93 100644
--- a/payload_consumer/delta_performer.cc
+++ b/payload_consumer/delta_performer.cc
@@ -690,27 +690,24 @@ bool DeltaPerformer::Write(const void* bytes, size_t count, ErrorCode* error) {
     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.