Expand zipfs tests. Fix [9a80630571], [6ed3447a7e], [01d8f30342] and manpages.

1 files changed, 137 insertions, 47 deletions

diff --git a/generic/tclZipfs.c b/generic/tclZipfs.c
index 49e1c00..6ed23f7 100644
--- a/generic/tclZipfs.c
+++ b/generic/tclZipfs.c
@@ -19,6 +19,8 @@
 #include "tclInt.h"
 #include "tclFileSystem.h"
 
+#include <assert.h>
+
 #ifndef _WIN32
 #include <sys/mman.h>
 #endif /* _WIN32*/
@@ -197,7 +199,8 @@ typedef struct ZipFile {
     size_t baseOffset;		/* Archive start */
     size_t passOffset;		/* Password start */
     size_t directoryOffset;	/* Archive directory start */
-    unsigned char passBuf[264];	/* Password buffer */
+    size_t directorySize;       /* Size of archive directory */
+    unsigned char passBuf[264]; /* Password buffer */
     size_t numOpen;		/* Number of open files on archive */
     struct ZipEntry *entries;	/* List of files in archive */
     struct ZipEntry *topEnts;	/* List of top-level dirs in archive */
@@ -217,8 +220,10 @@ typedef struct ZipEntry {
     char *name;			/* The full pathname of the virtual file */
     ZipFile *zipFilePtr;	/* The ZIP file holding this virtual file */
     size_t offset;		/* Data offset into memory mapped ZIP file */
-    int numBytes;		/* Uncompressed size of the virtual file */
-    int numCompressedBytes;	/* Compressed size of the virtual file */
+    int numBytes;		/* Uncompressed size of the virtual file.
+    				   -1 for zip64 */
+    int numCompressedBytes;	/* Compressed size of the virtual file.
+    				   -1 for zip64 */
     int compressMethod;		/* Compress method */
     int isDirectory;		/* Set to 1 if directory, or -1 if root */
     int depth;			/* Number of slashes in path. */
@@ -905,10 +910,16 @@ CanonicalPath(
 	break;
     default:
 	if (inZipfs) {
-	    Tcl_DStringSetLength(dsPtr, i + j + ZIPFS_VOLUME_LEN);
-	    path = Tcl_DStringValue(dsPtr);
-	    memcpy(path, ZIPFS_VOLUME, ZIPFS_VOLUME_LEN);
-	    memcpy(path + ZIPFS_VOLUME_LEN + i , tail, j);
+	    /* pathLen = zipfs vol len + root len + separator + tail len */
+	    Tcl_DStringInit(dsPtr);
+	    (void) Tcl_DStringAppend(dsPtr, ZIPFS_VOLUME, ZIPFS_VOLUME_LEN);
+	    if (i) {
+		(void) Tcl_DStringAppend(dsPtr, root, i);
+		if (root[i-1] != '/') {
+		    Tcl_DStringAppend(dsPtr, "/", 1);
+		}
+	    }
+	    path = Tcl_DStringAppend(dsPtr, tail, j);
 	} else {
 	    Tcl_DStringSetLength(dsPtr, i + j + 1);
 	    path = Tcl_DStringValue(dsPtr);
@@ -1216,11 +1227,13 @@ ZipFSFindTOC(
 	    zf->baseOffset = zf->passOffset = zf->length;
 	    return TCL_OK;
 	}
-	ZIPFS_ERROR(interp, "wrong end signature");
+	ZIPFS_ERROR(interp, "archive directory end signature not found");
 	ZIPFS_ERROR_CODE(interp, "END_SIG");
 	goto error;
     }
 
+    /* p -> End of Central Directory (EOCD) record at this point */
+
     /*
      * How many files in the archive? If that's bogus, we're done here.
      */
@@ -1237,37 +1250,71 @@ ZipFSFindTOC(
     }
 
     /*
-     * Where does the central directory start?
+     * The Central Directory (CD) is a series of Central Directory File
+     * Header (CDFH) records preceding the EOCD (but not necessarily
+     * immediately preceding). cdirZipOffset is the offset into the
+     * *archive* to the CD (first CDFH). The size of the CD is given by
+     * cdirSize. NOTE: offset into archive does NOT mean offset into
+     * (zf->data) as other data may precede the archive in the file.
      */
+    ptrdiff_t eocdDataOffset = p - zf->data;
+    unsigned int cdirZipOffset = ZipReadInt(start, end, p + ZIP_CENTRAL_DIRSTART_OFFS);
+    unsigned int cdirSize = ZipReadInt(start, end, p + ZIP_CENTRAL_DIRSIZE_OFFS);
 
-    q = zf->data + ZipReadInt(start, end, p + ZIP_CENTRAL_DIRSTART_OFFS);
-    p -= ZipReadInt(start, end, p + ZIP_CENTRAL_DIRSIZE_OFFS);
-    zf->baseOffset = zf->passOffset = (p>q) ? p - q : 0;
-    zf->directoryOffset = q - zf->data + zf->baseOffset;
-    if ((p < q) || (p < zf->data) || (p > zf->data + zf->length)
-	    || (q < zf->data) || (q > zf->data + zf->length)) {
+    /*
+     * As computed above, 
+     *    eocdDataOffset < zf->length.
+     * In addition, the following consistency checks must be met
+     * (1) cdirZipOffset <= eocdDataOffset (to prevent under flow in computation of (2))
+     * (2) cdirZipOffset + cdirSize <= eocdDataOffset. Else the CD will be overlapping
+     * the EOCD. Note this automatically means cdirZipOffset+cdirSize < zf->length.
+     */
+    if (!(cdirZipOffset <= eocdDataOffset &&
+	  cdirSize <= eocdDataOffset - cdirZipOffset)) {
 	if (!needZip) {
-	    zf->baseOffset = zf->passOffset = zf->length;
+	    /* Simply point to end od data */
+	    zf->directoryOffset = zf->baseOffset = zf->passOffset = zf->length;
 	    return TCL_OK;
 	}
-	ZIPFS_ERROR(interp, "archive directory not found");
+	ZIPFS_ERROR(interp, "archive directory truncated");
 	ZIPFS_ERROR_CODE(interp, "NO_DIR");
 	goto error;
     }
 
     /*
-     * Read the central directory.
+     * Calculate the offset of the CD in the *data*. If there was no extra
+     * "junk" preceding the archive, this would just be cdirZipOffset but
+     * otherwise we have to account for it.
      */
+    if (eocdDataOffset - cdirSize > cdirZipOffset) {
+	zf->baseOffset = eocdDataOffset - cdirSize - cdirZipOffset;
+    } else {
+	zf->baseOffset = 0;
+    }
+    zf->passOffset = zf->baseOffset;
+    zf->directoryOffset = cdirZipOffset + zf->baseOffset;
+    zf->directorySize = cdirSize;
 
-    q = p;
-    minoff = zf->length;
-    for (i = 0; i < zf->numFiles; i++) {
-	int pathlen, comlen, extra;
-	size_t localhdr_off = zf->length;
+    const unsigned char *const cdirStart = p - cdirSize; /* Start of CD */
+
+    /*
+     * Original pointer based validation replaced by simpler checks above.
+     * Ensure still holds. The assigments to p, q are only there for use in
+     * the asserts. May be removed at some future date.
+     */
+    q = zf->data + cdirZipOffset;
+    p -= cdirSize;
+    assert(!((p < q) || (p < zf->data) || (p > zf->data + zf->length) ||
+	    (q < zf->data) || (q > zf->data + zf->length)));
 
-	if (q + ZIP_CENTRAL_HEADER_LEN > end) {
-	    ZIPFS_ERROR(interp, "wrong header length");
-	    ZIPFS_ERROR_CODE(interp, "HDR_LEN");
+    /*
+     * Read the central directory.
+     */
+    minoff = zf->length;
+    for (q = cdirStart, i = 0; i < zf->numFiles; i++) {
+	if ((q-cdirStart) + ZIP_CENTRAL_HEADER_LEN > (ptrdiff_t)zf->directorySize) {
+	    ZIPFS_ERROR(interp, "truncated directory");
+	    ZIPFS_ERROR_CODE(interp, "TRUNC_DIR");
 	    goto error;
 	}
 	if (ZipReadInt(start, end, q) != ZIP_CENTRAL_HEADER_SIG) {
@@ -1275,11 +1322,13 @@ ZipFSFindTOC(
 	    ZIPFS_ERROR_CODE(interp, "HDR_SIG");
 	    goto error;
 	}
-	pathlen = ZipReadShort(start, end, q + ZIP_CENTRAL_PATHLEN_OFFS);
-	comlen = ZipReadShort(start, end, q + ZIP_CENTRAL_FCOMMENTLEN_OFFS);
-	extra = ZipReadShort(start, end, q + ZIP_CENTRAL_EXTRALEN_OFFS);
-	localhdr_off = ZipReadInt(start, end, q + ZIP_CENTRAL_LOCALHDR_OFFS);
- 	if (ZipReadInt(start, end, zf->data + zf->baseOffset + localhdr_off) != ZIP_LOCAL_HEADER_SIG) {
+	int pathlen = ZipReadShort(start, end, q + ZIP_CENTRAL_PATHLEN_OFFS);
+	int comlen = ZipReadShort(start, end, q + ZIP_CENTRAL_FCOMMENTLEN_OFFS);
+	int extra = ZipReadShort(start, end, q + ZIP_CENTRAL_EXTRALEN_OFFS);
+	size_t localhdr_off = ZipReadInt(start, end, q + ZIP_CENTRAL_LOCALHDR_OFFS);
+	const unsigned char *localP = zf->data + zf->baseOffset + localhdr_off;
+	if (localP > (p - ZIP_LOCAL_HEADER_LEN) ||
+	    ZipReadInt(start, end, localP) != ZIP_LOCAL_HEADER_SIG) {
 	    ZIPFS_ERROR(interp, "Failed to find local header");
 	    ZIPFS_ERROR_CODE(interp, "LCL_HDR");
 	    goto error;
@@ -1289,6 +1338,12 @@ ZipFSFindTOC(
 	}
 	q += pathlen + comlen + extra + ZIP_CENTRAL_HEADER_LEN;
     }
+    if ((q-cdirStart) < (ptrdiff_t) zf->directorySize) {
+	/* file count and dir size do not match */
+	ZIPFS_ERROR(interp, "short file count");
+	ZIPFS_ERROR_CODE(interp, "FILE_COUNT");
+	goto error;
+    }
 
     zf->passOffset = minoff + zf->baseOffset;
 
@@ -1457,14 +1512,15 @@ ZipMapArchive(
      * Determine the file size.
      */
 
-#   ifdef _WIN64
     readSuccessful = GetFileSizeEx(hFile, (PLARGE_INTEGER) &zf->length) != 0;
-#   else /* !_WIN64 */
-    zf->length = GetFileSize(hFile, 0);
-    readSuccessful = (zf->length != (size_t) INVALID_FILE_SIZE);
-#   endif /* _WIN64 */
-    if (!readSuccessful || (zf->length < ZIP_CENTRAL_END_LEN)) {
-	ZIPFS_POSIX_ERROR(interp, "invalid file size");
+    if (!readSuccessful) {
+	Tcl_WinConvertError(GetLastError());
+	ZIPFS_POSIX_ERROR(interp, "failed to retrieve file size");
+	return TCL_ERROR;
+    }
+    if (zf->length < ZIP_CENTRAL_END_LEN) {
+	Tcl_SetErrno(EINVAL);
+	ZIPFS_POSIX_ERROR(interp, "truncated file");
 	return TCL_ERROR;
     }
 
@@ -1475,12 +1531,14 @@ ZipMapArchive(
     zf->mountHandle = CreateFileMappingW(hFile, 0, PAGE_READONLY, 0,
 	    zf->length, 0);
     if (zf->mountHandle == INVALID_HANDLE_VALUE) {
+	Tcl_WinConvertError(GetLastError());
 	ZIPFS_POSIX_ERROR(interp, "file mapping failed");
 	return TCL_ERROR;
     }
     zf->data = (unsigned char *)
 	    MapViewOfFile(zf->mountHandle, FILE_MAP_READ, 0, 0, zf->length);
     if (!zf->data) {
+	Tcl_WinConvertError(GetLastError());
 	ZIPFS_POSIX_ERROR(interp, "file mapping failed");
 	return TCL_ERROR;
     }
@@ -1492,8 +1550,13 @@ ZipMapArchive(
      */
 
     zf->length = lseek(fd, 0, SEEK_END);
-    if (zf->length == ERROR_LENGTH || zf->length < ZIP_CENTRAL_END_LEN) {
-	ZIPFS_POSIX_ERROR(interp, "invalid file size");
+    if (zf->length == ERROR_LENGTH) {
+	ZIPFS_POSIX_ERROR(interp, "failed to retrieve file size");
+	return TCL_ERROR;
+    }
+    if (zf->length < ZIP_CENTRAL_END_LEN) {
+	Tcl_SetErrno(EINVAL);
+	ZIPFS_POSIX_ERROR(interp, "truncated file");
 	return TCL_ERROR;
     }
     lseek(fd, 0, SEEK_SET);
@@ -2116,7 +2179,7 @@ TclZipfs_MountBuffer(
 	zf->data = (unsigned char *) data;
 	zf->ptrToFree = NULL;
     }
-    if (ZipFSFindTOC(interp, 0, zf) != TCL_OK) {
+    if (ZipFSFindTOC(interp, 1, zf) != TCL_OK) {
 	return TCL_ERROR;
     }
     result = ZipFSCatalogFilesystem(interp, zf, mountPoint, NULL,
@@ -2348,9 +2411,13 @@ static int
 ZipFSRootObjCmd(
     TCL_UNUSED(void *),
     Tcl_Interp *interp,		/* Current interpreter. */
-    TCL_UNUSED(int) /*objc*/,
-    TCL_UNUSED(Tcl_Obj *const *)) /*objv*/
+    int objc,
+    Tcl_Obj *const *objv)
 {
+    if (objc != 1) {
+	Tcl_WrongNumArgs(interp, 1, objv, "");
+	return TCL_ERROR;
+    }
     Tcl_SetObjResult(interp, Tcl_NewStringObj(ZIPFS_VOLUME, -1));
     return TCL_OK;
 }
@@ -3581,7 +3648,7 @@ ZipFSLMkImgObjCmd(
     Tcl_Obj *originFile, *password;
 
     if (objc < 3 || objc > 5) {
-	Tcl_WrongNumArgs(interp, 1, objv, "outfile inlist ?password infile?");
+	Tcl_WrongNumArgs(interp, 1, objv, "outfile inlist ?password? ?infile?");
 	return TCL_ERROR;
     }
     if (Tcl_IsSafe(interp)) {
@@ -3722,6 +3789,7 @@ ZipFSInfoObjCmd(
 {
     char *filename;
     ZipEntry *z;
+    int ret;
 
     if (objc != 2) {
 	Tcl_WrongNumArgs(interp, 1, objv, "filename");
@@ -3740,11 +3808,21 @@ ZipFSInfoObjCmd(
 	Tcl_ListObjAppendElement(interp, result,
 		Tcl_NewWideIntObj(z->numCompressedBytes));
 	Tcl_ListObjAppendElement(interp, result, Tcl_NewWideIntObj(z->offset));
+	ret = TCL_OK;
+    } else {
+	Tcl_SetErrno(ENOENT);
+	if (interp) {
+	    Tcl_SetObjResult(
+		interp,
+		Tcl_ObjPrintf("path \"%s\" not found in any zipfs volume",
+			      filename));
+	}
+	ret = TCL_ERROR;
     }
     Unlock();
-    return TCL_OK;
+    return ret;
 }
-
+
 /*
  *-------------------------------------------------------------------------
  *
@@ -4343,6 +4421,14 @@ ZipChannelOpen(
 	goto error;
     }
 
+    if (z->numBytes < 0 || z->numCompressedBytes < 0 ||
+	z->offset >= z->zipFilePtr->length) {
+	/* Normally this should only happen for zip64. */
+	ZIPFS_ERROR(interp, "file size error (may be zip64)");
+	ZIPFS_ERROR_CODE(interp, "FILE_SIZE");
+	goto error;
+    }
+
     /*
      * Do we support opening the file that way?
      */
@@ -4601,7 +4687,8 @@ InitWritableChannel(
  * InitReadableChannel --
  *
  *	Assistant for ZipChannelOpen() that sets up a readable channel. It's
- *	up to the caller to actually register the channel.
+ *	up to the caller to actually register the channel. Caller should have
+ *	validated the passed ZipEntry (byte counts in particular)
  *
  * Returns:
  *	Tcl result code.
@@ -4628,6 +4715,9 @@ InitReadableChannel(
     info->ubuf = z->zipFilePtr->data + z->offset;
     info->isDirectory = z->isDirectory;
     info->isEncrypted = z->isEncrypted;
+
+    /* Caller must validate - bug [6ed3447a7e] */
+    assert(z->numBytes >= 0 && z->numCompressedBytes >= 0);
     info->numBytes = z->numBytes;
 
     if (info->isEncrypted) {