From 9375cacd65f812d604e0775d9891e69a2415ee1e Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Thu, 28 Sep 2023 11:02:02 +0000 Subject: Start on password related zipfs bugs --- generic/tclZipfs.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++---- tests/zipfiles/README | 2 +- tests/zipfs.test | 6 ++-- 3 files changed, 87 insertions(+), 10 deletions(-) diff --git a/generic/tclZipfs.c b/generic/tclZipfs.c index 213ae23..b342c05 100644 --- a/generic/tclZipfs.c +++ b/generic/tclZipfs.c @@ -229,10 +229,13 @@ typedef struct ZipEntry { int crc32; /* CRC-32 as stored in ZIP */ int timestamp; /* Modification time */ int isEncrypted; /* True if data is encrypted */ - int flags; -#define ZE_F_CRC_COMPARED 0x00000001 /* If 1, the CRC has been compared. */ -#define ZE_F_CRC_CORRECT 0x00000002 /* Only meaningful if ZE_F_CRC_COMPARED is 1 */ -#define ZE_F_VOLUME 0x00000004 /* Entry corresponds to //zipfs:/ */ + unsigned short flags; +#define ZE_F_CRC_COMPARED 0x0001 /* If 1, the CRC has been compared. */ +#define ZE_F_CRC_CORRECT 0x0002 /* Only meaningful if ZE_F_CRC_COMPARED is 1 */ +#define ZE_F_VOLUME 0x0004 /* Entry corresponds to //zipfs:/ */ +#define ZE_F_CRYPT_HDR_COMPARED 0x0008 /* If 1, the encryption header CRC has been compared. */ +#define ZE_F_CRYPT_HDR_CORRECT 0x0010 /* Only meaningful if ZE_F_CRYPT_HDR_COMPARED is 1 */ + unsigned char cryptHeaderCRC[2];/* May be one or two bytes depending on zip version */ unsigned char *data; /* File data if written */ struct ZipEntry *next; /* Next file in the same archive */ struct ZipEntry *tnext; /* Next top-level dir in archive */ @@ -328,6 +331,7 @@ static void SerializeLocalEntryHeader( const unsigned char *start, const unsigned char *end, unsigned char *buf, ZipEntry *z, int nameLength, int align); +static void ComputeCryptHeaderValidity(ZipEntry *z); #if !defined(STATIC_BUILD) static int ZipfsAppHookFindTclInit(const char *archive); #endif @@ -544,7 +548,7 @@ ZipWriteShort( ptr[0] = value & 0xff; ptr[1] = (value >> 8) & 0xff; } - + /* *------------------------------------------------------------------------- * @@ -729,6 +733,58 @@ CountSlashes( } /* + *------------------------------------------------------------------------ + * + * ComputeCryptHeaderValidity -- + * + * Computes the validity of the encryption header CRC for a ZipEntry. + * + * Results: + * None. + * + * Side effects: + * Sets the flags bits of the ZipEntry based on whether the validity + * could be computed and if so, whether the encryption header was valid. + * + *------------------------------------------------------------------------ + */ +static void ComputeCryptHeaderValidity(ZipEntry *z) +{ + if (z->flags & ZE_F_CRYPT_HDR_COMPARED) { + /* Already computed */ + return; + } + /* + * There are multiple possibilities. The last one or two bytes of the + * encryption header should match the last one or two bytes of the + * CRC of the file. Or the last byte of the encryption header should + * be the high order byte of the file time. Depending on the archiver + * and version, any of the might be in used. We follow libzip in checking + * only one byte against both the crc and the time. Note that by design + * the check generates high number of false positives in any case. + * Also, in case a check is passed when it should not, the final CRC + * calculation will (should) catch it. Only difference is it will be + * reported as a corruption error instead of incorrect password. + */ + int dosTime = ToDosTime(z->timestamp); + if (z->cryptHeaderCRC[1] == (unsigned char)(dosTime >> 8)) { + /* Infozip style - Tested with test-password.zip */ + z->flags |= ZE_F_CRYPT_HDR_COMPARED | ZE_F_CRYPT_HDR_CORRECT; + return; + } + /* DOS time did not match, may be CRC does */ + if (z->crc32) { + /* Pkware style - Tested with test-password2.zip */ + if (z->cryptHeaderCRC[1] == (unsigned char)(z->crc32 >> 24)) { + z->flags |= ZE_F_CRYPT_HDR_COMPARED | ZE_F_CRYPT_HDR_CORRECT; + } + else { + z->flags |= ZE_F_CRYPT_HDR_COMPARED; /* Compared and incorrect */ + } + } +} + +/* *------------------------------------------------------------------------- * * DecodeZipEntryText -- @@ -4930,9 +4986,30 @@ InitReadableChannel( passBuf[i] = '\0'; init_keys(passBuf, info->keys, crc32tab); memset(passBuf, 0, sizeof(passBuf)); + unsigned char encheader[12]; + memcpy(encheader, info->ubuf, 12); for (i = 0; i < 12; i++) { ch = info->ubuf[i]; - zdecode(info->keys, crc32tab, ch); + ch ^= decrypt_byte(info->keys, crc32tab); + encheader[i] = ch; + update_keys(info->keys, crc32tab, ch); + } + /* + * Validate the encryption key. This is tricky thanks to multiple + * "standards" as to where the checksum for the encryption header + * is + */ + z->cryptHeaderCRC[0] = encheader[10]; + z->cryptHeaderCRC[1] = encheader[11]; + ComputeCryptHeaderValidity(z); + if (z->flags & ZE_F_CRYPT_HDR_COMPARED) { + /* We had enough information to compare the encryption CRC */ + if (!(z->flags & ZE_F_CRYPT_HDR_CORRECT)) { + /* And it was found to be invalid */ + ZIPFS_ERROR(interp, "invalid password"); + ZIPFS_ERROR_CODE(interp, "PASSWORD"); + goto error_cleanup; + } } info->ubuf += i; } diff --git a/tests/zipfiles/README b/tests/zipfiles/README index 38ce998..a036635 100644 --- a/tests/zipfiles/README +++ b/tests/zipfiles/README @@ -1,7 +1,7 @@ The files in this directory are used for testing zipfs file systems. They fall under the following licenses: -test-overlay.zip, test-password.zip, test-zip-in-zip.zip - Tcl's license +test-overlay.zip, test-password[2].zip, test-zip-in-zip.zip - Tcl's license All other files - test files from libzip (https://libzip.org) and are covered by the license in LICENSE-libzip. \ No newline at end of file diff --git a/tests/zipfs.test b/tests/zipfs.test index 705f26f..972790f 100644 --- a/tests/zipfs.test +++ b/tests/zipfs.test @@ -1219,13 +1219,13 @@ namespace eval test_ns_zipfs { testConstraint bug-bbe7c6ff9e [expr {$::tcl_platform(os) ne "Darwin"}] testpassword plain plain.txt password plaintext testpassword plain-nopassword plain.txt "" plaintext - testpassword plain-badpassword plain.txt xxx plaintext + testpassword plain-badpassword plain.txt badpassword plaintext testpassword cipher cipher.bin password ciphertext -constraints bug-bbe7c6ff9e testpassword cipher-nopassword cipher.bin {} "decryption failed - no password provided" -returnCodes error - testpassword cipher-badpassword cipher.bin xxx "invalid CRC" -returnCodes error + testpassword cipher-badpassword cipher.bin badpassword "invalid password" -returnCodes error testpassword cipher-deflate cipher-deflate.bin password [lseq 100] -constraints bug-bbe7c6ff9e testpassword cipher-deflate-nopassword cipher-deflate.bin {} "decryption failed - no password provided" -returnCodes error - testpassword cipher-deflate-badpassword cipher-deflate.bin xxx "decompression error" -returnCodes error + testpassword cipher-deflate-badpassword cipher-deflate.bin badpassword "invalid password" -returnCodes error # # CRC errors -- cgit v0.12