From 44a4328ea8977b6f4b165e47ecaee54c23af7e97 Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles@charleskerr.com>
Date: Fri, 6 Sep 2024 20:22:44 -0500
Subject: [PATCH] refactor: take a `uint8_t` span in `ValidateIntegrityOrDie()`
 (#43592)

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.
---
 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 edaae7b043..b3daddcf8c 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 a2076217b6..cba95a00c4 100644
--- a/shell/common/asar/asar_util.cc
+++ b/shell/common/asar/asar_util.cc
@@ -132,25 +132,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 357793a9e2..32b192dda0 100644
--- a/shell/common/asar/scoped_temporary_file.cc
+++ b/shell/common/asar/scoped_temporary_file.cc
@@ -67,9 +67,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())