Issue 782191  has been merged into this issue.

This seems to have started since the time when added decryption by Zip Archiver.
2d011c2ad69ad5e2638657f83a193fd052959832
https://chromium-review.googlesource.com/569321

This happens when the encryption method is not AES.
When it is AES, the Files app. asks password again.
> 7za u -tzip -pa -mem=AES256 aes.zip a.txt

On the other hand, when it is zipcrypto, both unzOpenCurrentFilePassword and unzReadCurrentFile returned success (UNZ_OK) result even when wrong password was passed.

I've found unzOpenCurrentFile3 in unzip.c can return BAD_PASSWORD only when the encryption method is AES. So I think Zip Archiver should not expect that value for wrong password input at here.
https://cs.chromium.org/chromium/src/chrome/browser/resources/chromeos/zip_archiver/cpp/volume_archive_minizip.cc?q=unzOpenCurrentFilePassword&sq=package:chromium&l=350

Tentatively assigning to yawano@ as he would have more background on this.

I've found that the MiniUnz, demo of zLib + Unz package (miniunz_exec target) does the same.

If wrong password is entered for zipcrypto-encrypted file,
> error -105 with zipfile in unzCloseCurrentFile
(-105: UNZ_CRCERROR)

When AES:
> error -106 with zipfile in unzOpenCurrentFilePassword
> error -105 with zipfile in unzCloseCurrentFile
> error -106 with zipfile in unzGoToNextFile
(-106: UNZ_WRONG_PASSWORD)

So I think Zip Archiver should also handle the error at unzCloseCurrentFile.
However, as far as I saw, unzCloseCurrentFile itself is not called when reading a file with Zip Archiver in ChromeOS.

At the API level, libarchive (which is used by ZIP unpacker) has a callback mechanism for requiring a password when failed, but minizip doesn't.
https://github.com/libarchive/libarchive/wiki/ManPageArchiveReadAddPassphrase3
https://github.com/libarchive/libarchive/blob/master/libarchive/archive_read_support_format_zip.c#L1738
https://github.com/libarchive/libarchive/blob/master/libarchive/archive_read_support_format_zip.c#L1802
Based on this, libarchive seems to support AES and Traditional PKWARE.

Instead it returns error value upon file open -- but only if it's AES. Othrewise it's ignored.


One possible fix is to add implementation of PKWARE's decryption and verification procedure into minizip.

https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT
>    After the header is decrypted,  the last 1 or 2 bytes in Buffer
>    should be the high-order word/byte of the CRC for the file being
>    decrypted, stored in Intel low-byte/high-byte order.  Versions of
>    PKZIP prior to 2.0 used a 2 byte CRC check; a 1 byte CRC check is
>    used on versions after 2.0.  This can be used to test if the password
>    supplied is correct or not.

For reference,
The file which I attached in the first report (encrypted.zip) was made with Zip 3.0 (July 5th 2008) by Info-ZIP. It's ZipCrypto Store, with version 10.
On the other hand, this attached file (p7zip.zip) was made with p7zip Version 9.20 and ZipCryptoStore with version 20.

Deleted: p7zip.zip 300 bytes

p7zip.zip 300 bytes Download

1. Latest version of Minizip has evolved much (change of interface, filename and directory change) since the revision which is currently used by Zip Archiver. We will need to follow what change first.
2. As described before, we need to add code to verify the password for Traditional PKWARE.

3. I once tried the verification of the CRC32 based on the first 12 bytes as in the application note:

<For the current revision (e07e141475220196b55294c8172b274cc32d642d) being used by ZipArchive>
diff --git a/unzip.c b/unzip.c
index 24f52a5..67cc169 100644
--- a/unzip.c
+++ b/unzip.c
@@ -1256,6 +1256,11 @@ extern int ZEXPORT unzOpenCurrentFile3(unzFile file, int 
*method, int *level, in
 
             for (i = 0; i < 12; i++)
                 zdecode(s->keys, s->pcrc_32_tab, source[i]);
+            int from_header = s->pfile_in_zip_read->crc32_wait >> 24;
+            int actual = source[11] & 0xff;
+            if (from_header != actual) {
+              return UNZ_BADPASSWORD;
+            }
 
             s->pfile_in_zip_read->rest_read_compressed -= 12;
             s->pfile_in_zip_read->pos_in_zipfile += 12;

It worked correctly with (p7zip.zip) attached above, but falsely rejected correct password for (encrypted.zip). I am investigating if this is specific to the Zip or the patch above is wrong. libarchive can handle this kind of file, so it's likely the latter.

Now I found how libarchive accepts [encrypted.zip].
When the bit 3 of the general purpose bit is set, it uses the higher 2 bytes of the last modified date, instead of the higher order bytes of the file's CRC32.

archive_read_support_format_zip.c in libarchive
>	if (zip_entry->zip_flags & ZIP_LENGTH_AT_END)
>		zip_entry->decdat = p[11];
>	else
>		zip_entry->decdat = p[17];

I am not 100% sure as I'm still new to this area, but I think the implementation above does not match with the statement in the application note https://pkware.cachefly.net/webdocs/APPNOTE/APPNOTE-1.0.txt :
> After the header is decrypted,  the last 1 or 2 bytes in Buffer
> should be the high-order word/byte of the CRC for the file being
> decrypted, stored in Intel low-byte/high-byte order.  Versions of
> PKZIP prior to 2.0 used a 2 byte CRC check; a 1 byte CRC check is
> used on versions after 2.0.  This can be used to test if the password
supplied is correct or not.

Anyways we'll implement the same algorithm to keep old behavior of the ZIP unpacker extension.

release target milestone changed.

The fix has been merged to the tip of 1.2 branch of minizip, 
4966c4d65f3ba591d8d4e501f1950bbfde0331fe
https://github.com/nmoinvaz/minizip/pull/210
however we still need to adjust minizip 1.2 branch to compile it on Linux (not BSD).

https://github.com/nmoinvaz/minizip/issues/175
> On Ubuntu, libbsd-dev is required for arc4random.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0c5fc82ad739d803fcb8ca79071767903843f465

commit 0c5fc82ad739d803fcb8ca79071767903843f465
Author: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Date: Wed Dec 27 12:58:20 2017

Zip Archiver: Handle wrong passwords input for Traditional PKZIP.

Changes the reference to third_party/minizip to newer revision, which
includes this change:

"Verify decrypted header to detect wrong password for Traditional PKZIP."
https://chromium.googlesource.com/external/github.com/nmoinvaz/minizip/+/0c1b4ea6a9735ba0588998c2aa1508eedfefdefb

Bug:  782197 
Test: manually tested as noted in the bug
Change-Id: Iec8c45a01ea2e515619928e858beb131ef9b247d
Reviewed-on: https://chromium-review.googlesource.com/844397
Reviewed-by: Tomasz Mikolajewski <mtomasz@chromium.org>
Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526220}
[modify] https://crrev.com/0c5fc82ad739d803fcb8ca79071767903843f465/DEPS
[modify] https://crrev.com/0c5fc82ad739d803fcb8ca79071767903843f465/third_party/minizip/BUILD.gn
[modify] https://crrev.com/0c5fc82ad739d803fcb8ca79071767903843f465/third_party/minizip/README.chromium