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

Can't open an AES-encrypted Zip archive with password set to empty string. Is this archive nonstandard? #471

Closed
michaelrgibbs opened this issue Jun 9, 2020 · 3 comments
Labels
question zip Related to ZIP file format

Comments

@michaelrgibbs
Copy link

michaelrgibbs commented Jun 9, 2020

Steps to reproduce

  1. Create a zip file with AES-256 encryption and an empty password. I have some created in the past with DotNetZip. I don't think they can be created with SharpZipLib.
  2. Create a ZipFile object from the zip file and set the Password field to "".
  3. Try to extract.

Expected behavior

The zip file is opened and extracted.

Actual behavior

The code acts as if no password has been set and throws exception "No password available for AES encrypted stream".

I see the setter code doesn't differentiate between null and empty string: https://github.com/icsharpcode/SharpZipLib/blob/master/src/ICSharpCode.SharpZipLib/Zip/ZipFile.cs#L360

7zip and DotNetZip allow these files to be extracted with an empty password, but I can't work out if they are just being more lenient than they need to be.

Version of SharpZipLib

1.2.0

@Numpsy Numpsy added question zip Related to ZIP file format labels Jun 9, 2020
@Numpsy
Copy link
Contributor

Numpsy commented Jun 9, 2020

Not sure why you'd want to be able to create an archive like that, but the WinZip AES spec doesn't seem to say you can't, so maybe something to handle in a read only fashion?

As fas as SharpZipLib goes:

The AES decryption machinery itself seems to handle it if you can get the empty string password into it, but the ZipCrypto machinery does require a value (that's the GenerateKeys call in the setter you link to).

Passing an empty value into GenerateKeys causes it to throw (hence the way the null/empty check works I suppose), but that wouldn't matter for AES decryption as that doesn't use that key, it just uses the password string directly.

Related observation: Doesn't the way that setter works mean that if you set the password to some string value, and then set it to null, that the rawPassword_ string will get left at the previous value?

The AES encryption setup @

also checks if the 'key' has been setup but then sets up the decryption using the password string instead of that, which is also maybe not correct...

Maybe the Password setter should always set rawPassword_ to the passed value, and CreateAndInitDecryptionStream for AES entries should just check for the string being non-null ?

@michaelrgibbs
Copy link
Author

I think it's a bug that creates them in the first place, but I want to support archives created in the past when the bug was present.

@piksel
Copy link
Member

piksel commented Jun 19, 2020

Yeah, as @Numpsy said, PBKDF2 (which is what is used to derive the key from the password) does not specify a minimum length. Even though it seems somewhat useless, that is not something for us to decide 😁
This should now be fixed as of #472

@piksel piksel closed this as completed Jun 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question zip Related to ZIP file format
Projects
None yet
Development

No branches or pull requests

3 participants