fix(crypto): ConfigureAwait convention, destination writability check, self-contained pass rewind, short-stream signature check

Author: GabrielMarquezMatte

src/ExcelReader.Core/Crypto/PackageEncryptor.cs

@@ -26,6 +26,10 @@ internal static class PackageEncryptor
 
         internal static void Encrypt(Stream package, Stream destination, ExcelPassword password)
         {
+            if (!destination.CanWrite)
+            {
+                throw new ArgumentException("The destination stream must be writable.", nameof(destination));
+            }
             (byte[] encryptionInfo, Session session) = Prepare(package, password);
             try
             {
@@ -63,6 +67,10 @@ internal static void Encrypt(Stream package, Stream destination, ExcelPassword p
 
         internal static async ValueTask EncryptAsync(Stream package, Stream destination, ExcelPassword password, CancellationToken ct)
         {
+            if (!destination.CanWrite)
+            {
+                throw new ArgumentException("The destination stream must be writable.", nameof(destination));
+            }
             (byte[] encryptionInfo, Session session) = Prepare(package, password);
             try
             {
@@ -160,6 +168,11 @@ private static (byte[] EncryptionInfo, Session Session) Prepare(Stream package,
             {
                 throw new ArgumentException("The package stream is empty.", nameof(package));
             }
+            if (plainLength < 4)
+            {
+                throw new ArgumentException(
+                    "The package stream is not an OOXML package (missing the PK\\x03\\x04 signature).", nameof(package));
+            }
             if (password.Chars.IsEmpty)
             {
                 throw new ArgumentException("The password must not be empty.", nameof(password));
@@ -254,7 +267,6 @@ internal byte[] ComputeHmac(byte[] hmacKey)
             {
                 using IncrementalHash hmac = PackageIntegrity.CreateHmac(_descriptor.KeyData.Hash, hmacKey);
                 hmac.AppendData(Prefix);
-                Package.Position = _origin;
                 using (Local local = BeginPass())
                 {
                     int read;
@@ -269,6 +281,7 @@ internal byte[] ComputeHmac(byte[] hmacKey)
 
             internal Local BeginPass()
             {
+                Package.Position = _origin;
                 return new Local(_descriptor, _packageKey);
             }
 

src/ExcelReader.Core/Reader/Excel.Encrypt.cs

@@ -79,13 +79,17 @@ public static async ValueTask EncryptPackageAsync(string packagePath, string des
         {
             ArgumentException.ThrowIfNullOrEmpty(packagePath);
             ArgumentException.ThrowIfNullOrEmpty(destinationPath);
-#pragma warning disable CA2007, MA0004
-            await using FileStream package = new(packagePath, FileMode.Open, FileAccess.Read, FileShare.Read,
+            FileStream package = new(packagePath, FileMode.Open, FileAccess.Read, FileShare.Read,
                 bufferSize: 4096, useAsync: true);
-            await using FileStream destination = new(destinationPath, FileMode.Create, FileAccess.Write,
-                FileShare.None, bufferSize: 4096, useAsync: true);
-#pragma warning restore CA2007, MA0004
-            await PackageEncryptor.EncryptAsync(package, destination, password, ct).ConfigureAwait(false);
+            await using (package.ConfigureAwait(false))
+            {
+                FileStream destination = new(destinationPath, FileMode.Create, FileAccess.Write,
+                    FileShare.None, bufferSize: 4096, useAsync: true);
+                await using (destination.ConfigureAwait(false))
+                {
+                    await PackageEncryptor.EncryptAsync(package, destination, password, ct).ConfigureAwait(false);
+                }
+            }
         }
     }
 }

tests/ExcelReader.Tests/EncryptedPackageWriteTests.cs

@@ -151,9 +151,10 @@ public async Task EncryptPackageAsync_StreamOverload_RoundTrips()
         public void EncryptPackage_NullArguments_Throw()
         {
             using var stream = new MemoryStream([0x50, 0x4B, 0x03, 0x04], writable: false);
-            Assert.Throws<ArgumentNullException>(() => Excel.EncryptPackage(null!, stream, Password));
+            using var destination = new MemoryStream();
+            Assert.Throws<ArgumentNullException>(() => Excel.EncryptPackage(null!, destination, Password));
             Assert.Throws<ArgumentNullException>(() => Excel.EncryptPackage(stream, null!, Password));
-            Assert.Throws<ArgumentNullException>(() => Excel.EncryptPackage(stream, stream, null!));
+            Assert.Throws<ArgumentNullException>(() => Excel.EncryptPackage(stream, destination, null!));
         }
     }
 }

tests/ExcelReader.Tests/PackageEncryptorTests.cs

@@ -111,6 +111,22 @@ public void Encrypt_EmptyPackage_Throws()
             Assert.Throws<ArgumentException>(() => PackageEncryptor.Encrypt(source, destination, Password));
         }
 
+        [Fact]
+        public void Encrypt_PackageShorterThanSignature_ThrowsArgumentException()
+        {
+            using var source = new MemoryStream([0x50, 0x4B], writable: false);
+            using var destination = new MemoryStream();
+            Assert.Throws<ArgumentException>(() => PackageEncryptor.Encrypt(source, destination, Password));
+        }
+
+        [Fact]
+        public void Encrypt_NonWritableDestination_ThrowsArgumentException()
+        {
+            using var source = new MemoryStream(SamplePackage(), writable: false);
+            using var destination = new MemoryStream([], writable: false);
+            Assert.Throws<ArgumentException>(() => PackageEncryptor.Encrypt(source, destination, Password));
+        }
+
         // The index of a stream's last real data byte, as opposed to the zero padding a big stream
         // carries out to its final sector boundary — nothing reads that padding, so flipping a byte
         // in it cannot be detected as tampering.