Browse Source

refactor: take a `uint8_t` span in `ValidateIntegrityOrDie()` (#43612)

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 <[email protected]>
trop[bot] 7 months ago
parent
commit
8d76b8dba9

+ 2 - 3
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();

+ 4 - 12
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 << ")";

+ 3 - 2
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

+ 2 - 3
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())