winget-cli

commit 14ee740909cb64abda8f1962f2d6333081b147a3
parent e58bc985d677ab34d77e6f82785e1408bf245781
Author: JohnMcPMS <johnmcp@microsoft.com>
Date:   Thu, 17 Jun 2021 13:48:44 -0700

Fix a potential infinite loop due to file read errors during hashing (#1175)

The `std::istream` was not handled properly here, as only `eof` determined the loop termination, when any of the state bits should have.  Now requires `good` and additionally lets `bad` throw exceptions directly, while throwing our own generic exception if `fail` is set without `eof`.
Diffstat:
M
.github/actions/spelling/expect.txt
 | 
1
+
M
src/AppInstallerCommonCore/Errors.cpp
 | 
2
++
M
src/AppInstallerCommonCore/Public/AppInstallerErrors.h
 | 
1
+
M
src/AppInstallerCommonCore/Public/AppInstallerSHA256.h
 | 
4
++--
M
src/AppInstallerCommonCore/SHA256.cpp
 | 
40
++++++++++++++++++++++++----------------

5 files changed, 30 insertions(+), 18 deletions(-)
diff --git a/.github/actions/spelling/expect.txt b/.github/actions/spelling/expect.txt
@@ -29,6 +29,7 @@ AType
 AUrl
 Aysnc
 azurewebsites
+badbit
 Baz
 bcp
 Beigi
diff --git a/src/AppInstallerCommonCore/Errors.cpp b/src/AppInstallerCommonCore/Errors.cpp
@@ -141,6 +141,8 @@ namespace AppInstaller
                 return "Invalid rest source contract version";
             case APPINSTALLER_CLI_ERROR_SOURCE_DATA_INTEGRITY_FAILURE:
                 return "The source data is corrupted or tampered";
+            case APPINSTALLER_CLI_ERROR_STREAM_READ_FAILURE:
+                return "Error reading from the stream";
             default:
                 return "Unknown Error Code";
             }
diff --git a/src/AppInstallerCommonCore/Public/AppInstallerErrors.h b/src/AppInstallerCommonCore/Public/AppInstallerErrors.h
@@ -76,6 +76,7 @@
 #define APPINSTALLER_CLI_ERROR_RESTSOURCE_UNSUPPORTED_MIME_TYPE ((HRESULT)0x8a15003D)
 #define APPINSTALLER_CLI_ERROR_RESTSOURCE_INVALID_VERSION       ((HRESULT)0x8a15003E)
 #define APPINSTALLER_CLI_ERROR_SOURCE_DATA_INTEGRITY_FAILURE    ((HRESULT)0x8a15003F)
+#define APPINSTALLER_CLI_ERROR_STREAM_READ_FAILURE              ((HRESULT)0x8a150040)
 
 namespace AppInstaller
 {
diff --git a/src/AppInstallerCommonCore/Public/AppInstallerSHA256.h b/src/AppInstallerCommonCore/Public/AppInstallerSHA256.h
@@ -44,10 +44,10 @@ namespace AppInstaller::Utility {
         }
 
         // Computes the hash of the given buffer immediately.
-        static std::vector<uint8_t> ComputeHash(const uint8_t* buffer, std::uint32_t cbBuffer);
+        static HashBuffer ComputeHash(const uint8_t* buffer, std::uint32_t cbBuffer);
 
         // Computes the hash from a given stream.
-        static std::vector<uint8_t> ComputeHash(std::istream& in);
+        static HashBuffer ComputeHash(std::istream& in);
 
         static std::string ConvertToString(const HashBuffer& hashBuffer);
 
diff --git a/src/AppInstallerCommonCore/SHA256.cpp b/src/AppInstallerCommonCore/SHA256.cpp
@@ -1,11 +1,11 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
-
 #include <pch.h>
 #define WIN32_NO_STATUS
 #include <bcrypt.h>
 #include "Public/AppInstallerSHA256.h"
 #include "Public/AppInstallerRuntime.h"
+#include "Public/AppInstallerErrors.h"
 
 using namespace AppInstaller::Runtime;
 
@@ -108,7 +108,7 @@ namespace AppInstaller::Utility {
         return std::string(resultBuffer);
     }
 
-    std::vector<uint8_t> SHA256::ConvertToBytes(const std::string& hashStr)
+    SHA256::HashBuffer SHA256::ConvertToBytes(const std::string& hashStr)
     {
         if (hashStr.size() != HashStringSizeInChars)
         {
@@ -116,7 +116,7 @@ namespace AppInstaller::Utility {
         }
 
         auto hashCStr = hashStr.c_str();
-        std::vector<uint8_t> resultBuffer;
+        SHA256::HashBuffer resultBuffer;
 
         resultBuffer.resize(HashBufferSizeInBytes);
 
@@ -128,34 +128,42 @@ namespace AppInstaller::Utility {
         return resultBuffer;
     }
 
-    std::vector<uint8_t> SHA256::ComputeHash(const std::uint8_t* buffer, std::uint32_t cbBuffer)
+    SHA256::HashBuffer SHA256::ComputeHash(const std::uint8_t* buffer, std::uint32_t cbBuffer)
     {
         SHA256 hasher;
         hasher.Add(buffer, cbBuffer);
-
-        std::vector<uint8_t> result;
-        hasher.Get(result);
-
-        return result;
+        return hasher.Get();
     }
 
-    std::vector<uint8_t> SHA256::ComputeHash(std::istream& in)
+    SHA256::HashBuffer SHA256::ComputeHash(std::istream& in)
     {
+        // Throw exceptions on badbit
+        auto excState = in.exceptions();
+        auto revertExcState = wil::scope_exit([excState, &in]() { in.exceptions(excState); });
+        in.exceptions(std::ios_base::badbit);
+
         const int bufferSize = 1024 * 1024; // 1MB
         auto buffer = std::make_unique<uint8_t[]>(bufferSize);
 
         SHA256 hasher;
 
-        while (!in.eof())
+        while (in.good())
         {
             in.read((char*)(buffer.get()), bufferSize);
-            hasher.Add(buffer.get(), static_cast<size_t>(in.gcount()));
+            if (in.gcount())
+            {
+                hasher.Add(buffer.get(), static_cast<size_t>(in.gcount()));
+            }
         }
 
-        std::vector<uint8_t> result;
-        hasher.Get(result);
-
-        return result;
+        if (in.eof())
+        {
+            return hasher.Get();
+        }
+        else
+        {
+            THROW_HR(APPINSTALLER_CLI_ERROR_STREAM_READ_FAILURE);
+        }
     }
 
     void SHA256::SHA256ContextDeleter::operator()(SHA256Context* context)