-- cgit v0.12 From 8934db4c07a97d71f9cb628460418ac9353486f2 Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Sat, 9 Sep 2023 07:51:42 +0000 Subject: Bug [6ed3447a7e]. Fix crash. Add length checks. --- generic/tclZipfs.c | 57 +++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 41 insertions(+), 16 deletions(-) diff --git a/generic/tclZipfs.c b/generic/tclZipfs.c index cfb0cf7..1d424b6 100644 --- a/generic/tclZipfs.c +++ b/generic/tclZipfs.c @@ -19,6 +19,8 @@ #include "tclInt.h" #include "tclFileSystem.h" +#include + #ifndef _WIN32 #include #endif /* _WIN32*/ @@ -217,8 +219,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. */ @@ -1216,7 +1220,7 @@ 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; } @@ -1250,7 +1254,7 @@ ZipFSFindTOC( 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; } @@ -1279,7 +1283,9 @@ ZipFSFindTOC( 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) { + 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; @@ -1457,14 +1463,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 +1482,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 +1501,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 +2130,7 @@ TclZipfs_MountBuffer( zf->data = 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, @@ -4344,6 +4358,13 @@ ZipChannelOpen( goto error; } + if (z->numBytes < 0 || z->numCompressedBytes < 0 || z->offset < 0) { + /* 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? */ @@ -4602,7 +4623,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. @@ -4629,6 +4651,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) { -- cgit v0.12 From 2b4f6b9dd0d044f507ee0d18f7af319d752e176b Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Sat, 9 Sep 2023 18:11:56 +0000 Subject: Improve test coverage for zipfs mount/unmount --- doc/zipfs.n | 4 + generic/tclZipfs.c | 2 +- tests/tcltests.tcl | 47 +++++++++++ tests/zipfs.test | 232 ++++++++++++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 272 insertions(+), 13 deletions(-) diff --git a/doc/zipfs.n b/doc/zipfs.n index a75a70b..b37702a 100644 --- a/doc/zipfs.n +++ b/doc/zipfs.n @@ -114,6 +114,10 @@ On Unix, this value is \fBzipfs unmount \fImountpoint\fR . Unmounts a previously mounted ZIP archive mounted to \fImountpoint\fR. +If the current directory is located within the mounted archive, +the directory that was previously the current directory is restored +on the unmount. The command will fail with an error exception if +there are any files within the mounted archive are open. .SS "ZIP CREATION COMMANDS" This package also provides several commands to aid the creation of ZIP archives as Tcl applications. diff --git a/generic/tclZipfs.c b/generic/tclZipfs.c index 1d424b6..a29a7a1 100644 --- a/generic/tclZipfs.c +++ b/generic/tclZipfs.c @@ -2385,7 +2385,7 @@ ZipFSUnmountObjCmd( Tcl_Obj *const objv[]) /* Argument objects. */ { if (objc != 2) { - Tcl_WrongNumArgs(interp, 1, objv, "zipfile"); + Tcl_WrongNumArgs(interp, 1, objv, "mountpoint"); return TCL_ERROR; } return TclZipfs_Unmount(interp, Tcl_GetString(objv[1])); diff --git a/tests/tcltests.tcl b/tests/tcltests.tcl index 61366a4..67c6bf9 100644 --- a/tests/tcltests.tcl +++ b/tests/tcltests.tcl @@ -62,6 +62,53 @@ namespace eval ::tcltests { error [list {could not create temporary directory}] } + # Generates test cases for 0, min and max number of arguments for a command. + # Expected result is as generated by Tcl_WrongNumArgs + # Only works if optional arguments come after fixed arguments + # E.g. + # testnumargs "zipfs mount" "" "?mountpoint? ?zipfile? ?password?" + # testnumargs "lappend" "varName" "?value ...?" + proc testnumargs {cmd {fixed {}} {optional {}} args} { + set minargs [llength $fixed] + set maxargs [expr {$minargs + [llength $optional]}] + if {[regexp {\.\.\.\??$} [lindex $optional end]]} { + unset maxargs; # No upper limit on num of args + } + set message "wrong # args: should be \"$cmd" + if {[llength $fixed]} { + append message " $fixed" + } + if {[llength $optional]} { + append message " $optional" + } + if {[llength $fixed] == 0 && [llength $optional] == 0} { + append message " \"" + } else { + append message "\"" + } + set label [join $cmd -] + if {$minargs > 0} { + set arguments [lrepeat [expr {$minargs-1}] x] + test $label-minargs-1 "$label no arguments" \ + -body "$cmd" \ + -result $message -returnCodes error \ + {*}$args + if {$minargs > 1} { + test $label-minargs-1 "$label missing arguments" \ + -body "$cmd $arguments" \ + -result $message -returnCodes error \ + {*}$args + } + } + if {[info exists maxargs]} { + set arguments [lrepeat [expr {$maxargs+1}] x] + test $label-maxargs-1 "$label extra arguments" \ + -body "$cmd $arguments" \ + -result $message -returnCodes error \ + {*}$args + } + } + init package provide tcltests 0.1 diff --git a/tests/zipfs.test b/tests/zipfs.test index bf9c969..834112c 100644 --- a/tests/zipfs.test +++ b/tests/zipfs.test @@ -14,10 +14,9 @@ if {"::tcltest" ni [namespace children]} { package require tcltest 2.5 namespace import -force ::tcltest::* } +source [file join [file dirname [info script]] tcltests.tcl] -testConstraint zipfs [expr { - [llength [info commands zlib]] && [regexp tcltest [info nameofexecutable]] -}] +testConstraint zipfs [expr {[llength [info commands zipfs]]}] testConstraint zipfslib 1 set ziproot [zipfs root] @@ -91,12 +90,6 @@ test zipfs-0.12 {zipfs basics: join} -constraints {zipfs zipfslib} -body { file normalize [zipfs root]//bar/baz//qux/../ } -result "[zipfs root]bar/baz" -test zipfs-1.3 {zipfs errors} -constraints zipfs -returnCodes error -body { - zipfs mount a b c d e f -} -result {wrong # args: should be "zipfs mount ?mountpoint? ?zipfile? ?password?"} -test zipfs-1.4 {zipfs errors} -constraints zipfs -returnCodes error -body { - zipfs unmount a b c d e f -} -result {wrong # args: should be "zipfs unmount zipfile"} test zipfs-1.5 {zipfs errors} -constraints zipfs -returnCodes error -body { zipfs mkkey a b c d e f } -result {wrong # args: should be "zipfs mkkey password"} @@ -383,16 +376,16 @@ test zipfs-4.5 {zipfs lmkimg: making image from mounted} -constraints zipfs -set test zipfs-5.1 {zipfs mount_data: short data} -constraints zipfs -body { zipfs mount_data gorp {} -} -returnCodes error -result {bad zip data} +} -returnCodes error -result {archive directory end signature not found} test zipfs-5.2 {zipfs mount_data: short data} -constraints zipfs -body { zipfs mount_data gorp gorpGORPgorp -} -returnCodes error -result {bad zip data} +} -returnCodes error -result {archive directory end signature not found} test zipfs-5.3 {zipfs mount_data: short data} -constraints zipfs -body { set data PK\x03\x04..................................... append data PK\x01\x02..................................... append data PK\x05\x06..................................... zipfs mount_data gorp $data -} -returnCodes error -result {bad zip data} +} -returnCodes error -result {archive directory truncated} test zipfs-5.4 {zipfs mount_data: bad arg count} -constraints zipfs -body { zipfs mount_data gorp {} foobar } -returnCodes error -result {wrong # args: should be "zipfs mount_data ?mountpoint? ?data?"} @@ -402,6 +395,221 @@ test zipfs-6.1 {zipfs mkkey} -constraints zipfs -body { return $x } -result {224 226 111 103 4 80 75 90 90} + +# +# Additional tests for more coverage. Some of the ones above may be duplicated. + +namespace eval test_ns_zipfs { + namespace import ::tcltest::test + namespace path ::tcltests + variable zipTestDir [file normalize [file join [file dirname [info script]] zipfiles]] + variable defaultMountPoint [file join [zipfs root] test] + + proc readbin {path} { + set fd [open $path rb] + set data [read $fd] + close $fd + return $data + } + + # Wrapper to ease transition if Tcl changes order of argument to zipfs mount + # or the zipfs prefix + proc mount [list zippath [list mountpoint $defaultMountPoint]] { + zipfs mount $mountpoint $zippath + } + + # Make full path to zip file + proc zippath {zipfile} { + variable zipTestDir + return [file join $zipTestDir $zipfile] + } + + proc cleanup {} { + variable defaultMountPoint + # Catch in case mount was not done + catch {zipfs unmount $defaultMountPoint} + } + + proc mounttarget {mountpoint} { + return [dict getdef [zipfs mount] $mountpoint ""] + } + + # + # zipfs mount + + proc testbadmount {id fname messagePattern args} { + variable zipTestDir + variable defaultMountPoint + set zipfile [zippath $fname] + test zipfs-mount-$id $id -body { + list [catch {mount $zipfile} message] \ + [string match $messagePattern $message] \ + [mounttarget $defaultMountPoint] + } -cleanup { + # In case mount succeeded when it should not + cleanup + } -result {1 1 {}} + + if {![file exists $zipfile]} { + return + } + set data [readbin $zipfile] + test zipfs-mount_data-$id $id -body { + list [catch {zipfs mount_data $defaultMountPoint $data} message] \ + [string match $messagePattern $message] \ + [mounttarget $defaultMountPoint] + } -cleanup { + # In case mount succeeded when it should not + cleanup + } -result {1 1 {}} + + } + proc testmount {id fname checkPath args} { + variable zipTestDir + variable defaultMountPoint + set zippath [zippath $fname] + test zipfs-mount-$id $id -body { + mount $zippath + list [file exists [file join $defaultMountPoint $checkPath]] \ + [mounttarget $defaultMountPoint] + } -cleanup { + cleanup + } -result [list 1 $zippath] + } + + testnumargs "zipfs mount" "" "?mountpoint? ?zipfile? ?password?" + + # Not supported zip files + testbadmount non-existent-file nosuchfile.zip "couldn't open*nosuchfile.zip*no such file or directory" + testbadmount not-zipfile [file normalize [info script]] "archive directory end signature not found" + testbadmount zip64-unsupported zip64.zip "wrong header signature" + + # Inconsistent metadata + testbadmount bad-directory-offset incons-cdoffset.zip "archive directory truncated" + testbadmount bad-directory-magic incons-central-magic-bad.zip "wrong header signature" + testbadmount bad-local-magic incons-local-magic-bad.zip "Failed to find local header" + + # TODO testbadmount bad-directory-crc incons-central-crc.zip "" + # TODO - at open testbadmount bad-file-count-low incons-file-count-low.zip "" + # TODO - at open testbadmount bad-file-count-high incons-file-count-high.zip "" + + testmount basic test.zip testdir/test2 + testmount zip-at-end junk-at-start.zip testdir/test2 + testmount zip-at-start junk-at-end.zip testdir/test2 + + test zipfs-mount-no-args-1 "mount - get mount list" -setup { + mount [zippath test.zip] + } -cleanup { + cleanup + } -body { + set mounts [zipfs mount] + lsearch -inline -stride 2 $mounts $defaultMountPoint + } -result [list $defaultMountPoint [zippath test.zip]] + + test zipfs-mount-one-arg-1 "mount - get mount target - absolute path" -setup { + mount [zippath test.zip] + } -cleanup { + cleanup + } -body { + zipfs mount $defaultMountPoint + } -result [zippath test.zip] + + test zipfs-mount-one-arg-2 "mount - get mount target - relative path" -setup { + mount ../tests/zipfiles/test.zip + } -cleanup { + cleanup + } -body { + zipfs mount $defaultMountPoint + } -result [zippath test.zip] + + test zipfs-mount-password-1 "mount - verify plaintext readable without password" -body { + zipfs mount $defaultMountPoint [zippath test-password.zip] + readbin [file join $defaultMountPoint plain.txt] + } -cleanup { + cleanup + } -result plaintext + + test zipfs-mount-password-2 "mount - verify uncompressed cipher unreadable without password" -body { + zipfs mount $defaultMountPoint [zippath test-password.zip] + set chans [lsort [chan names]]; # Want to ensure open does not leave dangling channel + set result [list ] + lappend result [catch {open [file join $defaultMountPoint cipher.bin]} message] + lappend result $message + lappend result [string equal $chans [lsort [chan names]]] + } -cleanup { + cleanup + } -result {1 {decryption failed} 1} + + test zipfs-mount-password-3 "mount - verify compressed cipher unreadable without password" -body { + zipfs mount $defaultMountPoint [zippath test-password.zip] + set chans [lsort [chan names]]; # Want to ensure open does not leave dangling channel + set result [list ] + lappend result [catch {open [file join $defaultMountPoint cipher-deflate.bin]} message] + lappend result $message + lappend result [string equal $chans [lsort [chan names]]] + } -cleanup { + cleanup + } -result {1 {decryption failed} 1} + + test zipfs-mount-password-4 "mount - verify uncompressed cipher readable with password" -body { + zipfs mount $defaultMountPoint [zippath test-password.zip] password + readbin [file join $defaultMountPoint cipher.bin] + } -cleanup { + cleanup + } -result ciphertext + + test zipfs-mount-password-5 "mount - verify compressed cipher readable with password" -body { + zipfs mount $defaultMountPoint [zippath test-password.zip] password + readbin [file join $defaultMountPoint cipher-deflate.bin] + } -cleanup { + cleanup + } -result [lseq 100] + + test xxzipfs-mount-existing-1 "Attempt to mount on existing mount point" -setup { + zipfs mount $defaultMountPoint + } + + # + # unmount - only special cases. Normal case already tested as part of other tests + + testnumargs "zipfs unmount" "mountpoint" "" + + test zipfs-unmount-1 "Unmount bogus mount" -body { + zipfs unmount [file join [zipfs root] nosuchmount] + } -result "" + + test zipfs-unmount-2 "Unmount mount with open files" -setup { + mount [zippath test.zip] + set fd [open [file join $defaultMountPoint test]] + } -cleanup { + close $fd + cleanup + } -body { + zipfs unmount $defaultMountPoint + } -result {filesystem is busy} -returnCodes error + + test zipfs-unmount-3 "Unmount mount with current directory" -setup { + mount [zippath test.zip] + } -cleanup { + cleanup + } -body { + set cwd [pwd] + cd [file join $defaultMountPoint testdir] + list [pwd] [zipfs unmount $defaultMountPoint] [string equal [pwd] $cwd] + } -result [list [file join $defaultMountPoint testdir] {} 1] + + test bug-6ed3447a7e "Crash opening file in streamed archive" -setup { + mount [zippath streamed.zip] + } -cleanup { + cleanup + } -body { + set fd [open [file join $defaultMountPoint -]] + list [catch {read $fd} message] [close $fd] $message + close $fd + } -result {file size error (may be zip64)} -returnCodes error +} + + ::tcltest::cleanupTests return -- cgit v0.12 From d158e3119f62de1142a940a45747d1e58559644b Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Sun, 10 Sep 2023 09:58:36 +0000 Subject: Oops. Inadvertent check-in. --- tests/zipfs.test | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/zipfs.test b/tests/zipfs.test index c2f5e6e..2837758 100644 --- a/tests/zipfs.test +++ b/tests/zipfs.test @@ -565,9 +565,14 @@ namespace eval test_ns_zipfs { cleanup } -result [lseq 100] - test xxzipfs-mount-existing-1 "Attempt to mount on existing mount point" -setup { - zipfs mount $defaultMountPoint - } + test zipfs-mount-existing-1 "Attempt to mount on existing mount point" -setup { + mount [zippath test.zip] + } -cleanup { + cleanup + } -body { + zipfs mount [zippath test2.zip] $defaultMountPoint + } -result {} -returnCodes error + # # unmount - only special cases. Normal case already tested as part of other tests -- cgit v0.12 From 587c1e337ee676ee7232a6ee86b225fe2224e9a4 Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Mon, 11 Sep 2023 16:49:32 +0000 Subject: More tests and some manpage corrections --- doc/zipfs.n | 20 ++-- tests/zipfs.test | 318 +++++++++++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 297 insertions(+), 41 deletions(-) diff --git a/doc/zipfs.n b/doc/zipfs.n index 2768f23..d57aff3 100644 --- a/doc/zipfs.n +++ b/doc/zipfs.n @@ -55,10 +55,10 @@ Return 1 if the given filename exists in the mounted zipfs and 0 if it does not. .TP \fBzipfs find\fR \fIdirectoryName\fR . -Recursively lists files including and below the directory \fIdirectoryName\fR. -The result list consists of relative path names starting from the given -directory. This command is also used by the \fBzipfs mkzip\fR and \fBzipfs -mkimg\fR commands. +Returns the list of paths under directory \fIdirectoryName\fR which need not be +within a zipfs mounted archive. The paths are prefixed with \fIdirectoryName\fR. +This command is also used by the \fBzipfs mkzip\fR and \fBzipfs mkimg\fR +commands. .TP \fBzipfs info\fR \fIfile\fR . @@ -80,9 +80,13 @@ in (4), which can be used to truncate the zip information from an executable. .TP \fBzipfs list\fR ?(\fB\-glob\fR|\fB\-regexp\fR)? ?\fIpattern\fR? . -Return a list of all files in the mounted zipfs, or just those matching -\fIpattern\fR (optionally controlled by the option parameters). The order of -the names in the list is arbitrary. +If \fIpattern\fR is not specified, the command returns a list of files across +all zipfs mounted archives. If \fIpattern\fR is specified, only those paths +matching the pattern are returned. By default, or with the \fB-glob\fR option, +the pattern is treated as a glob pattern and matching is done as described for +the \fBstring match\fR command. Alternatively, the \fB-regexp\fR option may be +used to specify matching \fBpattern\fR as a regular expression. The file names +are returned in arbitrary order. .TP \fBzipfs mount\fR .TP @@ -95,7 +99,7 @@ The \fBzipfs mount\fR command mounts ZIP archives as Tcl virtual file systems and returns information about current mounts. .PP With no arguments, the command returns a dictionary mapping -current mount points to the file path of the corresponding ZIP archive. +mount points to the path of the corresponding ZIP archive. .PP In the single argument form, the command returns the file path of the ZIP archive mounted at the specified mount point. diff --git a/tests/zipfs.test b/tests/zipfs.test index 2837758..db45f94 100644 --- a/tests/zipfs.test +++ b/tests/zipfs.test @@ -99,15 +99,9 @@ test zipfs-1.6 {zipfs errors} -constraints zipfs -returnCodes error -body { test zipfs-1.7 {zipfs errors} -constraints zipfs -returnCodes error -body { zipfs mkzip a b c d e f } -result {wrong # args: should be "zipfs mkzip outfile indir ?strip? ?password?"} -test zipfs-1.8 {zipfs errors} -constraints zipfs -returnCodes error -body { - zipfs exists a b c d e f -} -result {wrong # args: should be "zipfs exists filename"} test zipfs-1.9 {zipfs errors} -constraints zipfs -returnCodes error -body { zipfs info a b c d e f } -result {wrong # args: should be "zipfs info filename"} -test zipfs-1.10 {zipfs errors} -constraints zipfs -returnCodes error -body { - zipfs list a b c d e f -} -result {wrong # args: should be "zipfs list ?(-glob|-regexp)? ?pattern?"} file mkdir tmp test zipfs-2.1 {zipfs mkzip empty archive} -constraints zipfs -returnCodes error -body { @@ -403,7 +397,7 @@ namespace eval test_ns_zipfs { namespace import ::tcltest::test namespace path ::tcltests variable zipTestDir [file normalize [file join [file dirname [info script]] zipfiles]] - variable defaultMountPoint [file join [zipfs root] test] + variable defaultMountPoint [file join [zipfs root] testmount] proc readbin {path} { set fd [open $path rb] @@ -419,15 +413,22 @@ namespace eval test_ns_zipfs { } # Make full path to zip file - proc zippath {zipfile} { + proc zippath {zippath} { variable zipTestDir - return [file join $zipTestDir $zipfile] + if {[file pathtype $zippath] eq "absolute"} { + return $zippath + } else { + return [file join $zipTestDir $zippath] + } } proc cleanup {} { - variable defaultMountPoint - # Catch in case mount was not done - catch {zipfs unmount $defaultMountPoint} + dict for {mount -} [zipfs mount] { + if {[string match //zipfs:/test* $mount]} { + zipfs unmount $mount + } + } + zipfs unmount [zipfs root] } proc mounttarget {mountpoint} { @@ -437,23 +438,22 @@ namespace eval test_ns_zipfs { # # zipfs mount - proc testbadmount {id fname messagePattern args} { - variable zipTestDir + proc testbadmount {id zippath messagePattern args} { variable defaultMountPoint - set zipfile [zippath $fname] + set zippath [zippath $zippath] test zipfs-mount-$id $id -body { - list [catch {mount $zipfile} message] \ + list [catch {mount $zippath} message] \ [string match $messagePattern $message] \ [mounttarget $defaultMountPoint] } -cleanup { # In case mount succeeded when it should not cleanup - } -result {1 1 {}} + } -result {1 1 {}} {*}$args - if {![file exists $zipfile]} { + if {![file exists $zippath]} { return } - set data [readbin $zipfile] + set data [readbin $zippath] test zipfs-mount_data-$id $id -body { list [catch {zipfs mount_data $data $defaultMountPoint} message] \ [string match $messagePattern $message] \ @@ -461,20 +461,39 @@ namespace eval test_ns_zipfs { } -cleanup { # In case mount succeeded when it should not cleanup - } -result {1 1 {}} + } -result {1 1 {}} {*}$args } - proc testmount {id fname checkPath args} { - variable zipTestDir - variable defaultMountPoint - set zippath [zippath $fname] + + # Generates tests for file, file on root, memory buffer cases for an archive + proc testmount [list id zippath checkPath [list mountpoint $defaultMountPoint] args] { + set zippath [zippath $zippath] test zipfs-mount-$id $id -body { mount $zippath - list [file exists [file join $defaultMountPoint $checkPath]] \ - [mounttarget $defaultMountPoint] + list [file exists [file join $mountpoint $checkPath]] \ + [mounttarget $mountpoint] + } -cleanup { + cleanup + } -result [list 1 $zippath] {*}$args + # Also separately test mounting on root itself as some bugs + # have manifested in that scenario + test zipfs-mount-$id-on-root $id -body { + mount $zippath [zipfs root] + list [file exists [file join [zipfs root] $checkPath]] \ + [mounttarget [zipfs root]] } -cleanup { cleanup - } -result [list 1 $zippath] + } -result [list 1 $zippath] {*}$args + + # Mount memory buffer + test zipfs-mount_data-$id $id -body { + zipfs mount_data [readbin $zippath] $mountpoint + list [file exists [file join $mountpoint $checkPath]] \ + [mounttarget $mountpoint] + } -cleanup { + cleanup + } -result [list 1 {Memory Buffer}] {*}$args + } testnumargs "zipfs mount" "" "?zipfile? ?mountpoint? ?password?" @@ -496,6 +515,17 @@ namespace eval test_ns_zipfs { testmount basic test.zip testdir/test2 testmount zip-at-end junk-at-start.zip testdir/test2 testmount zip-at-start junk-at-end.zip testdir/test2 + testmount zip-in-zip [file join [zipfs root] test2 test.zip] testdir/test2 $defaultMountPoint -setup { + mount [zippath test-zip-in-zip.zip] [file join [zipfs root] test2] + } + + test zipfs-mount-busy-1 "Attempt to mount on existing mount point" -setup { + mount [zippath test.zip] + } -cleanup { + cleanup + } -body { + zipfs mount [zippath testfile-cp437.zip] $defaultMountPoint + } -result "[zippath test.zip] is already mounted on $defaultMountPoint" -returnCodes error test zipfs-mount-no-args-1 "mount - get mount list" -setup { mount [zippath test.zip] @@ -515,12 +545,14 @@ namespace eval test_ns_zipfs { } -result [zippath test.zip] test zipfs-mount-one-arg-2 "mount - get mount target - relative path" -setup { - mount ../tests/zipfiles/test.zip + file copy [zippath test.zip] test.zip + mount ./test.zip } -cleanup { cleanup + file delete ./test.zip } -body { zipfs mount $defaultMountPoint - } -result [zippath test.zip] + } -result [file normalize ./test.zip] test zipfs-mount-password-1 "mount - verify plaintext readable without password" -body { zipfs mount [zippath test-password.zip] $defaultMountPoint @@ -565,14 +597,34 @@ namespace eval test_ns_zipfs { cleanup } -result [lseq 100] - test zipfs-mount-existing-1 "Attempt to mount on existing mount point" -setup { + test zipfs-mount-nested-1 "mount - nested mount on non-existing path" -setup { mount [zippath test.zip] } -cleanup { cleanup } -body { - zipfs mount [zippath test2.zip] $defaultMountPoint - } -result {} -returnCodes error - + set newmount [file join $defaultMountPoint newdir] + mount [zippath test-overlay.zip] $newmount + list \ + [lsort [glob -tails -dir $defaultMountPoint *]] \ + [lsort [glob -tails -dir $newmount *]] \ + [readbin [file join $newmount test2]] + } -result {{newdir test testdir} {test2 test3} test2-overlay} + + test zipfs-mount-nested-2 "mount - nested mount on existing path" -setup { + mount [zippath test.zip] + } -cleanup { + cleanup + } -body { + set newmount [file join $defaultMountPoint testdir] + mount [zippath test-overlay.zip] $newmount + # Note - file from existing mount is preserved (testdir/test2) + # Not clear this is desired but defined as such by the + # current implementation + list \ + [lsort [glob -tails -dir $defaultMountPoint *]] \ + [lsort [glob -tails -dir $newmount *]] \ + [readbin [file join $newmount test2]] + } -result [list {test testdir} {test2 test3} test\n] # # unmount - only special cases. Normal case already tested as part of other tests @@ -603,6 +655,206 @@ namespace eval test_ns_zipfs { list [pwd] [zipfs unmount $defaultMountPoint] [string equal [pwd] $cwd] } -result [list [file join $defaultMountPoint testdir] {} 1] + test zipfs-unmount-nested-1 "unmount parent of nested mount on new directory should not affect nested mount" -setup { + mount [zippath test.zip] + set newmount [file join [zipfs root] test newdir] + mount [zippath test-overlay.zip] $newmount + } -cleanup { + cleanup + } -body { + zipfs unmount $defaultMountPoint + list \ + [zipfs mount $defaultMountPoint] \ + [lsort [glob -tails -dir $newmount *]] \ + [readbin [file join $newmount test2]] + } -result {{} {test2 test3} test2-overlay} + + test zipfs-unmount-nested-2 "unmount parent of nested mount on existing directory should not affect nested mount" -setup { + mount [zippath test.zip] + set newmount [file join [zipfs root] test testdir] + mount [zippath test-overlay.zip] $newmount + } -constraints knownBug -cleanup { + cleanup + } -body { + # KNOWN BUG. The test2 file is also present in parent mount. + # After the unmount, the test2 in the nested mount is not + # made available. + zipfs unmount $defaultMountPoint + list \ + [zipfs mount $defaultMountPoint] \ + [lsort [glob -tails -dir $newmount *]] \ + [readbin [file join $newmount test2]] + } -result {{} {test2 test3} test2-overlay} + + # + # zipfs list + testnumargs "zipfs list" "" "?(-glob|-regexp)? ?pattern?" + + # Generates zipfs list tests for file, memory buffer cases for an archive + proc testzipfslist {id cmdargs mounts resultpaths args} { + set resultpaths [lmap path $resultpaths { + file join [zipfs root] $path + }] + set resultpaths [lsort $resultpaths] + test zipfs-list-$id $id -body { + lsort [zipfs list {*}$cmdargs] + } -setup { + foreach {zippath mountpoint} $mounts { + zipfs mount [zippath $zippath] [file join [zipfs root] $mountpoint] + } + } -cleanup { + cleanup + } -result $resultpaths {*}$args + + # Mount memory buffer + test zipfs-list-memory-$id $id -body { + lsort [zipfs list {*}$cmdargs] + } -setup { + foreach {zippath mountpoint} $mounts { + zipfs mount_data [readbin [zippath $zippath]] [file join [zipfs root] $mountpoint] + } + } -cleanup { + cleanup + } -result $resultpaths {*}$args + } + testzipfslist no-mounts "" {} {} + testzipfslist no-pattern "" {test.zip testmountA} {testmountA testmountA/test testmountA/testdir testmountA/testdir/test2} + testzipfslist no-pattern-mount-on-root "" {test.zip {}} {{} test testdir testdir/test2} -constraints knownBug + testzipfslist no-pattern-multiple "" {test.zip testmountA test.zip testmountB/subdir} { + testmountA testmountA/test testmountA/testdir testmountA/testdir/test2 + testmountB/subdir testmountB/subdir/test testmountB/subdir/testdir testmountB/subdir/testdir/test2 + } + testzipfslist glob [list "*2*"] {test.zip testmountA test.zip testmountB/subdir} { + testmountA/testdir/test2 + testmountB/subdir/testdir/test2 + } + testzipfslist opt-glob [list -glob "*2*"] {test.zip testmountA test.zip testmountB/subdir} { + testmountA/testdir/test2 + testmountB/subdir/testdir/test2 + } + testzipfslist opt-regexp [list -regexp "A|2"] {test.zip testmountA test.zip testmountB/subdir} { + testmountA testmountA/test testmountA/testdir testmountA/testdir/test2 + testmountB/subdir/testdir/test2 + } + + # + # zipfs exists + testnumargs "zipfs exists" "filename" "" + + # Generates tests for zipfs exists + proc testzipfsexists {id path result args} { + test zipfs-exists-$id $id -body { + zipfs exists $path + } -setup { + mount [zippath test.zip] + } -cleanup { + cleanup + } -result $result {*}$args + } + testzipfsexists native-file [info nameofexecutable] 0 + testzipfsexists nonexistent-file [file join $defaultMountPoint nosuchfile] 0 + testzipfsexists file [file join $defaultMountPoint test] 1 + testzipfsexists dir [file join $defaultMountPoint testdir] 1 + testzipfsexists mountpoint $defaultMountPoint 1 + testzipfsexists root [zipfs root] 1 -constraints knownBug + + # + # zipfs find + testnumargs "zipfs find" "directoryName" "" + # Generates zipfs find tests for file, memory buffer cases for an archive + proc testzipfsfind {id findtarget mounts resultpaths args} { + set setup { + foreach {zippath mountpoint} $mounts { + zipfs mount [zippath $zippath] [file join [zipfs root] $mountpoint] + } + } + set memory_setup { + foreach {zippath mountpoint} $mounts { + zipfs mount_data [readbin [zippath $zippath]] [file join [zipfs root] $mountpoint] + } + } + if {[dict exists $args -setup]} { + append setup \n[dict get $args -setup] + append memory_setup \n[dict get $args -setup] + dict unset args -setup + } + set cleanup cleanup + if {[dict exists $args -cleanup]} { + set cleanup "[dict get $args -cleanup]\n$cleanup" + dict unset args -cleanup + } + set resultpaths [lsort $resultpaths] + test zipfs-find-$id $id -body { + lsort [zipfs find $findtarget] + } -setup $setup -cleanup $cleanup -result $resultpaths {*}$args + + # Mount memory buffer + test zipfs-find-memory-$id $id -body { + lsort [zipfs find $findtarget] + } -setup $memory_setup -cleanup $cleanup -result $resultpaths {*}$args + } + + testzipfsfind nonexistingmount [file join [zipfs root] nosuchmount] { + test.zip testmountA test.zip testmountB/subdir + } {} + + testzipfsfind absolute-path [file join [zipfs root] testmountA] { + test.zip testmountA test.zip testmountB/subdir + } [lmap path { + testmountA/test testmountA/testdir testmountA/testdir/test2 + } {file join [zipfs root] $path}] + + testzipfsfind relative-path testdir { + test.zip testmountA test.zip testmountB/subdir + } { testdir/test2 } -setup { + set cwd [pwd] + cd [file join [zipfs root] testmountA] + } -cleanup { + cd $cwd + } + + test zipfs-find-native-absolute "zipfs find on native file system" -setup { + set dir [makeDirectory zipfs-native-absolute] + set subdir [file join $dir subdir] + file mkdir $subdir + set file [file join $subdir native] + close [open $file w] + } -cleanup { + removeDirectory zipfs-native-absolute + } -body { + string equal [zipfs find $dir] [list $subdir $file] + } -result 1 + + test zipfs-find-native-relative "zipfs find relative on native file system" -setup { + set dir [makeDirectory zipfs-native-relative] + set subdir [file join $dir subdir] + file mkdir $subdir + set file [file join $subdir native] + close [open $file w] + set cwd [pwd] + } -cleanup { + cd $cwd + removeDirectory zipfs-native-relative + } -body { + cd [file dirname $dir] + # string equal [zipfs find [file tail $subdir]] [list subdir subdir/native] + zipfs find [file tail $dir] + } -result {zipfs-native-relative/subdir zipfs-native-relative/subdir/native} + + testzipfsfind bug-6183f535c8 [zipfs root] { + test.zip {} test.zip testmountB/subdir + } [lmap path { + test testdir testdir/test2 + } {file join [zipfs root] $path}] -constraints knownBug + + # + # zipfs info + testnumargs "zipfs info" "filename" "" + + + # + # Bug regressions + test bug-6ed3447a7e "Crash opening file in streamed archive" -setup { mount [zippath streamed.zip] } -cleanup { -- cgit v0.12 From 7872e34ec6eda0e3119b3b340e4c003315adfe18 Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Tue, 12 Sep 2023 14:57:25 +0000 Subject: Fix zipfs root arg count checks --- generic/tclZipfs.c | 27 ++++++++++++++++++----- tests/zipfs.test | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 84 insertions(+), 7 deletions(-) diff --git a/generic/tclZipfs.c b/generic/tclZipfs.c index 50a8aeb..1d863b8 100644 --- a/generic/tclZipfs.c +++ b/generic/tclZipfs.c @@ -1270,8 +1270,8 @@ ZipFSFindTOC( size_t localhdr_off = zf->length; if (q + ZIP_CENTRAL_HEADER_LEN > end) { - ZIPFS_ERROR(interp, "wrong header length"); - ZIPFS_ERROR_CODE(interp, "HDR_LEN"); + ZIPFS_ERROR(interp, "truncated directory"); + ZIPFS_ERROR_CODE(interp, "TRUNC_DIR"); goto error; } if (ZipReadInt(start, end, q) != ZIP_CENTRAL_HEADER_SIG) { @@ -2362,9 +2362,13 @@ static int ZipFSRootObjCmd( TCL_UNUSED(ClientData), 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; } @@ -3736,6 +3740,7 @@ ZipFSInfoObjCmd( { char *filename; ZipEntry *z; + int ret; if (objc != 2) { Tcl_WrongNumArgs(interp, 1, objv, "filename"); @@ -3754,11 +3759,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; } - + /* *------------------------------------------------------------------------- * diff --git a/tests/zipfs.test b/tests/zipfs.test index db45f94..cce5899 100644 --- a/tests/zipfs.test +++ b/tests/zipfs.test @@ -436,6 +436,11 @@ namespace eval test_ns_zipfs { } # + # zipfs root - only arg count check since do not want to assume + # what it resolves to + testnumargs "zipfs root" "" "" + + # # zipfs mount proc testbadmount {id zippath messagePattern args} { @@ -507,10 +512,10 @@ namespace eval test_ns_zipfs { testbadmount bad-directory-offset incons-cdoffset.zip "archive directory truncated" testbadmount bad-directory-magic incons-central-magic-bad.zip "wrong header signature" testbadmount bad-local-magic incons-local-magic-bad.zip "Failed to find local header" + testbadmount bad-file-count-high incons-file-count-high.zip "truncated directory" # TODO testbadmount bad-directory-crc incons-central-crc.zip "" # TODO - at open testbadmount bad-file-count-low incons-file-count-low.zip "" - # TODO - at open testbadmount bad-file-count-high incons-file-count-high.zip "" testmount basic test.zip testdir/test2 testmount zip-at-end junk-at-start.zip testdir/test2 @@ -851,6 +856,63 @@ namespace eval test_ns_zipfs { # zipfs info testnumargs "zipfs info" "filename" "" + test zipfs-info-native-nosuchfile "zipfs info on non-existent native path" -body { + zipfs info nosuchfile + } -result {path "nosuchfile" not found in any zipfs volume} -returnCodes error + + test zipfs-info-native-file "zipfs info on native path" -body { + zipfs info [info nameofexecutable] + } -result "path \"[info nameofexecutable]\" not found in any zipfs volume" -returnCodes error + + test zipfs-info-nosuchfile "zipfs info non-existent path in mounted archive" -setup { + mount [zippath test.zip] + } -cleanup { + cleanup + } -body { + zipfs info [file join $defaultMountPoint nosuchfile] + } -result "path \"[file join $defaultMountPoint nosuchfile]\" not found in any zipfs volume" -returnCodes error + + test zipfs-info-file "zipfs info file within mounted archive" -setup { + mount [zippath testdeflated2.zip] + } -cleanup { + cleanup + } -body { + zipfs info [file join $defaultMountPoint abac-repeat.txt] + } -result [list [zippath testdeflated2.zip] 60 17 108] + + test zipfs-info-dir "zipfs info dir within mounted archive" -setup { + mount [zippath test.zip] + } -cleanup { + cleanup + } -body { + zipfs info [file join $defaultMountPoint testdir] + } -result [list [zippath test.zip] 0 0 119] + + test zipfs-info-mountpoint "zipfs info on mount point - verify correct offset of zip content" -setup { + # zip starts at offset 4 + mount [zippath junk-at-start.zip] + } -cleanup { + cleanup + } -body { + zipfs info $defaultMountPoint + } -result [list [zippath junk-at-start.zip] 0 0 4] + + # + # zipfs canonical - TODO - semantics are very unclear. Can produce nonsensical paths like + # //zipfs:/n/zipfs:/m/test + + # + # TODO - open of zipfs file + + + # + # TODO - glob of zipfs file + + # + # TODO - stat of zipfs file + + # + # TODO - copy, rename etc. # # Bug regressions -- cgit v0.12 From 114fb3a9414df6a097cd8008d34c2a1ccd7997f1 Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Wed, 13 Sep 2023 06:20:12 +0000 Subject: Bug [9a80630571] - zipfs directory consistency checks --- generic/tclZipfs.c | 86 +++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 65 insertions(+), 21 deletions(-) diff --git a/generic/tclZipfs.c b/generic/tclZipfs.c index 1d863b8..77bc6b5 100644 --- a/generic/tclZipfs.c +++ b/generic/tclZipfs.c @@ -199,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 */ @@ -1225,6 +1226,8 @@ ZipFSFindTOC( 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. */ @@ -1241,17 +1244,30 @@ 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 truncated"); @@ -1260,16 +1276,37 @@ ZipFSFindTOC( } /* - * 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 */ - if (q + ZIP_CENTRAL_HEADER_LEN > end) { + /* + * 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. + */ + 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))); + + /* + * 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; @@ -1279,10 +1316,10 @@ 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); + 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) { @@ -1295,6 +1332,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; @@ -4372,7 +4415,8 @@ ZipChannelOpen( goto error; } - if (z->numBytes < 0 || z->numCompressedBytes < 0 || z->offset < 0) { + 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"); -- cgit v0.12 From 8200cc5892deff530ac8b23bc7bb66d38abc30fa Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Wed, 13 Sep 2023 06:42:54 +0000 Subject: Add !zipfslib constraint for tests that should only run without an attached Tcl lib vfs --- doc/zipfs.n | 4 +++- generic/tclZipfs.c | 4 ++-- tests/zipfs.test | 17 +++++++++-------- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/doc/zipfs.n b/doc/zipfs.n index d57aff3..b59e072 100644 --- a/doc/zipfs.n +++ b/doc/zipfs.n @@ -86,7 +86,9 @@ matching the pattern are returned. By default, or with the \fB-glob\fR option, the pattern is treated as a glob pattern and matching is done as described for the \fBstring match\fR command. Alternatively, the \fB-regexp\fR option may be used to specify matching \fBpattern\fR as a regular expression. The file names -are returned in arbitrary order. +are returned in arbitrary order. Note that path separators are treated as +ordinary characters in the matching. Thus forward slashes should be used +as path separators in the pattern. .TP \fBzipfs mount\fR .TP diff --git a/generic/tclZipfs.c b/generic/tclZipfs.c index 77bc6b5..ef2abcd 100644 --- a/generic/tclZipfs.c +++ b/generic/tclZipfs.c @@ -1289,12 +1289,12 @@ ZipFSFindTOC( zf->directoryOffset = cdirZipOffset + zf->baseOffset; zf->directorySize = cdirSize; - const unsigned char *const cdirStart = p - cdirSize; /* Start of */ + 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. + * the asserts. May be removed at some future date. */ q = zf->data + cdirZipOffset; p -= cdirSize; diff --git a/tests/zipfs.test b/tests/zipfs.test index cce5899..7754432 100644 --- a/tests/zipfs.test +++ b/tests/zipfs.test @@ -512,10 +512,10 @@ namespace eval test_ns_zipfs { testbadmount bad-directory-offset incons-cdoffset.zip "archive directory truncated" testbadmount bad-directory-magic incons-central-magic-bad.zip "wrong header signature" testbadmount bad-local-magic incons-local-magic-bad.zip "Failed to find local header" - testbadmount bad-file-count-high incons-file-count-high.zip "truncated directory" + testbadmount bad-file-count-high incons-file-count-high.zip "truncated directory" + testbadmount bad-file-count-low incons-file-count-low.zip "short file count" # TODO testbadmount bad-directory-crc incons-central-crc.zip "" - # TODO - at open testbadmount bad-file-count-low incons-file-count-low.zip "" testmount basic test.zip testdir/test2 testmount zip-at-end junk-at-start.zip testdir/test2 @@ -722,22 +722,23 @@ namespace eval test_ns_zipfs { cleanup } -result $resultpaths {*}$args } - testzipfslist no-mounts "" {} {} - testzipfslist no-pattern "" {test.zip testmountA} {testmountA testmountA/test testmountA/testdir testmountA/testdir/test2} + # Some tests have !zipfslib constraint because otherwise they dump the entire Tcl library which is mounted on root + testzipfslist no-mounts "" {} {} -constraints !zipfslib + testzipfslist no-pattern "" {test.zip testmountA} {testmountA testmountA/test testmountA/testdir testmountA/testdir/test2} -constraints !zipfslib testzipfslist no-pattern-mount-on-root "" {test.zip {}} {{} test testdir testdir/test2} -constraints knownBug testzipfslist no-pattern-multiple "" {test.zip testmountA test.zip testmountB/subdir} { testmountA testmountA/test testmountA/testdir testmountA/testdir/test2 testmountB/subdir testmountB/subdir/test testmountB/subdir/testdir testmountB/subdir/testdir/test2 - } - testzipfslist glob [list "*2*"] {test.zip testmountA test.zip testmountB/subdir} { + } -constraints !zipfslib + testzipfslist glob [list "*testmount*2*"] {test.zip testmountA test.zip testmountB/subdir} { testmountA/testdir/test2 testmountB/subdir/testdir/test2 } - testzipfslist opt-glob [list -glob "*2*"] {test.zip testmountA test.zip testmountB/subdir} { + testzipfslist opt-glob [list -glob "*testmount*2*"] {test.zip testmountA test.zip testmountB/subdir} { testmountA/testdir/test2 testmountB/subdir/testdir/test2 } - testzipfslist opt-regexp [list -regexp "A|2"] {test.zip testmountA test.zip testmountB/subdir} { + testzipfslist opt-regexp [list -regexp "testmount.*(A|2)"] {test.zip testmountA test.zip testmountB/subdir} { testmountA testmountA/test testmountA/testdir testmountA/testdir/test2 testmountB/subdir/testdir/test2 } -- cgit v0.12 From 3d8ff23d678f618a50b69ec2752d47a544fdd5ce Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Wed, 13 Sep 2023 15:33:24 +0000 Subject: Couple of more special cases for zipfs mount --- tests/zipfs.test | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/tests/zipfs.test b/tests/zipfs.test index 7754432..e8dbccc 100644 --- a/tests/zipfs.test +++ b/tests/zipfs.test @@ -471,30 +471,23 @@ namespace eval test_ns_zipfs { } # Generates tests for file, file on root, memory buffer cases for an archive - proc testmount [list id zippath checkPath [list mountpoint $defaultMountPoint] args] { + proc testmount {id zippath checkPath mountpoint args} { set zippath [zippath $zippath] test zipfs-mount-$id $id -body { - mount $zippath - list [file exists [file join $mountpoint $checkPath]] \ - [mounttarget $mountpoint] + mount $zippath $mountpoint + set canon [zipfs canonical $mountpoint] + list [file exists [file join $canon $checkPath]] \ + [mounttarget $canon] } -cleanup { - cleanup - } -result [list 1 $zippath] {*}$args - # Also separately test mounting on root itself as some bugs - # have manifested in that scenario - test zipfs-mount-$id-on-root $id -body { - mount $zippath [zipfs root] - list [file exists [file join [zipfs root] $checkPath]] \ - [mounttarget [zipfs root]] - } -cleanup { - cleanup + zipfs unmount $mountpoint } -result [list 1 $zippath] {*}$args # Mount memory buffer test zipfs-mount_data-$id $id -body { zipfs mount_data [readbin $zippath] $mountpoint - list [file exists [file join $mountpoint $checkPath]] \ - [mounttarget $mountpoint] + set canon [zipfs canonical $mountpoint] + list [file exists [file join $canon $checkPath]] \ + [mounttarget $canon] } -cleanup { cleanup } -result [list 1 {Memory Buffer}] {*}$args @@ -517,12 +510,20 @@ namespace eval test_ns_zipfs { # TODO testbadmount bad-directory-crc incons-central-crc.zip "" - testmount basic test.zip testdir/test2 - testmount zip-at-end junk-at-start.zip testdir/test2 - testmount zip-at-start junk-at-end.zip testdir/test2 + testmount basic test.zip testdir/test2 $defaultMountPoint + testmount basic-on-default test.zip testdir/test2 "" + testmount basic-on-root test.zip testdir/test2 [zipfs root] + # TODO - testmount basic-on-slash test.zip testdir/test2 / + testmount basic-on-relative test.zip testdir/test2 testmount + testmount zip-at-end junk-at-start.zip testdir/test2 $defaultMountPoint + testmount zip-at-start junk-at-end.zip testdir/test2 $defaultMountPoint testmount zip-in-zip [file join [zipfs root] test2 test.zip] testdir/test2 $defaultMountPoint -setup { mount [zippath test-zip-in-zip.zip] [file join [zipfs root] test2] + } -cleanup { + zipfs unmount $mountpoint + zipfs unmount [file join [zipfs root] test2] } + testmount relative-mount-point test.zip testdir/test2 "" test zipfs-mount-busy-1 "Attempt to mount on existing mount point" -setup { mount [zippath test.zip] -- cgit v0.12 From e28353f4c36473ce17523409cca53c6d3dfc2612 Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Wed, 13 Sep 2023 16:45:19 +0000 Subject: Bug [01d8f30342]. zipfs canonical produces garbage. --- generic/tclZipfs.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/generic/tclZipfs.c b/generic/tclZipfs.c index 4d86b8c..40ee564 100644 --- a/generic/tclZipfs.c +++ b/generic/tclZipfs.c @@ -910,10 +910,12 @@ CanonicalPath( break; default: if (inZipfs) { - Tcl_DStringSetLength(dsPtr, i + j + ZIPFS_VOLUME_LEN); + Tcl_DStringSetLength(dsPtr, i + 1 + j + ZIPFS_VOLUME_LEN); path = Tcl_DStringValue(dsPtr); memcpy(path, ZIPFS_VOLUME, ZIPFS_VOLUME_LEN); - memcpy(path + ZIPFS_VOLUME_LEN + i , tail, j); + memcpy(path+ZIPFS_VOLUME_LEN, root, i); + path[ZIPFS_VOLUME_LEN+i] = '/'; + memcpy(path + ZIPFS_VOLUME_LEN + 1 + i, tail, j); } else { Tcl_DStringSetLength(dsPtr, i + j + 1); path = Tcl_DStringValue(dsPtr); -- cgit v0.12 From e9833540e420da80ee1f57cbde0020ad7233c0a3 Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Thu, 14 Sep 2023 14:49:39 +0000 Subject: Added test files for zipfs --- tests/zipfiles/LICENSE-libzip | 31 ++++++++++++++++++++++++++++ tests/zipfiles/README | 7 +++++++ tests/zipfiles/broken.zip | Bin 0 -> 75091 bytes tests/zipfiles/incons-cdoffset.zip | Bin 0 -> 153 bytes tests/zipfiles/incons-central-crc.zip | Bin 0 -> 153 bytes tests/zipfiles/incons-central-magic-bad.zip | Bin 0 -> 153 bytes tests/zipfiles/incons-file-count-high.zip | Bin 0 -> 153 bytes tests/zipfiles/incons-file-count-low.zip | Bin 0 -> 304 bytes tests/zipfiles/incons-local-magic-bad.zip | Bin 0 -> 153 bytes tests/zipfiles/junk-at-end.zip | Bin 0 -> 416 bytes tests/zipfiles/junk-at-start.zip | Bin 0 -> 416 bytes tests/zipfiles/streamed.zip | Bin 0 -> 120 bytes tests/zipfiles/test-overlay.zip | Bin 0 -> 498 bytes tests/zipfiles/test-password.zip | Bin 0 -> 956 bytes tests/zipfiles/test-zip-in-zip.zip | Bin 0 -> 665 bytes tests/zipfiles/test.zip | Bin 0 -> 412 bytes tests/zipfiles/testbzip2.zip | Bin 0 -> 175 bytes tests/zipfiles/testdeflated2.zip | Bin 0 -> 270 bytes tests/zipfiles/testfile-UTF8.zip | Bin 0 -> 126 bytes tests/zipfiles/testfile-cp437.zip | Bin 0 -> 130 bytes tests/zipfiles/testfile-lzma.zip | Bin 0 -> 161 bytes tests/zipfiles/testfile-nocompression.zip | Bin 0 -> 122 bytes tests/zipfiles/testfile-xz.zip | Bin 0 -> 200 bytes tests/zipfiles/testfile-zstd.zip | Bin 0 -> 160 bytes tests/zipfiles/teststored.zip | Bin 0 -> 188 bytes tests/zipfiles/zip64.zip | Bin 0 -> 198 bytes 26 files changed, 38 insertions(+) create mode 100644 tests/zipfiles/LICENSE-libzip create mode 100644 tests/zipfiles/README create mode 100644 tests/zipfiles/broken.zip create mode 100644 tests/zipfiles/incons-cdoffset.zip create mode 100644 tests/zipfiles/incons-central-crc.zip create mode 100644 tests/zipfiles/incons-central-magic-bad.zip create mode 100644 tests/zipfiles/incons-file-count-high.zip create mode 100644 tests/zipfiles/incons-file-count-low.zip create mode 100644 tests/zipfiles/incons-local-magic-bad.zip create mode 100644 tests/zipfiles/junk-at-end.zip create mode 100644 tests/zipfiles/junk-at-start.zip create mode 100644 tests/zipfiles/streamed.zip create mode 100644 tests/zipfiles/test-overlay.zip create mode 100644 tests/zipfiles/test-password.zip create mode 100644 tests/zipfiles/test-zip-in-zip.zip create mode 100644 tests/zipfiles/test.zip create mode 100644 tests/zipfiles/testbzip2.zip create mode 100644 tests/zipfiles/testdeflated2.zip create mode 100644 tests/zipfiles/testfile-UTF8.zip create mode 100644 tests/zipfiles/testfile-cp437.zip create mode 100644 tests/zipfiles/testfile-lzma.zip create mode 100644 tests/zipfiles/testfile-nocompression.zip create mode 100644 tests/zipfiles/testfile-xz.zip create mode 100644 tests/zipfiles/testfile-zstd.zip create mode 100644 tests/zipfiles/teststored.zip create mode 100644 tests/zipfiles/zip64.zip diff --git a/tests/zipfiles/LICENSE-libzip b/tests/zipfiles/LICENSE-libzip new file mode 100644 index 0000000..fa70609 --- /dev/null +++ b/tests/zipfiles/LICENSE-libzip @@ -0,0 +1,31 @@ +Copyright (C) 1999-2020 Dieter Baron and Thomas Klausner + +The authors can be contacted at + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions +are met: + +1. Redistributions of source code must retain the above copyright + notice, this list of conditions and the following disclaimer. + +2. Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in + the documentation and/or other materials provided with the + distribution. + +3. The names of the authors may not be used to endorse or promote + products derived from this software without specific prior + written permission. + +THIS SOFTWARE IS PROVIDED BY THE AUTHORS ``AS IS'' AND ANY EXPRESS +OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED +WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHORS BE LIABLE FOR ANY +DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE +GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER +IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR +OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN +IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. diff --git a/tests/zipfiles/README b/tests/zipfiles/README new file mode 100644 index 0000000..e6c366e --- /dev/null +++ b/tests/zipfiles/README @@ -0,0 +1,7 @@ +The files in this directory are used for testing zipfs file systems. +They fall under the following licenses: + +test-overlay.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/zipfiles/broken.zip b/tests/zipfiles/broken.zip new file mode 100644 index 0000000..2008c32 Binary files /dev/null and b/tests/zipfiles/broken.zip differ diff --git a/tests/zipfiles/incons-cdoffset.zip b/tests/zipfiles/incons-cdoffset.zip new file mode 100644 index 0000000..153e7a8 Binary files /dev/null and b/tests/zipfiles/incons-cdoffset.zip differ diff --git a/tests/zipfiles/incons-central-crc.zip b/tests/zipfiles/incons-central-crc.zip new file mode 100644 index 0000000..1f882ac Binary files /dev/null and b/tests/zipfiles/incons-central-crc.zip differ diff --git a/tests/zipfiles/incons-central-magic-bad.zip b/tests/zipfiles/incons-central-magic-bad.zip new file mode 100644 index 0000000..954563b Binary files /dev/null and b/tests/zipfiles/incons-central-magic-bad.zip differ diff --git a/tests/zipfiles/incons-file-count-high.zip b/tests/zipfiles/incons-file-count-high.zip new file mode 100644 index 0000000..876a886 Binary files /dev/null and b/tests/zipfiles/incons-file-count-high.zip differ diff --git a/tests/zipfiles/incons-file-count-low.zip b/tests/zipfiles/incons-file-count-low.zip new file mode 100644 index 0000000..4af9038 Binary files /dev/null and b/tests/zipfiles/incons-file-count-low.zip differ diff --git a/tests/zipfiles/incons-local-magic-bad.zip b/tests/zipfiles/incons-local-magic-bad.zip new file mode 100644 index 0000000..9a6a061 Binary files /dev/null and b/tests/zipfiles/incons-local-magic-bad.zip differ diff --git a/tests/zipfiles/junk-at-end.zip b/tests/zipfiles/junk-at-end.zip new file mode 100644 index 0000000..30387b3 Binary files /dev/null and b/tests/zipfiles/junk-at-end.zip differ diff --git a/tests/zipfiles/junk-at-start.zip b/tests/zipfiles/junk-at-start.zip new file mode 100644 index 0000000..8c98325 Binary files /dev/null and b/tests/zipfiles/junk-at-start.zip differ diff --git a/tests/zipfiles/streamed.zip b/tests/zipfiles/streamed.zip new file mode 100644 index 0000000..737d56f Binary files /dev/null and b/tests/zipfiles/streamed.zip differ diff --git a/tests/zipfiles/test-overlay.zip b/tests/zipfiles/test-overlay.zip new file mode 100644 index 0000000..9dc2c52 Binary files /dev/null and b/tests/zipfiles/test-overlay.zip differ diff --git a/tests/zipfiles/test-password.zip b/tests/zipfiles/test-password.zip new file mode 100644 index 0000000..bb9780a Binary files /dev/null and b/tests/zipfiles/test-password.zip differ diff --git a/tests/zipfiles/test-zip-in-zip.zip b/tests/zipfiles/test-zip-in-zip.zip new file mode 100644 index 0000000..8797c32 Binary files /dev/null and b/tests/zipfiles/test-zip-in-zip.zip differ diff --git a/tests/zipfiles/test.zip b/tests/zipfiles/test.zip new file mode 100644 index 0000000..e4efd71 Binary files /dev/null and b/tests/zipfiles/test.zip differ diff --git a/tests/zipfiles/testbzip2.zip b/tests/zipfiles/testbzip2.zip new file mode 100644 index 0000000..7c9a9e7 Binary files /dev/null and b/tests/zipfiles/testbzip2.zip differ diff --git a/tests/zipfiles/testdeflated2.zip b/tests/zipfiles/testdeflated2.zip new file mode 100644 index 0000000..b5ded7d Binary files /dev/null and b/tests/zipfiles/testdeflated2.zip differ diff --git a/tests/zipfiles/testfile-UTF8.zip b/tests/zipfiles/testfile-UTF8.zip new file mode 100644 index 0000000..7279615 Binary files /dev/null and b/tests/zipfiles/testfile-UTF8.zip differ diff --git a/tests/zipfiles/testfile-cp437.zip b/tests/zipfiles/testfile-cp437.zip new file mode 100644 index 0000000..169a903 Binary files /dev/null and b/tests/zipfiles/testfile-cp437.zip differ diff --git a/tests/zipfiles/testfile-lzma.zip b/tests/zipfiles/testfile-lzma.zip new file mode 100644 index 0000000..f855b2a Binary files /dev/null and b/tests/zipfiles/testfile-lzma.zip differ diff --git a/tests/zipfiles/testfile-nocompression.zip b/tests/zipfiles/testfile-nocompression.zip new file mode 100644 index 0000000..2fa5ba0 Binary files /dev/null and b/tests/zipfiles/testfile-nocompression.zip differ diff --git a/tests/zipfiles/testfile-xz.zip b/tests/zipfiles/testfile-xz.zip new file mode 100644 index 0000000..6be8f9c Binary files /dev/null and b/tests/zipfiles/testfile-xz.zip differ diff --git a/tests/zipfiles/testfile-zstd.zip b/tests/zipfiles/testfile-zstd.zip new file mode 100644 index 0000000..bf42d3e Binary files /dev/null and b/tests/zipfiles/testfile-zstd.zip differ diff --git a/tests/zipfiles/teststored.zip b/tests/zipfiles/teststored.zip new file mode 100644 index 0000000..138c6ad Binary files /dev/null and b/tests/zipfiles/teststored.zip differ diff --git a/tests/zipfiles/zip64.zip b/tests/zipfiles/zip64.zip new file mode 100644 index 0000000..c1ba76b Binary files /dev/null and b/tests/zipfiles/zip64.zip differ -- cgit v0.12 From 16905581a386e5438de74712dd90b45d1f7760b9 Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Thu, 14 Sep 2023 14:52:06 +0000 Subject: More tests and fix one more case triggering bug [01d8f30342] --- generic/tclZipfs.c | 16 +++++--- tests/zipfs.test | 117 +++++++++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 115 insertions(+), 18 deletions(-) diff --git a/generic/tclZipfs.c b/generic/tclZipfs.c index 40ee564..71d3c55 100644 --- a/generic/tclZipfs.c +++ b/generic/tclZipfs.c @@ -910,12 +910,16 @@ CanonicalPath( break; default: if (inZipfs) { - Tcl_DStringSetLength(dsPtr, i + 1 + j + ZIPFS_VOLUME_LEN); - path = Tcl_DStringValue(dsPtr); - memcpy(path, ZIPFS_VOLUME, ZIPFS_VOLUME_LEN); - memcpy(path+ZIPFS_VOLUME_LEN, root, i); - path[ZIPFS_VOLUME_LEN+i] = '/'; - memcpy(path + ZIPFS_VOLUME_LEN + 1 + 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); diff --git a/tests/zipfs.test b/tests/zipfs.test index e8dbccc..3bbecb8 100644 --- a/tests/zipfs.test +++ b/tests/zipfs.test @@ -467,7 +467,6 @@ namespace eval test_ns_zipfs { # In case mount succeeded when it should not cleanup } -result {1 1 {}} {*}$args - } # Generates tests for file, file on root, memory buffer cases for an archive @@ -758,12 +757,12 @@ namespace eval test_ns_zipfs { cleanup } -result $result {*}$args } - testzipfsexists native-file [info nameofexecutable] 0 + testzipfsexists native-file [info nameofexecutable] 0 testzipfsexists nonexistent-file [file join $defaultMountPoint nosuchfile] 0 - testzipfsexists file [file join $defaultMountPoint test] 1 - testzipfsexists dir [file join $defaultMountPoint testdir] 1 - testzipfsexists mountpoint $defaultMountPoint 1 - testzipfsexists root [zipfs root] 1 -constraints knownBug + testzipfsexists file [file join $defaultMountPoint test] 1 + testzipfsexists dir [file join $defaultMountPoint testdir] 1 + testzipfsexists mountpoint $defaultMountPoint 1 + testzipfsexists root [zipfs root] 1 -constraints knownBug # # zipfs find @@ -900,21 +899,115 @@ namespace eval test_ns_zipfs { } -result [list [zippath junk-at-start.zip] 0 0 4] # - # zipfs canonical - TODO - semantics are very unclear. Can produce nonsensical paths like - # //zipfs:/n/zipfs:/m/test + # zipfs canonical - + # TODO - semantics are very unclear. Can produce nonsensical paths like + # //zipfs:/n/zipfs:/m/test. Minimal sanity tests for now. + test zipfs-canonical-minargs {zipfs canonical min args} -body { + zipfs canonical + } -returnCodes error -result {wrong # args: should be "zipfs canonical ?mountpoint? filename ?inZipfs?"} + test zipfs-canonical-maxargs {zipfs canonical max args} -body { + zipfs canonical a b c d + } -returnCodes error -result {wrong # args: should be "zipfs canonical ?mountpoint? filename ?inZipfs?"} + proc testzipfscanonical {id cmdargs result args} { + test zipfs-canonical-$id "zipfs canonical $id" \ + -body [list zipfs canonical {*}$cmdargs] \ + -result $result {*}$args + } + testzipfscanonical basic-relative PATH [file join [zipfs root] PATH] + testzipfscanonical basic-absolute /PATH [file join [zipfs root] PATH] + testzipfscanonical mountpoint-relative {MT PATH} [file join [zipfs root] MT PATH] + testzipfscanonical mountpoint-absolute {MT /PATH} [file join [zipfs root] PATH] + testzipfscanonical mountpoint-trailslash-relative {MT/ PATH} [file join [zipfs root] MT PATH] + testzipfscanonical mountpoint-trailslash-absolute {MT/ /PATH} [file join [zipfs root] PATH] + testzipfscanonical mountpoint-root-relative [list [zipfs root] PATH] [file join [zipfs root] PATH] + testzipfscanonical mountpoint-root-absolute [list [zipfs root] /PATH] [file join [zipfs root] PATH] + testzipfscanonical mountpoint-empty-relative {{} PATH} [file join [zipfs root] PATH] + + testzipfscanonical driveletter X: [zipfs root] -constraints win + testzipfscanonical drivepath X:/foo/bar [file join [zipfs root] foo bar] -constraints win + # (backslashes need additional escaping passed to testzipfscanonical) + testzipfscanonical backslashes X:\\\\foo\\\\bar [file join [zipfs root] foo bar] -constraints win + testzipfscanonical backslashes X:/foo\\\\bar [file join [zipfs root] foo bar] -constraints win + + + + # + # TODO - read of zipfs file Bad CRC + + # + # Read/uncompress + proc testuncompress {id zippath result args} { + variable defaultMountPoint + set zippath [zippath $zippath] + test zipfs-uncompress-$id $id -setup { + unset -nocomplain fd + mount $zippath + } -cleanup { + # In case mount succeeded when it should not + if {[info exists fd]} { + close $fd + } + cleanup + } -body { + set fd [open [file join $defaultMountPoint abac-repeat.txt]] + gets $fd + } -result $result {*}$args + } + testuncompress deflate testdeflated2.zip aaaaaaaaaaaaaa + testuncompress stored teststored.zip aaaaaaaaaaaaaa + testuncompress bzip2 testbzip2.zip {unsupported compression method} -returnCodes error + testuncompress lzma testfile-lzma.zip {unsupported compression method} -returnCodes error + testuncompress xz testfile-xz.zip {unsupported compression method} -returnCodes error # - # TODO - open of zipfs file + # file stat + test zipfs-file-stat-nosuchfile "Read stat of nonexistent file" -setup { + mount [zippath test.zip] + } -cleanup cleanup -body { + file stat [file join $defaultMountPoint nosuchfile] + } -result "could not read \"[file join $defaultMountPoint nosuchfile]\": no such file or directory" -returnCodes error + + test zipfs-file-stat-nosuchmount "Read stat of nonexistent mount" -body { + file stat [file join $defaultMountPoint nosuchfile] + } -result "could not read \"[file join $defaultMountPoint nosuchfile]\": no such file or directory" -returnCodes error + + test zipfs-file-stat-file "Read stat of file" -setup { + mount [zippath test.zip] + } -cleanup cleanup -body { + lsort -stride 2 [file stat [file join $defaultMountPoint test]] + } -result {atime 1065435402 ctime 1065435402 dev 0 gid 0 ino 0 mode 33133 mtime 1065435402 nlink 0 size 5 type file uid 0} + test zipfs-file-stat-dir "Read stat of dir" -setup { + mount [zippath test.zip] + } -cleanup cleanup -body { + lsort -stride 2 [file stat [file join $defaultMountPoint testdir]] + } -result {atime 1105450434 ctime 1105450434 dev 0 gid 0 ino 0 mode 16749 mtime 1105450434 nlink 0 size 0 type directory uid 0} + + test zipfs-file-stat-mount "Read stat of mount point" -setup { + mount [zippath test.zip] + } -cleanup cleanup -body { + lsort -stride 2 [file stat $defaultMountPoint] + } -result {atime 0 ctime 0 dev 0 gid 0 ino 0 mode 16749 mtime 0 nlink 0 size 0 type directory uid 0} + + test zipfs-file-stat-root-mount "Read stat of root" -setup { + mount [zippath test.zip] [zipfs root] + } -cleanup cleanup -body { + lsort -stride 2 [file stat [zipfs root]] + } -result {atime 0 ctime 0 dev 0 gid 0 ino 0 mode 16749 mtime 0 nlink 0 size 0 type directory uid 0} + + test zipfs-file-stat-root-subdir-mount "Read stat of root when mount is subdir" -setup { + mount [zippath test.zip] + } -cleanup cleanup -constraints knownBug -body { + lsort -stride 2 [file stat [zipfs root]] + } -result {atime 0 ctime 0 dev 0 gid 0 ino 0 mode 16749 mtime 0 nlink 0 size 0 type directory uid 0} # # TODO - glob of zipfs file - # - # TODO - stat of zipfs file + # TODO tests for compress and save # - # TODO - copy, rename etc. + # TODO - file copy, file rename etc. # # Bug regressions -- cgit v0.12 From 748f1c39e564df9766e43843cc48167342d39dc2 Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Thu, 14 Sep 2023 16:30:02 +0000 Subject: Fix more arg count tests. Adapt stat dict for non-windows --- generic/tclZipfs.c | 2 +- tests/zipfs.test | 41 ++++++++++++++++++++++------------------- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/generic/tclZipfs.c b/generic/tclZipfs.c index 71d3c55..6ed23f7 100644 --- a/generic/tclZipfs.c +++ b/generic/tclZipfs.c @@ -3648,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)) { diff --git a/tests/zipfs.test b/tests/zipfs.test index 3bbecb8..44228c7 100644 --- a/tests/zipfs.test +++ b/tests/zipfs.test @@ -90,19 +90,6 @@ test zipfs-0.12 {zipfs basics: join} -constraints {zipfs zipfslib} -body { file normalize ${ziproot}//bar/baz//qux/../ } -result "${ziproot}bar/baz" -test zipfs-1.5 {zipfs errors} -constraints zipfs -returnCodes error -body { - zipfs mkkey a b c d e f -} -result {wrong # args: should be "zipfs mkkey password"} -test zipfs-1.6 {zipfs errors} -constraints zipfs -returnCodes error -body { - zipfs mkimg a b c d e f -} -result {wrong # args: should be "zipfs mkimg outfile indir ?strip? ?password? ?infile?"} -test zipfs-1.7 {zipfs errors} -constraints zipfs -returnCodes error -body { - zipfs mkzip a b c d e f -} -result {wrong # args: should be "zipfs mkzip outfile indir ?strip? ?password?"} -test zipfs-1.9 {zipfs errors} -constraints zipfs -returnCodes error -body { - zipfs info a b c d e f -} -result {wrong # args: should be "zipfs info filename"} - file mkdir tmp test zipfs-2.1 {zipfs mkzip empty archive} -constraints zipfs -returnCodes error -body { zipfs mkzip [file join $tmpdir empty.zip] $tcl_library/xxxx @@ -958,14 +945,22 @@ namespace eval test_ns_zipfs { testuncompress bzip2 testbzip2.zip {unsupported compression method} -returnCodes error testuncompress lzma testfile-lzma.zip {unsupported compression method} -returnCodes error testuncompress xz testfile-xz.zip {unsupported compression method} -returnCodes error + testuncompress zstd testfile-zstd.zip {unsupported compression method} -returnCodes error # # file stat + proc fixupstat {stat} { + if {$::tcl_platform(platform) ne "windows"} { + dict set stat blksize 0 + dict set stat blocks 0 + } + return [lsort -stride 2 $stat] + } test zipfs-file-stat-nosuchfile "Read stat of nonexistent file" -setup { mount [zippath test.zip] } -cleanup cleanup -body { file stat [file join $defaultMountPoint nosuchfile] - } -result "could not read \"[file join $defaultMountPoint nosuchfile]\": no such file or directory" -returnCodes error + } -result "could not read \"[file join $defaultMountPoint nosuchfile]\": *" -match glob -returnCodes error test zipfs-file-stat-nosuchmount "Read stat of nonexistent mount" -body { file stat [file join $defaultMountPoint nosuchfile] @@ -975,31 +970,31 @@ namespace eval test_ns_zipfs { mount [zippath test.zip] } -cleanup cleanup -body { lsort -stride 2 [file stat [file join $defaultMountPoint test]] - } -result {atime 1065435402 ctime 1065435402 dev 0 gid 0 ino 0 mode 33133 mtime 1065435402 nlink 0 size 5 type file uid 0} + } -result [fixupstat {atime 1065435402 ctime 1065435402 dev 0 gid 0 ino 0 mode 33133 mtime 1065435402 nlink 0 size 5 type file uid 0}] test zipfs-file-stat-dir "Read stat of dir" -setup { mount [zippath test.zip] } -cleanup cleanup -body { lsort -stride 2 [file stat [file join $defaultMountPoint testdir]] - } -result {atime 1105450434 ctime 1105450434 dev 0 gid 0 ino 0 mode 16749 mtime 1105450434 nlink 0 size 0 type directory uid 0} + } -result [fixupstat {atime 1105450434 ctime 1105450434 dev 0 gid 0 ino 0 mode 16749 mtime 1105450434 nlink 0 size 0 type directory uid 0}] test zipfs-file-stat-mount "Read stat of mount point" -setup { mount [zippath test.zip] } -cleanup cleanup -body { lsort -stride 2 [file stat $defaultMountPoint] - } -result {atime 0 ctime 0 dev 0 gid 0 ino 0 mode 16749 mtime 0 nlink 0 size 0 type directory uid 0} + } -result [fixupstat {atime 0 ctime 0 dev 0 gid 0 ino 0 mode 16749 mtime 0 nlink 0 size 0 type directory uid 0}] test zipfs-file-stat-root-mount "Read stat of root" -setup { mount [zippath test.zip] [zipfs root] } -cleanup cleanup -body { lsort -stride 2 [file stat [zipfs root]] - } -result {atime 0 ctime 0 dev 0 gid 0 ino 0 mode 16749 mtime 0 nlink 0 size 0 type directory uid 0} + } -result [fixupstat {atime 0 ctime 0 dev 0 gid 0 ino 0 mode 16749 mtime 0 nlink 0 size 0 type directory uid 0}] test zipfs-file-stat-root-subdir-mount "Read stat of root when mount is subdir" -setup { mount [zippath test.zip] } -cleanup cleanup -constraints knownBug -body { lsort -stride 2 [file stat [zipfs root]] - } -result {atime 0 ctime 0 dev 0 gid 0 ino 0 mode 16749 mtime 0 nlink 0 size 0 type directory uid 0} + } -result [fixupstat {atime 0 ctime 0 dev 0 gid 0 ino 0 mode 16749 mtime 0 nlink 0 size 0 type directory uid 0}] # # TODO - glob of zipfs file @@ -1009,6 +1004,14 @@ namespace eval test_ns_zipfs { # # TODO - file copy, file rename etc. + + # TODO - mkkey, mkimg, mkzip, lmkimg, lmkzip + testnumargs "zipfs mkkey" "password" "" -constraints zipfs + testnumargs "zipfs mkimg" "outfile indir" "?strip? ?password? ?infile?" + testnumargs "zipfs lmkimg" "outfile inlist" "?password? ?infile?" + testnumargs "zipfs mkzip" "outfile indir" "?strip? ?password?" + testnumargs "zipfs lmkzip" "outfile inlist" "?password?" + # # Bug regressions -- cgit v0.12 From 53287645760f083e2fbaf93ea73ec0f1992ca67d Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Fri, 15 Sep 2023 06:05:01 +0000 Subject: zipfs password tests --- doc/zipfs.n | 12 ++++++++---- tests/zipfiles/README | 2 +- tests/zipfs.test | 46 +++++++++++++++++++++++++++++++++++++++------- 3 files changed, 48 insertions(+), 12 deletions(-) diff --git a/doc/zipfs.n b/doc/zipfs.n index b59e072..247cac2 100644 --- a/doc/zipfs.n +++ b/doc/zipfs.n @@ -35,11 +35,14 @@ zipfs \- Mount and work with ZIP files within Tcl .BE .SH DESCRIPTION .PP -The \fBzipfs\fR command (the sole public command provided by the built-in -package with the same name) provides Tcl with the ability to mount the -contents of a ZIP archive file as a virtual file system. ZIP archives support +The \fBzipfs\fR command provides Tcl with the ability to mount the +contents of a ZIP archive file as a virtual file system. Tcl's ZIP +archive support is limited to basic features and options. +Supported storage methods include only STORE and DEFLATE with optional simple encryption, sufficient to prevent casual inspection of their contents but not able to prevent access by even a moderately determined attacker. +Strong encryption, multi-part archives, platform metadata, +zip64 formats and other compression methods like bzip2 are not supported. .TP \fBzipfs canonical\fR ?\fImountpoint\fR? \fIfilename\fR ?\fIinZipfs\fR? . @@ -125,7 +128,8 @@ to access the files inside the mount). Returns a constant string which indicates the mount point for zipfs volumes for the current platform. This value is -.QW \fB//zipfs:/\fR . +.QW \fB//zipfs:/\fR +on most platforms. .TP \fBzipfs unmount \fImountpoint\fR . diff --git a/tests/zipfiles/README b/tests/zipfiles/README index e6c366e..38ce998 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-zip-in-zip.zip - Tcl's license +test-overlay.zip, test-password.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 44228c7..4edb629 100644 --- a/tests/zipfs.test +++ b/tests/zipfs.test @@ -665,7 +665,7 @@ namespace eval test_ns_zipfs { mount [zippath test.zip] set newmount [file join [zipfs root] test testdir] mount [zippath test-overlay.zip] $newmount - } -constraints knownBug -cleanup { + } -constraints bug-4ae42446ab -cleanup { cleanup } -body { # KNOWN BUG. The test2 file is also present in parent mount. @@ -712,7 +712,7 @@ namespace eval test_ns_zipfs { # Some tests have !zipfslib constraint because otherwise they dump the entire Tcl library which is mounted on root testzipfslist no-mounts "" {} {} -constraints !zipfslib testzipfslist no-pattern "" {test.zip testmountA} {testmountA testmountA/test testmountA/testdir testmountA/testdir/test2} -constraints !zipfslib - testzipfslist no-pattern-mount-on-root "" {test.zip {}} {{} test testdir testdir/test2} -constraints knownBug + testzipfslist no-pattern-mount-on-root "" {test.zip {}} {{} test testdir testdir/test2} -constraints bug-d056ee6d30 testzipfslist no-pattern-multiple "" {test.zip testmountA test.zip testmountB/subdir} { testmountA testmountA/test testmountA/testdir testmountA/testdir/test2 testmountB/subdir testmountB/subdir/test testmountB/subdir/testdir testmountB/subdir/testdir/test2 @@ -749,7 +749,7 @@ namespace eval test_ns_zipfs { testzipfsexists file [file join $defaultMountPoint test] 1 testzipfsexists dir [file join $defaultMountPoint testdir] 1 testzipfsexists mountpoint $defaultMountPoint 1 - testzipfsexists root [zipfs root] 1 -constraints knownBug + testzipfsexists root [zipfs root] 1 -constraints bug-02acab5aea # # zipfs find @@ -838,7 +838,7 @@ namespace eval test_ns_zipfs { test.zip {} test.zip testmountB/subdir } [lmap path { test testdir testdir/test2 - } {file join [zipfs root] $path}] -constraints knownBug + } {file join [zipfs root] $path}] -constraints bug-6183f535c8 # # zipfs info @@ -928,7 +928,7 @@ namespace eval test_ns_zipfs { set zippath [zippath $zippath] test zipfs-uncompress-$id $id -setup { unset -nocomplain fd - mount $zippath + zipfs mount $zippath $defaultMountPoint } -cleanup { # In case mount succeeded when it should not if {[info exists fd]} { @@ -947,6 +947,38 @@ namespace eval test_ns_zipfs { testuncompress xz testfile-xz.zip {unsupported compression method} -returnCodes error testuncompress zstd testfile-zstd.zip {unsupported compression method} -returnCodes error + proc testpassword {id filename password result args} { + variable defaultMountPoint + set zippath [zippath test-password.zip] + test zipfs-password-read-$id $id -setup { + unset -nocomplain fd + if {$password ne ""} { + zipfs mount $zippath $defaultMountPoint $password + } else { + zipfs mount $zippath $defaultMountPoint + } + } -cleanup { + # In case mount succeeded when it should not + if {[info exists fd]} { + close $fd + } + cleanup + } -body { + set fd [open [file join $defaultMountPoint $filename]] + gets $fd + } -result $result {*}$args + } + testpassword plain plain.txt password plaintext + testpassword plain-nopassword plain.txt "" plaintext + testpassword plain-badpassword plain.txt xxx plaintext + testpassword cipher cipher.bin password ciphertext + testpassword cipher-nopassword cipher.bin {} "decryption failed" -returnCodes error + testpassword cipher-badpassword cipher.bin xxx "decryption failed" -returnCodes error -constraints bug-b3c7429255 + testpassword cipher-deflate cipher-deflate.bin password [lseq 100] + testpassword cipher-deflate-nopassword cipher-deflate.bin {} "decryption failed" -returnCodes error + testpassword cipher-deflate-badpassword cipher-deflate.bin xxx "decryption failed" -returnCodes error bug-b3c7429255 + + # # file stat proc fixupstat {stat} { @@ -992,14 +1024,14 @@ namespace eval test_ns_zipfs { test zipfs-file-stat-root-subdir-mount "Read stat of root when mount is subdir" -setup { mount [zippath test.zip] - } -cleanup cleanup -constraints knownBug -body { + } -cleanup cleanup -constraints bug-02acab5aea -body { lsort -stride 2 [file stat [zipfs root]] } -result [fixupstat {atime 0 ctime 0 dev 0 gid 0 ino 0 mode 16749 mtime 0 nlink 0 size 0 type directory uid 0}] # # TODO - glob of zipfs file - # TODO tests for compress and save + # TODO tests for compress and save, + with password # # TODO - file copy, file rename etc. -- cgit v0.12