From c4893094cba29fe4dbd08397a5585c398cd1617a Mon Sep 17 00:00:00 2001 From: alinpahontu2912 Date: Thu, 23 Jul 2026 13:24:52 +0200 Subject: [PATCH] add missing check for corrupted central directory --- .../IO/Compression/ZipArchiveEntry.Async.cs | 9 ++++ .../System/IO/Compression/ZipArchiveEntry.cs | 9 ++++ .../zip_InvalidParametersAndStrangeFiles.cs | 49 +++++++++++++++++++ 3 files changed, 67 insertions(+) diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Async.cs b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Async.cs index f352d28b4578f2..02c1c9b764239e 100644 --- a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Async.cs +++ b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Async.cs @@ -182,6 +182,15 @@ internal async Task ReadEncryptionSaltIfNeededAsync(CancellationToken cancellati return; } + // A corrupt central directory can point the local header offset past the end of the + // archive. Seeking there throws ArgumentOutOfRangeException on some streams (e.g. + // MemoryStream). Mirror the check in IsOpenableInitialVerifications and defer the error + // to when the entry is actually opened, same as for non AES encrypted entries. + if (_offsetOfLocalHeader > _archive.ArchiveStream.Length) + { + return; + } + long savedPosition = _archive.ArchiveStream.Position; try { diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs index 4dd6b616f35bb6..0b53e9c9fb9d84 100644 --- a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs +++ b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs @@ -630,6 +630,15 @@ internal void ReadEncryptionSaltIfNeeded() return; } + // A corrupt central directory can point the local header offset past the end of the + // archive. Seeking there throws ArgumentOutOfRangeException on some streams (e.g. + // MemoryStream). Mirror the check in IsOpenableInitialVerifications and defer the error + // to when the entry is actually opened, same as for non AES encrypted entries. + if (_offsetOfLocalHeader > _archive.ArchiveStream.Length) + { + return; + } + long savedPosition = _archive.ArchiveStream.Position; try { diff --git a/src/libraries/System.IO.Compression/tests/ZipArchive/zip_InvalidParametersAndStrangeFiles.cs b/src/libraries/System.IO.Compression/tests/ZipArchive/zip_InvalidParametersAndStrangeFiles.cs index 43ce9d2f78b554..04e4c229ce3044 100644 --- a/src/libraries/System.IO.Compression/tests/ZipArchive/zip_InvalidParametersAndStrangeFiles.cs +++ b/src/libraries/System.IO.Compression/tests/ZipArchive/zip_InvalidParametersAndStrangeFiles.cs @@ -181,6 +181,55 @@ public static async Task ZipArchiveEntry_InvalidUpdate(string zipname, bool asyn await DisposeZipArchive(async, archive); } + [Theory] + [InlineData(false)] + [InlineData(true)] + public static async Task ReadCentralDirectory_AesEntryLocalHeaderOffsetPastEndOfStream_DoesNotThrowUnexpectedly(bool async) + { + // Take a valid AES-encrypted archive and corrupt the first central directory record so its + // local header offset points past the end of the stream. Eagerly reading the AES salt while + // parsing the central directory must not leak an ArgumentOutOfRangeException from seeking + // there; the error is deferred until the entry is actually opened, matching the behavior of + // non-encrypted entries. + byte[] bytes; + using (LocalMemoryStream original = await LocalMemoryStream.ReadAppFileAsync(passwordProtected("PasswordProtected_DifferentPasswords.zip"))) + { + bytes = original.ToArray(); + } + + // Locate the central directory via the end-of-central-directory record (rather than + // searching for a signature, which could match coincidental bytes inside encrypted data), + // then overwrite the first record's 4-byte relative-offset-of-local-header field (at offset 42 + // of the 46-byte header) with a value larger than int.MaxValue. MemoryStream.Seek throws + // ArgumentOutOfRangeException for offsets above int.MaxValue, which is the crash being guarded. + ReadOnlySpan endOfCentralDirectorySignature = [0x50, 0x4B, 0x05, 0x06]; + ReadOnlySpan centralDirectorySignature = [0x50, 0x4B, 0x01, 0x02]; + const int OffsetOfStartOfCentralDirectoryPosition = 16; + const int RelativeOffsetOfLocalHeaderPosition = 42; + + int endOfCentralDirectory = bytes.AsSpan().LastIndexOf(endOfCentralDirectorySignature); + Assert.True(endOfCentralDirectory >= 0); + int centralDirectoryStart = (int)BinaryPrimitives.ReadUInt32LittleEndian( + bytes.AsSpan(endOfCentralDirectory + OffsetOfStartOfCentralDirectoryPosition)); + Assert.True(bytes.AsSpan(centralDirectoryStart).StartsWith(centralDirectorySignature)); + BinaryPrimitives.WriteUInt32LittleEndian( + bytes.AsSpan(centralDirectoryStart + RelativeOffsetOfLocalHeaderPosition), 0xF0000000); + + using MemoryStream stream = new MemoryStream(bytes); + ZipArchive archive = await CreateZipArchive(async, stream, ZipArchiveMode.Read); + + // Enumerating the entries parses the central directory and eagerly reads AES salts. For the + // corrupted offset this must not throw ArgumentOutOfRangeException from seeking past the end. + ZipArchiveEntry corruptedEntry = archive.Entries[0]; + Assert.True(corruptedEntry.IsEncrypted); + + // The error is deferred: opening the corrupted entry throws InvalidDataException, just like a + // non-encrypted entry whose local header offset points past the end of the stream. + await Assert.ThrowsAsync(() => OpenEntryStream(async, corruptedEntry)); + + await DisposeZipArchive(async, archive); + } + [Theory] [MemberData(nameof(Get_Booleans_Data))] public static async Task LargeArchive_DataDescriptor_Read_NonZip64_FileLengthGreaterThanIntMax(bool async)