Skip to content

Avoid rooting cryptography through ZipArchive on browser #130688

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Copilot wants to merge 6 commits into
base: main
Choose a base branch
from
+59 −10
Open

Avoid rooting cryptography through ZipArchive on browser #130688

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8cf8ad5
Exclude ZIP encryption crypto from browser builds
Copilot Jul 14, 2026
c315ffb
Preserve browser ZipCrypto support
Copilot Jul 14, 2026
119fe70
Check WASM P/Invoke table in ZIP test
Copilot Jul 15, 2026
7ad6074
Handle both WASM runtime table names
Copilot Jul 15, 2026
88e7cf0
Use CoreCLR PInvoke output name
Copilot Jul 15, 2026
84240f3
Merge branch 'main' into copilot/fix-ziparchive-password-support
alinpahontu2912 Jul 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/libraries/System.IO.Compression/src/Resources/Strings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,7 @@
<data name="WinZipEncryptionNotSupportedOnBrowser" xml:space="preserve">
<value>WinZip AES encryption is not supported on the browser platform.</value>
</data>

@rzikm rzikm Jul 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whitespace change should be reverted

<data name="UnsupportedEncryptionMethod" xml:space="preserve">
<value>The entry's encryption method is not supported.</value>
</data>
Expand Down
7 changes: 4 additions & 3 deletions src/libraries/System.IO.Compression/src/System.IO.Compression.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,9 @@
<Compile Include="$(CommonPath)System\Obsoletions.cs" Link="Common\System\Obsoletions.cs" />
<Compile Include="System\IO\Compression\ZipEncryptionMethod.cs" />
<Compile Include="System\IO\Compression\ZipCryptoStream.cs" />
<Compile Include="System\IO\Compression\WinZipAesStream.cs" />
<Compile Include="System\IO\Compression\WinZipAesKeyMaterial.cs" />
<Compile Include="System\IO\Compression\WinZipAesStream.cs" Condition="'$(TargetPlatformIdentifier)' != 'browser'" />
<Compile Include="System\IO\Compression\WinZipAesKeyMaterial.cs" Condition="'$(TargetPlatformIdentifier)' != 'browser'" />
<Compile Include="System\IO\Compression\WinZipAes.PlatformNotSupported.cs" Condition="'$(TargetPlatformIdentifier)' == 'browser'" />
<Compile Include="System\IO\Compression\ZipCryptoKeys.cs" />
</ItemGroup>

Expand Down Expand Up @@ -111,7 +112,7 @@
<ItemGroup>
<ProjectReference Include="$(LibrariesProjectRoot)System.Collections\src\System.Collections.csproj" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Memory\src\System.Memory.csproj" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Security.Cryptography\src\System.Security.Cryptography.csproj" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Security.Cryptography\src\System.Security.Cryptography.csproj" Condition="'$(TargetPlatformIdentifier)' != 'browser'" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Runtime\src\System.Runtime.csproj" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Runtime.InteropServices\src\System.Runtime.InteropServices.csproj" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Threading\src\System.Threading.csproj" />
Expand Down
23 changes: 23 additions & 0 deletions ...braries/System.IO.Compression/src/System/IO/Compression/WinZipAes.PlatformNotSupported.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Threading;
using System.Threading.Tasks;

namespace System.IO.Compression
{
internal readonly struct WinZipAesKeyMaterial;

internal static class WinZipAesStream
{
Comment on lines +7 to +12
internal static int GetSaltSize(int keySizeBits) => throw CreateException();

internal static WinZipAesKeyMaterial CreateKey(ReadOnlySpan<char> password, byte[]? salt, int keySizeBits) => throw CreateException();

internal static Stream Create(Stream baseStream, WinZipAesKeyMaterial keyMaterial, long totalStreamSize, bool encrypting, bool leaveOpen = false) => throw CreateException();

internal static Task<Stream> CreateAsync(Stream baseStream, WinZipAesKeyMaterial keyMaterial, long totalStreamSize, bool encrypting, bool leaveOpen = false, CancellationToken cancellationToken = default) => throw CreateException();

private static PlatformNotSupportedException CreateException() => new(SR.WinZipEncryptionNotSupportedOnBrowser);
}
}
4 changes: 2 additions & 2 deletions src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Async.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -728,7 +728,7 @@ private async Task WriteLocalFileHeaderAndDataIfNeededAsync(bool forceWrite, Can

ushort verifierLow2Bytes = (ushort)ZipHelper.DateTimeToDosTime(_lastModified.DateTime);

ZipCryptoStream encryptionStream = ZipCryptoStream.Create(
Stream encryptionStream = ZipCryptoStream.Create(
baseStream: _archive.ArchiveStream,
keys: _derivedZipCryptoKeyMaterial.Value,
passwordVerifierLow2Bytes: verifierLow2Bytes,
Expand Down Expand Up @@ -782,7 +782,7 @@ private async Task WriteLocalFileHeaderAndDataIfNeededAsync(bool forceWrite, Can
// The AES extra field stores the real compression method
bool useDeflate = _compressionLevel != CompressionLevel.NoCompression;

WinZipAesStream encryptionStream = WinZipAesStream.Create(
Stream encryptionStream = WinZipAesStream.Create(
baseStream: _archive.ArchiveStream,
keyMaterial: _derivedAesKeyMaterial.Value,
totalStreamSize: -1,
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1776,7 +1776,7 @@ private unsafe void WriteLocalFileHeaderAndDataIfNeeded(bool forceWrite)

ushort verifierLow2Bytes = (ushort)ZipHelper.DateTimeToDosTime(_lastModified.DateTime);

using (ZipCryptoStream encryptionStream = ZipCryptoStream.Create(
using (Stream encryptionStream = ZipCryptoStream.Create(
baseStream: _archive.ArchiveStream,
keys: _derivedZipCryptoKeyMaterial.Value,
passwordVerifierLow2Bytes: verifierLow2Bytes,
Expand Down Expand Up @@ -1807,7 +1807,7 @@ private unsafe void WriteLocalFileHeaderAndDataIfNeeded(bool forceWrite)

bool useDeflate = _compressionLevel != CompressionLevel.NoCompression;

using (WinZipAesStream encryptionStream = WinZipAesStream.Create(
using (Stream encryptionStream = WinZipAesStream.Create(
baseStream: _archive.ArchiveStream,
keyMaterial: _derivedAesKeyMaterial.Value,
totalStreamSize: -1,
Expand Down
7 changes: 4 additions & 3 deletions src/libraries/System.IO.Compression/src/System/IO/Compression/ZipCryptoStream.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System.Buffers;
using System.Buffers.Binary;
using System.Diagnostics;
using System.Security.Cryptography;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
Expand Down Expand Up @@ -138,7 +137,7 @@ internal static ZipCryptoKeys CreateKey(ReadOnlySpan<char> password)
}
finally
{
CryptographicOperations.ZeroMemory(passwordBytes);
Array.Clear(passwordBytes);
ArrayPool<byte>.Shared.Return(passwordBytes);
}

Expand All @@ -150,7 +149,9 @@ private void CalculateHeader(Span<byte> header)
Debug.Assert(header.Length == 12);

// bytes 0..9 random
RandomNumberGenerator.Fill(header.Slice(0, 10));
Span<byte> randomBytes = stackalloc byte[16];
Guid.NewGuid().TryWriteBytes(randomBytes);

@github-actions github-actions Bot Jul 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This replaces a cryptographically-secure RandomNumberGenerator.Fill with Guid.NewGuid() to source the 10 random bytes of the ZipCrypto encryption header. Guid.NewGuid() produces a version-4 (random) GUID, but the .NET documentation explicitly does not guarantee it is generated by a cryptographically-secure RNG, and only 122 of its 128 bits are random (6 bits are fixed version/variant markers). Here you copy randomBytes.Slice(0, 10), i.e. the first 10 bytes, which for a v4 GUID includes the 4 version bits at offset 6 — so those 10 bytes carry slightly less than 80 bits of entropy and their quality is implementation-defined.

For the legacy ZipCrypto scheme (already weak by design) this is likely acceptable in practice, and the goal of dropping the System.Security.Cryptography dependency on browser is reasonable. However, please confirm this trade-off is intentional and documented: the header randomness is what protects against known-plaintext recovery of the key stream, so weakening its source is a security-relevant change. If a stronger source without the crypto dependency is desired, consider seeding from a per-header call that mixes multiple Guid.NewGuid() values, or gating this fallback to the browser target only while keeping RandomNumberGenerator.Fill elsewhere.

randomBytes.Slice(0, 10).CopyTo(header);
Comment on lines +152 to +154

@rzikm rzikm Jul 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guid does guarantee uniqueness, not randomness, so I don't think this is good.

Since ZipCrypto is broken anyway, maybe using Random.Shared.NextBytes is good enough here to avoid rooting System.Security.Cryptography dll. Maybe keep using RandomNumberGenerator on non-browser platforms?

cc @bartonjs, @GrabYourPitchforks for opinions.


// bytes 10..11 verifier
if (_crc32ForHeader.HasValue)
Expand Down
20 changes: 20 additions & 0 deletions src/libraries/System.IO.Compression/tests/ZipArchive/zip_ReadTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1052,6 +1052,26 @@ public async Task DecryptEntries_MixedEncryptions(bool async)
await DisposeZipArchive(async, archive);
}

[Theory]
[MemberData(nameof(Get_Booleans_Data))]
[PlatformSpecific(TestPlatforms.Browser)]
public async Task DecryptZipCryptoEntry_Browser(bool async)
{
const string Password = "S3cur3P@ssw0rd";
using Stream archiveStream = await StreamHelpers.CreateTempCopyStream(passwordProtected("PasswordProtected_MixedEncryptions.zip"));
ZipArchive archive = await CreateZipArchive(async, archiveStream, ZipArchiveMode.Read);

ZipArchiveEntry entry = archive.GetEntry("hello.txt");
Assert.NotNull(entry);
Assert.Equal(ZipEncryptionMethod.ZipCrypto, entry.EncryptionMethod);

using Stream entryStream = await OpenEntryStream(async, entry, Password);
using StreamReader reader = new(entryStream);
Assert.Equal("Hello", reader.ReadToEnd().TrimEnd());

await DisposeZipArchive(async, archive);
}

[Theory]
[MemberData(nameof(Get_Booleans_Data))]
[SkipOnPlatform(TestPlatforms.Browser, "WinZip AES encryption is not supported on browser.")]
Expand Down
3 changes: 3 additions & 0 deletions src/mono/wasm/Wasm.Build.Tests/NativeBuildTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,9 @@ public async Task ZipArchiveInteropTest()
Configuration config = Configuration.Debug;
ProjectInfo info = CopyTestAsset(config, false, TestAsset.WasmBasicTestApp, "ZipArchiveInteropTest", extraProperties: "<WasmBuildNative>true</WasmBuildNative>");
BuildProject(info, config, new BuildOptions(AssertAppBundle: false));
BuildPaths paths = GetBuildPaths(config, forPublish: false);
string pinvokeTableFileName = IsCoreClrRuntime ? "callhelpers-pinvoke.cpp" : "pinvoke-table.h";
Assert.DoesNotContain("System_Security_Cryptography", File.ReadAllText(Path.Combine(paths.ObjWasmDir, pinvokeTableFileName)));
RunResult result = await RunForBuildWithDotnetRun(new BrowserRunOptions(config, TestScenario: "ZipArchiveInteropTest"));
Assert.Collection(
result.TestOutput,
Expand Down
Loading