From 00c0948e7b34c6523a3a89e1d6c71ce4396a0b2b Mon Sep 17 00:00:00 2001
From: "trop[bot]" <37223003+trop[bot]@users.noreply.github.com>
Date: Fri, 6 Sep 2024 22:16:51 -0500
Subject: [PATCH] refactor: take a `uint8_t` span in `ValidateIntegrityOrDie()`
 (#43614)

refactor: take a uint8_t span in ValidateIntegrityOrDie()

Doing some groundwork for fixing unsafe base::File() APIs:

- Change ValidateIntegrityOrDie() to take a span<const uint8_t> arg.
  We'll need this to migrate asar's base::File API calls away from the
  ones tagged `UNSAFE_BUFFER_USAGE` because the safe counterparts use
  span<uint8_t> too.

- Simplify ValidateIntegrityOrDie()'s implementation by using
  crypto::SHA256Hash() instead of reinventing the wheel.

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Charles Kerr <charles@charleskerr.com>
---
 shell/common/asar/archive.cc               |  5 ++---
 shell/common/asar/asar_util.cc             | 16 ++++------------
 shell/common/asar/asar_util.h              |  5 +++--
 shell/common/asar/scoped_temporary_file.cc |  5 ++---
 4 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/shell/common/asar/archive.cc b/shell/common/asar/archive.cc
index 93de8af187..479e8fe891 100644
--- a/shell/common/asar/archive.cc
+++ b/shell/common/asar/archive.cc
@@ -251,9 +251,8 @@ bool Archive::Init() {
     // Currently we only support the sha256 algorithm, we can add support for
     // more below ensure we read them in preference order from most secure to
     // least
-    if (integrity.value().algorithm != HashAlgorithm::kNone) {
-      ValidateIntegrityOrDie(header.c_str(), header.length(),
-                             integrity.value());
+    if (integrity->algorithm != HashAlgorithm::kNone) {
+      ValidateIntegrityOrDie(base::as_byte_span(header), *integrity);
     } else {
       LOG(FATAL) << "No eligible hash for validatable asar archive: "
                  << RelativePath().value();
diff --git a/shell/common/asar/asar_util.cc b/shell/common/asar/asar_util.cc
index e3af9969ff..0d3ebb33eb 100644
--- a/shell/common/asar/asar_util.cc
+++ b/shell/common/asar/asar_util.cc
@@ -133,25 +133,17 @@ bool ReadFileToString(const base::FilePath& path, std::string* contents) {
     return false;
   }
 
-  if (info.integrity.has_value()) {
-    ValidateIntegrityOrDie(contents->data(), contents->size(),
-                           info.integrity.value());
-  }
+  if (info.integrity)
+    ValidateIntegrityOrDie(base::as_byte_span(*contents), *info.integrity);
 
   return true;
 }
 
-void ValidateIntegrityOrDie(const char* data,
-                            size_t size,
+void ValidateIntegrityOrDie(base::span<const uint8_t> input,
                             const IntegrityPayload& integrity) {
   if (integrity.algorithm == HashAlgorithm::kSHA256) {
-    uint8_t hash[crypto::kSHA256Length];
-    auto hasher = crypto::SecureHash::Create(crypto::SecureHash::SHA256);
-    hasher->Update(data, size);
-    hasher->Finish(hash, sizeof(hash));
     const std::string hex_hash =
-        base::ToLowerASCII(base::HexEncode(hash, sizeof(hash)));
-
+        base::ToLowerASCII(base::HexEncode(crypto::SHA256Hash(input)));
     if (integrity.hash != hex_hash) {
       LOG(FATAL) << "Integrity check failed for asar archive ("
                  << integrity.hash << " vs " << hex_hash << ")";
diff --git a/shell/common/asar/asar_util.h b/shell/common/asar/asar_util.h
index ee148f26d1..1f678e6d39 100644
--- a/shell/common/asar/asar_util.h
+++ b/shell/common/asar/asar_util.h
@@ -8,6 +8,8 @@
 #include <memory>
 #include <string>
 
+#include "base/containers/span.h"
+
 namespace base {
 class FilePath;
 }
@@ -29,8 +31,7 @@ bool GetAsarArchivePath(const base::FilePath& full_path,
 // Same with base::ReadFileToString but supports asar Archive.
 bool ReadFileToString(const base::FilePath& path, std::string* contents);
 
-void ValidateIntegrityOrDie(const char* data,
-                            size_t size,
+void ValidateIntegrityOrDie(base::span<const uint8_t> input,
                             const IntegrityPayload& integrity);
 
 }  // namespace asar
diff --git a/shell/common/asar/scoped_temporary_file.cc b/shell/common/asar/scoped_temporary_file.cc
index d0f00034db..1ad0ccf55a 100644
--- a/shell/common/asar/scoped_temporary_file.cc
+++ b/shell/common/asar/scoped_temporary_file.cc
@@ -68,9 +68,8 @@ bool ScopedTemporaryFile::InitFromFile(
   if (len != static_cast<int>(size))
     return false;
 
-  if (integrity.has_value()) {
-    ValidateIntegrityOrDie(buf.data(), buf.size(), integrity.value());
-  }
+  if (integrity)
+    ValidateIntegrityOrDie(base::as_byte_span(buf), *integrity);
 
   base::File dest(path_, base::File::FLAG_OPEN | base::File::FLAG_WRITE);
   if (!dest.IsValid())