sharpcompress icon indicating copy to clipboard operation
sharpcompress copied to clipboard

RarRijndael throws NullReferenceException when there is no password

Open IS4Code opened this issue 3 years ago • 3 comments

Rar parsing (via RarReader, haven't tested other archive types) has a bug when the password is not set (null) and the archive is encrypted. When the header is encrypted too, it correctly throws CryptographicException, but when only the files are encrypted, MoveToNextEntry throws NullReferenceException, as it attempts to initialize the engine with a null password.

I could use a blank password, but this breaks encrypted headers, as it cannot determine the format. I would prefer a CryptographicException thrown when an encrypted file is opened without a password (but its entry should be accessible).

Ideally, there should be some sort of wrong password detection as well, but fixing the first bug is completely sufficient.

IS4Code avatar May 15 '21 21:05 IS4Code

I'll accept a PR for this.

I think there isn't a good way (or used not to be) to detect encrypted files so an explicit exception wasn't done at the time.

adamhathcock avatar Jun 04 '21 12:06 adamhathcock

I noticed this issue and found that checking IsEncrypted on the Entry was the best method. If you're not using passwords then you should not attempt to process an Entry with IsEncrypted is True.

If the archive is encrypted with encrypted filenames then catching SharpCompresses CryptographicException when no password is set indicates it's passworded. #663 aligns 7zip to this behaviour also.

I've only tested this behaviour with Archive Open(), but is should be the same for RarReader too.

Nanook avatar May 04 '22 10:05 Nanook

I noticed this issue and found that checking IsEncrypted on the Entry was the best method. If you're not using passwords then you should not attempt to process an Entry with IsEncrypted is True.

If the archive is encrypted with encrypted filenames then catching SharpCompresses CryptographicException when no password is set indicates it's passworded. #663 aligns 7zip to this behaviour also.

I've only tested this behaviour with Archive Open(), but is should be the same for RarReader too.

Yes, it makes sense not to open an encrypted entry, but as I said, MoveToNextEntry itself throws the exception. Iterating the entries and retrieving their metadata should be still valid even without a password.

IS4Code avatar May 04 '22 15:05 IS4Code