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

.net core zip+aes ReadAsync failed on one of files #572

Open
ShadowTeolog opened this issue Feb 11, 2021 · 5 comments
Open

.net core zip+aes ReadAsync failed on one of files #572

ShadowTeolog opened this issue Feb 11, 2021 · 5 comments

Comments

@ShadowTeolog
Copy link

ShadowTeolog commented Feb 11, 2021

Steps to reproduce

1.open archive with password "TestStorage"
2.try extract file "[www/autopilotandbridge/home.png]" using

var entry = _zf.GetEntry(path);
var zipStream = _zf.GetInputStream(entry);
var buffer = new byte[entry.Size];
var result = await zipStream.ReadAsync(buffer, 0, buffer.Length);

Expected behavior

file is extracted

Actual behavior

Got exception

TransformFinalBlock is not implemented and inputCount is greater than 0
   at ICSharpCode.SharpZipLib.Encryption.ZipAESTransform.TransformFinalBlock(Byte[] inputBuffer, Int32 inputOffset, Int32 inputCount)
   at System.Security.Cryptography.CryptoStream.<ReadAsyncCore>d__43.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.ConfiguredTaskAwaitable`1.ConfiguredTaskAwaiter.GetResult()
   at System.Security.Cryptography.CryptoStream.<ReadAsyncInternal>d__38.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
   at ResourceCollection.LazyZipDirectoryLoader.

Version of SharpZipLib

Obtained from (only keep the relevant lines)

@Numpsy
Copy link
Contributor

Numpsy commented Feb 11, 2021

It looks like calling ReadAsync on the CryptoStream in .NETCore 3.1 causes it to go through an internal code path that tries to do the crypto transform work itself, rather than using the Read() implementation in ZipAESStream (on .NET 4.6, it does seem to call through to Read to do the work).

That implementation calls ZipAESTransform.TransformFinalBlock(), which isn't really implemented (it doesn't need to do anything with the Read impl in ZipAesStream.)

TransformFinalBlock is called with a inputCount of 28, which is (presumably not cooincidentaly) the combined size of the AES header and auth block in the encrypted stream.

A few quick thoughts:

  1. We could possibly give ZipAESStream an implementation of ReadAsync() so the version in CryptoStream isn't called.
  2. We could maybe give the CryptoStream constructor a view of the base stream that only includes the actual encrypted data, and not the header and auth code parts (so they aren't included in any size calculations)?
  3. Review the way the transforms are handled and maybe to the final block/auth code handling in way that better fits in with what CryptoStream wants.

@piksel
Copy link
Member

piksel commented Feb 12, 2021

Yeah, 3 is probably the "right" way, and 1 is probably the "safest" way.

If that wasn't clear @ShadowTeolog, your code should work if you use the non-async read:

var result = zipStream.Read(buffer, 0, buffer.Length);

@ShadowTeolog
Copy link
Author

Yes, sync read work flawless.
But this should be fixed somehow anyway or throw exception on any attempt to ReadAsync on ZIP+AES. Too many people will be puzzled by an error like: (only this file out of a hundred cannot be read), a lot of projects will be moved to .net 5.0 from .net Framework

@Numpsy
Copy link
Contributor

Numpsy commented Feb 14, 2021

only this file out of a hundred cannot be read

I think it's only currently an issue with stored entries (because for deflated entries, the call to ReadAsync goes through InflatorInputStream, which itself only makes sync calls to the base stream, whereas for Stored entries, it does async reads on ZipAESStream).

@Numpsy
Copy link
Contributor

Numpsy commented May 8, 2021

@ShadowTeolog You should no longer get this error with the new 1.3.2 release, though the stream reads won't actually be async yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants