Skip to content

Fix compression method for encrypted empty files and directories when updating a ZipFile #902

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
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix compression method for encrypted empty files and directories when updating a ZipFile #902

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
76 changes: 41 additions & 35 deletions src/ICSharpCode.SharpZipLib/Zip/ZipFile.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,9 +398,9 @@ private bool HaveKeys
/// <exception cref="ZipException">
/// The file doesn't contain a valid zip archive.
/// </exception>
public ZipFile(string name) :
this(name, null)
{
public ZipFile(string name) :
this(name, null)
{

}

Expand Down Expand Up @@ -534,10 +534,10 @@ public ZipFile(Stream stream) :
/// <exception cref="ArgumentNullException">
/// The <see cref="Stream">stream</see> argument is null.
/// </exception>
public ZipFile(Stream stream, bool leaveOpen) :
this(stream, leaveOpen, null)
{
public ZipFile(Stream stream, bool leaveOpen) :
this(stream, leaveOpen, null)
{

}

/// <summary>
Expand Down Expand Up @@ -789,7 +789,8 @@ public Encoding ZipCryptoEncoding
/// <inheritdoc cref="Zip.StringCodec"/>
public StringCodec StringCodec
{
set {
set
{
_stringCodec = value;
if (!isNewArchive_)
{
Expand Down Expand Up @@ -1182,7 +1183,7 @@ private long TestLocalHeader(ZipEntry entry, HeaderTest tests)
bool testData = (tests & HeaderTest.Extract) != 0;

var entryAbsOffset = offsetOfFirstEntry + entry.Offset;

baseStream_.Seek(entryAbsOffset, SeekOrigin.Begin);
var signature = (int)ReadLEUint();

Expand Down Expand Up @@ -1258,9 +1259,9 @@ private long TestLocalHeader(ZipEntry entry, HeaderTest tests)
throw new ZipException($"Version required to extract this entry not supported ({extractVersion})");
}

const GeneralBitFlags notSupportedFlags = GeneralBitFlags.Patched
| GeneralBitFlags.StrongEncryption
| GeneralBitFlags.EnhancedCompress
const GeneralBitFlags notSupportedFlags = GeneralBitFlags.Patched
| GeneralBitFlags.StrongEncryption
| GeneralBitFlags.EnhancedCompress
| GeneralBitFlags.HeaderMasked;
if (localFlags.HasAny(notSupportedFlags))
{
Expand Down Expand Up @@ -1677,8 +1678,8 @@ public void CommitUpdate()
{
// Create an empty archive if none existed originally.
if (entries_.Length != 0) return;
byte[] theComment = (newComment_ != null)
? newComment_.RawComment
byte[] theComment = (newComment_ != null)
? newComment_.RawComment
: _stringCodec.ZipArchiveCommentEncoding.GetBytes(comment_);
ZipFormat.WriteEndOfCentralDirectory(baseStream_, 0, 0, 0, theComment);
}
Expand Down Expand Up @@ -2165,7 +2166,12 @@ private void WriteLocalEntryHeader(ZipUpdate update)
// No need to compress - no data.
entry.CompressedSize = entry.Size;
entry.Crc = 0;
entry.CompressionMethod = CompressionMethod.Stored;

// Crypted files should be deflated, even with no data (such as directories)
if (!HaveKeys)
{
entry.CompressionMethod = CompressionMethod.Stored;
}
}
}
else if (entry.CompressionMethod == CompressionMethod.Stored)
Expand Down Expand Up @@ -2583,15 +2589,15 @@ private void CopyBytes(ZipUpdate update, Stream destination, Stream source,
/// <returns>The descriptor size, zero if there isn't one.</returns>
private static int GetDescriptorSize(ZipUpdate update, bool includingSignature)
{
if (!((GeneralBitFlags)update.Entry.Flags).HasAny(GeneralBitFlags.Descriptor))
if (!((GeneralBitFlags)update.Entry.Flags).HasAny(GeneralBitFlags.Descriptor))
return 0;
var descriptorWithSignature = update.Entry.LocalHeaderRequiresZip64
? ZipConstants.Zip64DataDescriptorSize

var descriptorWithSignature = update.Entry.LocalHeaderRequiresZip64
? ZipConstants.Zip64DataDescriptorSize
: ZipConstants.DataDescriptorSize;

return includingSignature
? descriptorWithSignature
return includingSignature
? descriptorWithSignature
: descriptorWithSignature - sizeof(int);
}

Expand Down Expand Up @@ -2878,7 +2884,7 @@ private void CopyEntryDirect(ZipFile workFile, ZipUpdate update, ref long destin

// Clumsy way of handling retrieving the original name and extra data length for now.
// TODO: Stop re-reading name and data length in CopyEntryDirect.

uint nameLength = ReadLEUshort();
uint extraLength = ReadLEUshort();

Expand Down Expand Up @@ -3013,7 +3019,7 @@ private void UpdateCommentOnly()
}
finally
{
if(updateFile != baseStream_)
if (updateFile != baseStream_)
updateFile.Dispose();
}

Expand Down Expand Up @@ -3178,7 +3184,7 @@ private void RunUpdates()
}

byte[] theComment = newComment_?.RawComment ?? _stringCodec.ZipArchiveCommentEncoding.GetBytes(comment_);
ZipFormat.WriteEndOfCentralDirectory(workFile.baseStream_, updateCount_,
ZipFormat.WriteEndOfCentralDirectory(workFile.baseStream_, updateCount_,
sizeEntries, centralDirOffset, theComment);

endOfStream = workFile.baseStream_.Position;
Expand Down Expand Up @@ -3520,7 +3526,7 @@ private ulong ReadLEUlong()
#endregion Reading

// NOTE this returns the offset of the first byte after the signature.
private long LocateBlockWithSignature(int signature, long endLocation, int minimumBlockSize, int maximumVariableData)
private long LocateBlockWithSignature(int signature, long endLocation, int minimumBlockSize, int maximumVariableData)
=> ZipFormat.LocateBlockWithSignature(baseStream_, signature, endLocation, minimumBlockSize, maximumVariableData);

/// <summary>
Expand Down Expand Up @@ -3578,14 +3584,14 @@ private void ReadEntries()
}

bool isZip64 = false;

// Check if zip64 header information is required.
bool requireZip64 = thisDiskNumber == 0xffff ||
startCentralDirDisk == 0xffff ||
entriesForThisDisk == 0xffff ||
entriesForWholeCentralDir == 0xffff ||
centralDirSize == 0xffffffff ||
offsetOfCentralDir == 0xffffffff;
startCentralDirDisk == 0xffff ||
entriesForThisDisk == 0xffff ||
entriesForWholeCentralDir == 0xffff ||
centralDirSize == 0xffffffff ||
offsetOfCentralDir == 0xffffffff;

// #357 - always check for the existence of the Zip64 central directory.
// #403 - Take account of the fixed size of the locator when searching.
Expand Down Expand Up @@ -3676,7 +3682,7 @@ private void ReadEntries()
int extraLen = ReadLEUshort();
int commentLen = ReadLEUshort();


// ReSharper disable once UnusedVariable, Currently unused but needs to be read to offset the stream
int diskStartNo = ReadLEUshort();
// ReSharper disable once UnusedVariable, Currently unused but needs to be read to offset the stream
Expand Down Expand Up @@ -3773,9 +3779,9 @@ private Stream CreateAndInitDecryptionStream(Stream baseStream, ZipEntry entry)
int saltLen = entry.AESSaltLen;
byte[] saltBytes = new byte[saltLen];
int saltIn = StreamUtils.ReadRequestedBytes(baseStream, saltBytes, offset: 0, saltLen);

if (saltIn != saltLen) throw new ZipException($"AES Salt expected {saltLen} git {saltIn}");

byte[] pwdVerifyRead = new byte[2];
StreamUtils.ReadFully(baseStream, pwdVerifyRead);
int blockSize = entry.AESKeySize / 8; // bits to bytes
Expand Down Expand Up @@ -3819,7 +3825,7 @@ private Stream CreateAndInitDecryptionStream(Stream baseStream, ZipEntry entry)
private Stream CreateAndInitEncryptionStream(Stream baseStream, ZipEntry entry)
{
if (entry.Version >= ZipConstants.VersionStrongEncryption &&
entry.HasFlag(GeneralBitFlags.StrongEncryption)) return null;
entry.HasFlag(GeneralBitFlags.StrongEncryption)) return null;

var classicManaged = new PkzipClassicManaged();

Expand Down
48 changes: 48 additions & 0 deletions test/ICSharpCode.SharpZipLib.Tests/Zip/GeneralHandling.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -832,5 +832,53 @@ public void PasswordCheckingWithDateInExtraData()
Assert.AreEqual(checkTime.DateTime, uno.DateTime);
}
}

[Test]
[Category("Zip")]
[Category("Encryption")]
public void ReadingEncryptedZipWithDirectories()
{
var ms = new MemoryStream();
using (ZipOutputStream outStream = new ZipOutputStream(ms))
{
outStream.IsStreamOwner = false;
outStream.Password = "testPassword123";
outStream.SetLevel(5);

var entry = new ZipEntry("Folder/");
outStream.PutNextEntry(entry);

entry = new ZipEntry("Folder/File.txt");
outStream.PutNextEntry(entry);
byte[] fileData = Encoding.UTF8.GetBytes("This is some test data");
outStream.Write(fileData, 0, fileData.Length);
}

ms.Seek(0, SeekOrigin.Begin);
using (ZipFile zipFile = new ZipFile(ms))
{
zipFile.BeginUpdate();
zipFile.IsStreamOwner = false;
zipFile.Password = "testPassword123";
zipFile.AddDirectory("Folder2/");
zipFile.Add(new MemoryDataSource(Encoding.UTF8.GetBytes("Test content")), "Folder2/File2.txt");
zipFile.CommitUpdate();
}

ms.Seek(0, SeekOrigin.Begin);
using (var inStream = new ZipInputStream(ms))
{
inStream.IsStreamOwner = false;
inStream.Password = "testPassword123";
ZipEntry fileEntry = inStream.GetNextEntry();
Assert.AreEqual("Folder/", fileEntry.Name);
fileEntry = inStream.GetNextEntry();
Assert.AreEqual("Folder/File.txt", fileEntry.Name);
fileEntry = inStream.GetNextEntry();
Assert.AreEqual("Folder2/", fileEntry.Name);
fileEntry = inStream.GetNextEntry();
Assert.AreEqual("Folder2/File2.txt", fileEntry.Name);
}
}
}
}