Simplify password checks. No need to persist result

3 files changed, 37 insertions, 46 deletions

diff --git a/generic/tclZipfs.c b/generic/tclZipfs.c
index b342c05..a8dff30 100644
--- a/generic/tclZipfs.c
+++ b/generic/tclZipfs.c
@@ -229,13 +229,10 @@ typedef struct ZipEntry {
     int crc32;			/* CRC-32 as stored in ZIP */
     int timestamp;		/* Modification time */
     int isEncrypted;		/* True if data is encrypted */
-    unsigned short flags;
+    int 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 */
@@ -331,7 +328,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);
+static int		IsCryptHeaderValid(ZipEntry *z, unsigned char cryptHeader[12]);
 #if !defined(STATIC_BUILD)
 static int		ZipfsAppHookFindTclInit(const char *archive);
 #endif
@@ -735,25 +732,23 @@ CountSlashes(
 /*
  *------------------------------------------------------------------------
  *
- * ComputeCryptHeaderValidity --
+ * IsCryptHeaderValid --
  *
  *    Computes the validity of the encryption header CRC for a ZipEntry.
  *
  * Results:
- *    None.
+ *    Returns 1 if the header is valid else 0.
  *
  * 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.
+ *    None.
  *
  *------------------------------------------------------------------------
  */
-static void ComputeCryptHeaderValidity(ZipEntry *z)
+static int IsCryptHeaderValid(
+	ZipEntry *z, 
+	unsigned char cryptHeader[12]
+	)
 {
-    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
@@ -767,23 +762,20 @@ static void ComputeCryptHeaderValidity(ZipEntry *z)
      * reported as a corruption error instead of incorrect password.
      */
     int dosTime = ToDosTime(z->timestamp);
-    if (z->cryptHeaderCRC[1] == (unsigned char)(dosTime >> 8)) {
+    if (cryptHeader[11] == (unsigned char)(dosTime >> 8)) {
 	/* Infozip style - Tested with test-password.zip */
-	z->flags |= ZE_F_CRYPT_HDR_COMPARED | ZE_F_CRYPT_HDR_CORRECT;
-	return;
+	return 1;
     }
     /* 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 */
-	}
+	return (cryptHeader[11] == (unsigned char)(z->crc32 >> 24));
     }
+
+    /* No CRC, no way to verify. Assume valid */
+    return 1;
 }
-
+
 /*
  *-------------------------------------------------------------------------
  *
@@ -4999,17 +4991,10 @@ InitReadableChannel(
 	 * "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;
-	    }
+	if (!IsCryptHeaderValid(z, encheader)) {
+	    ZIPFS_ERROR(interp, "invalid password");
+	    ZIPFS_ERROR_CODE(interp, "PASSWORD");
+	    goto error_cleanup;
 	}
 	info->ubuf += i;
     }
diff --git a/tests/zipfiles/test-password2.zip b/tests/zipfiles/test-password2.zip
new file mode 100644
index 0000000..82bb732
--- /dev/null
+++ b/tests/zipfiles/test-password2.zip
diff --git a/tests/zipfs.test b/tests/zipfs.test
index 972790f..65f0ce7 100644
--- a/tests/zipfs.test
+++ b/tests/zipfs.test
@@ -1194,9 +1194,9 @@ namespace eval test_ns_zipfs {
 
     #
     # Password protected
-    proc testpassword {id filename password result args} {
+    proc testpassword {id zipfile filename password result args} {
         variable defaultMountPoint
-        set zippath [zippath test-password.zip]
+        set zippath [zippath $zipfile]
         test zipfs-password-read-$id "zipfs password read $id" -setup {
             unset -nocomplain fd
             if {$password ne ""} {
@@ -1217,15 +1217,21 @@ namespace eval test_ns_zipfs {
     }
     # The bug bug-bbe7c6ff9e only manifests on macos
     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 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 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 badpassword "invalid password" -returnCodes error
+
+    # NOTE: test-password.zip is the DOS time based encryption header validity check (infozip style)
+    # NOTE: test-password2.zip is the CRC based encryption header validity check (pkware style)
+    testpassword plain                  test-password.zip plain.txt password plaintext
+    testpassword plain-nopass           test-password.zip plain.txt "" plaintext
+    testpassword plain-badpass          test-password.zip plain.txt badpassword plaintext
+    testpassword cipher-1               test-password.zip cipher.bin password ciphertext -constraints bug-bbe7c6ff9e
+    testpassword cipher-2               test-password2.zip cipher.bin password ciphertext -constraints bug-bbe7c6ff9e
+    testpassword cipher-nopass-1        test-password.zip cipher.bin {} "decryption failed - no password provided" -returnCodes error
+    testpassword cipher-nopass-2        test-password2.zip cipher.bin {} "decryption failed - no password provided" -returnCodes error
+    testpassword cipher-badpass-1       test-password.zip cipher.bin badpassword "invalid password" -returnCodes error
+    testpassword cipher-badpass-2       test-password2.zip cipher.bin badpassword "invalid password" -returnCodes error
+    testpassword cipher-deflate         test-password.zip cipher-deflate.bin password [lseq 100] -constraints bug-bbe7c6ff9e
+    testpassword cipher-deflate-nopass  test-password.zip cipher-deflate.bin {} "decryption failed - no password provided" -returnCodes error
+    testpassword cipher-deflate-badpass test-password.zip cipher-deflate.bin badpassword "invalid password" -returnCodes error
 
     #
     # CRC errors