Skip to content
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.

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

Changes to AES zip handling to better handle stream read attempts only returning partial data #308

Merged
merged 2 commits into from
Jan 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
48 changes: 48 additions & 0 deletions src/ICSharpCode.SharpZipLib/Core/StreamUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,54 @@ static public void ReadFully(Stream stream, byte[] buffer, int offset, int count
}
}

/// <summary>
/// Read as much data as possible from a <see cref="Stream"/>", up to the requested number of bytes
/// </summary>
/// <param name="stream">The stream to read data from.</param>
/// <param name="buffer">The buffer to store data in.</param>
/// <param name="offset">The offset at which to begin storing data.</param>
/// <param name="count">The number of bytes of data to store.</param>
/// <exception cref="ArgumentNullException">Required parameter is null</exception>
/// <exception cref="ArgumentOutOfRangeException"><paramref name="offset"/> and or <paramref name="count"/> are invalid.</exception>
static public int ReadRequestedBytes(Stream stream, byte[] buffer, int offset, int count)
{
if (stream == null)
{
throw new ArgumentNullException(nameof(stream));
}

if (buffer == null)
{
throw new ArgumentNullException(nameof(buffer));
}

// Offset can equal length when buffer and count are 0.
if ((offset < 0) || (offset > buffer.Length))
{
throw new ArgumentOutOfRangeException(nameof(offset));
}

if ((count < 0) || (offset + count > buffer.Length))
{
throw new ArgumentOutOfRangeException(nameof(count));
}

int totalReadCount = 0;
while (count > 0)
{
int readCount = stream.Read(buffer, offset, count);
if (readCount <= 0)
{
break;
}
offset += readCount;
count -= readCount;
totalReadCount += readCount;
}

return totalReadCount;
}

/// <summary>
/// Copy the contents of one <see cref="Stream"/> to another.
/// </summary>
Expand Down
3 changes: 2 additions & 1 deletion src/ICSharpCode.SharpZipLib/Encryption/ZipAESStream.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System;
using System.IO;
using System.Security.Cryptography;
using ICSharpCode.SharpZipLib.Core;

namespace ICSharpCode.SharpZipLib.Encryption
{
Expand Down Expand Up @@ -78,7 +79,7 @@ public override int Read(byte[] buffer, int offset, int count)
_slideBufFreePos -= _slideBufStartPos; // Note the -=
_slideBufStartPos = 0;
}
int obtained = _stream.Read(_slideBuffer, _slideBufFreePos, lengthToRead);
int obtained = StreamUtils.ReadRequestedBytes(_stream, _slideBuffer, _slideBufFreePos, lengthToRead);
_slideBufFreePos += obtained;

// Recalculate how much data we now have
Expand Down
4 changes: 2 additions & 2 deletions src/ICSharpCode.SharpZipLib/Zip/ZipFile.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3523,12 +3523,12 @@ private Stream CreateAndInitDecryptionStream(Stream baseStream, ZipEntry entry)
}
int saltLen = entry.AESSaltLen;
byte[] saltBytes = new byte[saltLen];
int saltIn = baseStream.Read(saltBytes, 0, saltLen);
int saltIn = StreamUtils.ReadRequestedBytes(baseStream, saltBytes, 0, saltLen);
if (saltIn != saltLen)
throw new ZipException("AES Salt expected " + saltLen + " got " + saltIn);
//
byte[] pwdVerifyRead = new byte[2];
baseStream.Read(pwdVerifyRead, 0, 2);
StreamUtils.ReadFully(baseStream, pwdVerifyRead);
int blockSize = entry.AESKeySize / 8; // bits to bytes

var decryptor = new ZipAESTransform(rawPassword_, saltBytes, blockSize, false);
Expand Down
18 changes: 18 additions & 0 deletions test/ICSharpCode.SharpZipLib.Tests/TestSupport/Streams.cs
Original file line number Diff line number Diff line change
Expand Up @@ -453,4 +453,22 @@ protected override void Dispose(bool disposing)

#endregion Instance Fields
}

internal class SingleByteReadingStream : MemoryStream
{
/// <summary>
/// Initializes a new instance of the <see cref="SingleByteReadingStream"/> class.
/// </summary>
public SingleByteReadingStream()
{
}

public override int Read(byte[] buffer, int offset, int count)
{
if (count > 0)
count = 1;

return base.Read(buffer, offset, count);
}
}
}
32 changes: 32 additions & 0 deletions test/ICSharpCode.SharpZipLib.Tests/Zip/ZipEncryptionHandling.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Diagnostics;
using System.IO;
using System.Text;
using ICSharpCode.SharpZipLib.Tests.TestSupport;

namespace ICSharpCode.SharpZipLib.Tests.Zip
{
Expand Down Expand Up @@ -56,6 +57,37 @@ public void ZipFileAesDecryption()
}
}

[Test]
[Category("Encryption")]
[Category("Zip")]
public void ZipFileAesRead()
{
var password = "password";

using (var ms = new SingleByteReadingStream())
{
WriteEncryptedZipToStream(ms, password, 256);
ms.Seek(0, SeekOrigin.Begin);

var zipFile = new ZipFile(ms)
{
Password = password
};

foreach (ZipEntry entry in zipFile)
{
if (!entry.IsFile) continue;

using (var zis = zipFile.GetInputStream(entry))
using (var sr = new StreamReader(zis, Encoding.UTF8))
{
var content = sr.ReadToEnd();
Assert.AreEqual(DummyDataString, content, "Decompressed content does not match input data");
}
}
}
}

private static readonly string[] possible7zPaths = new[] {
// Check in PATH
"7z", "7za",
Expand Down