From e7bd5573df5961237ca1c08952aee3c2493e3eb5 Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Sat, 23 Sep 2023 13:36:10 +0000 Subject: Bug [4645658689] - zipfs file truncation with open r+ --- generic/tclZipfs.c | 65 +++++++++++++++++++++++-------------- tests/zipfs.test | 95 +++++++++++++++++++++++++++++++++++++----------------- 2 files changed, 107 insertions(+), 53 deletions(-) diff --git a/generic/tclZipfs.c b/generic/tclZipfs.c index 45a2041..b0dbd7d 100644 --- a/generic/tclZipfs.c +++ b/generic/tclZipfs.c @@ -4628,18 +4628,16 @@ InitWritableChannel( */ info->numBytes = 0; + z->crc32 = 0; /* Truncated, CRC no longer applicable */ } else if (z->data) { /* * Already got uncompressed data. */ + if (z->numBytes > info->maxWrite) + goto tooBigError; - unsigned int j = z->numBytes; - - if (j > info->maxWrite) { - j = info->maxWrite; - } - memcpy(info->ubuf, z->data, j); - info->numBytes = j; + memcpy(info->ubuf, z->data, z->numBytes); + info->numBytes = z->numBytes; } else { /* * Need to uncompress the existing data. @@ -4697,39 +4695,53 @@ InitWritableChannel( } err = inflate(&stream, Z_SYNC_FLUSH); inflateEnd(&stream); - if ((err == Z_STREAM_END) - || ((err == Z_OK) && (stream.avail_in == 0))) { - if (cbuf) { - memset(info->keys, 0, sizeof(info->keys)); - ckfree(cbuf); - } - return TCL_OK; + if ((err != Z_STREAM_END) && + ((err != Z_OK) || (stream.avail_in != 0))) { + goto corruptionError; + } + /* Even if decompression succeeded, counts should be as expected */ + if (stream.total_out != z->numBytes) + goto corruptionError; + info->numBytes = z->numBytes; + if (cbuf) { + ckfree(cbuf); } - goto corruptionError; } else if (z->isEncrypted) { /* * Need to decrypt some otherwise-simple stored data. */ - - for (i = 0; i < z->numBytes - 12; i++) { + if (z->numCompressedBytes <= 12 || + (z->numCompressedBytes - 12) != z->numBytes) + goto corruptionError; + int len = z->numCompressedBytes - 12; + for (i = 0; i < len; i++) { ch = zbuf[i]; info->ubuf[i] = zdecode(info->keys, crc32tab, ch); } - } else { + info->numBytes = len; + } + else { /* * Simple stored data. Copy into our working buffer. */ - memcpy(info->ubuf, zbuf, z->numBytes); + info->numBytes = z->numBytes; } memset(info->keys, 0, sizeof(info->keys)); } + + assert(info->numBytes == 0 || info->numBytes == z->numBytes); return TCL_OK; memoryError: ZIPFS_MEM_ERROR(interp); goto error_cleanup; + tooBigError: + ZIPFS_ERROR(interp, "file size exceeds max writable"); + ZIPFS_ERROR_CODE(interp, "TOOBIG"); + goto error_cleanup; + corruptionError: if (cbuf) { memset(info->keys, 0, sizeof(info->keys)); @@ -4857,6 +4869,9 @@ InitReadableChannel( && ((err != Z_OK) || (stream.avail_in != 0))) { goto corruptionError; } + /* Even if decompression succeeded, counts should be as expected */ + if (stream.total_out != z->numBytes) + goto corruptionError; if (ubuf) { info->isEncrypted = 0; @@ -4871,7 +4886,8 @@ InitReadableChannel( * Decode encrypted but uncompressed file, since we support Tcl_Seek() * on it, and it can be randomly accessed later. */ - + if (z->numCompressedBytes <= 12 || (z->numCompressedBytes - 12) != z->numBytes) + goto corruptionError; len = z->numCompressedBytes - 12; ubuf = (unsigned char *) attemptckalloc(len); if (ubuf == NULL) { @@ -5011,14 +5027,14 @@ ZipFSOpenFileChannelProc( int mode, TCL_UNUSED(int) /* permissions */) { - int trunc = (mode & O_TRUNC) != 0; - int wr = (mode & (O_WRONLY | O_RDWR)) != 0; - pathPtr = Tcl_FSGetNormalizedPath(NULL, pathPtr); if (!pathPtr) { return NULL; } + int trunc = (mode & O_TRUNC) != 0; + int wr = (mode & (O_WRONLY | O_RDWR)) != 0; + /* * Check for unsupported modes. */ @@ -5027,7 +5043,8 @@ ZipFSOpenFileChannelProc( Tcl_SetErrno(EACCES); if (interp) { Tcl_SetObjResult(interp, Tcl_ObjPrintf( - "write access not supported: %s", + "%s not supported: %s", + mode & O_APPEND ? "append mode" : "write access", Tcl_PosixError(interp))); } return NULL; diff --git a/tests/zipfs.test b/tests/zipfs.test index 5d20084..0275234 100644 --- a/tests/zipfs.test +++ b/tests/zipfs.test @@ -410,6 +410,11 @@ namespace eval test_ns_zipfs { } } + # list of paths -> list of paths under [zipfs root] + proc zipfspaths {args} { + return [lmap path $args {file join [zipfs root] $path}] + } + proc cleanup {} { dict for {mount -} [zipfs mount] { if {[string match //zipfs:/test* $mount]} { @@ -701,6 +706,7 @@ namespace eval test_ns_zipfs { testzipfslist no-pattern-mount-on-empty "" {test.zip {}} {{} test testdir testdir/test2} -constraints !zipfslib testzipfslist no-pattern-mount-on-root "" [list test.zip [zipfs root]] {{} test testdir testdir/test2} -constraints !zipfslib testzipfslist no-pattern-mount-on-slash "" [list test.zip /] {{} test testdir testdir/test2} -constraints !zipfslib + testzipfslist no-pattern-mount-on-level3 "" [list test.zip testmt/a/b] {{} testmt testmt/a testmt/a/b testmt/a/b/test testmt/a/b/testdir testmt/a/b/testdir/test2} -constraints {bug-02acab5aea !zipfslib} 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 @@ -723,21 +729,25 @@ namespace eval test_ns_zipfs { testnumargs "zipfs exists" "filename" "" # Generates tests for zipfs exists - proc testzipfsexists {id path result args} { + proc testzipfsexists [list id path result [list mountpoint $defaultMountPoint] args] { test zipfs-exists-$id "zipfs exists $id" -body { zipfs exists $path } -setup { - mount [zippath test.zip] + mount [zippath test.zip] $mountpoint } -cleanup { + zipfs unmount $mountpoint 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 bug-02acab5aea + testzipfsexists file [file join $defaultMountPoint test] 1 + testzipfsexists dir [file join $defaultMountPoint testdir] 1 + testzipfsexists mountpoint $defaultMountPoint 1 + testzipfsexists root [zipfs root] 1 \ + $defaultMountPoint -constraints bug-02acab5aea + testzipfsexists level3 [file join $defaultMountPoint a b] 1 \ + [file join $defaultMountPoint a b c] -constraints bug-02acab5aea # # zipfs find @@ -779,12 +789,9 @@ namespace eval test_ns_zipfs { test.zip testmountA test.zip testmountB/subdir } {} - variable path 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}] + } [zipfspaths testmountA/test testmountA/testdir testmountA/testdir/test2] testzipfsfind relative-path testdir { test.zip testmountA test.zip testmountB/subdir @@ -798,9 +805,17 @@ namespace eval test_ns_zipfs { # bug-6183f535c8 testzipfsfind root-path [zipfs root] { test.zip {} test.zip testmountB/subdir - } [lmap path { - test testdir testdir/test2 - } {file join [zipfs root] $path}] -constraints !zipfslib + } [zipfspaths test testdir testdir/test2] -constraints !zipfslib + + testzipfsfind level3 [file join [zipfs root] testmt a] { + test.zip testmt/a/b + } [zipfspaths testmt/a/b testmt/a/b/test testmt/a/b/testdir testmt/a/b/testdir/test2] \ + -constraints bug-02acab5aea + + testzipfsfind level3-root [zipfs root] { + test.zip testmt/a/b + } [zipfspaths testmt testmt/a testmt/a/b testmt/a/b/test testmt/a/b/testdir testmt/a/b/testdir/test2] \ + -constraints bug-02acab5aea test zipfs-find-native-absolute "zipfs find on native file system" -setup { set dir [makeDirectory zipfs-native-absolute] @@ -830,7 +845,6 @@ namespace eval test_ns_zipfs { zipfs find [file tail $dir] } -result {zipfs-native-relative/subdir zipfs-native-relative/subdir/native} - # # zipfs info testnumargs "zipfs info" "filename" "" @@ -876,6 +890,15 @@ namespace eval test_ns_zipfs { zipfs info $defaultMountPoint } -result [list [zippath junk-at-start.zip] 0 0 4] + test zipfs-info-level3 "zipfs info on mount point - verify correct offset of zip content" -setup { + # zip starts at offset 4 + mount [zippath junk-at-start.zip] /testmt/a/b + } -cleanup { + cleanup + } -body { + zipfs info [file join [zipfs root] testmt a] + } -result {{} 0 0 0} -constraints bug-02acab5aea + # # zipfs canonical - # TODO - semantics are very unclear. Can produce nonsensical paths like @@ -907,14 +930,9 @@ namespace eval test_ns_zipfs { testzipfscanonical backslashes X:\\\\foo\\\\bar [file join [zipfs root] foo bar] -constraints win testzipfscanonical backslashes-1 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 {filename abac-repeat.txt} args} { + proc testuncompress {id zippath result {filename abac-repeat.txt} {openopts {}} args} { variable defaultMountPoint set zippath [zippath $zippath] test zipfs-uncompress-$id "zipfs uncompress $id" -setup { @@ -927,17 +945,26 @@ namespace eval test_ns_zipfs { } cleanup } -body { - set fd [open [file join $defaultMountPoint $filename]] + set fd [open [file join $defaultMountPoint $filename] {*}$openopts] gets $fd } -result $result {*}$args } + testuncompress stored test.zip test test testuncompress stored teststored.zip aaaaaaaaaaaaaa testuncompress deflate testdeflated2.zip aaaaaaaaaaaaaa - testuncompress deflate-error broken.zip {decompression error} deflatezliberror -returnCodes error - testuncompress bzip2 testbzip2.zip {unsupported compression method} abac-repeat.txt -returnCodes error - testuncompress lzma testfile-lzma.zip {unsupported compression method} abac-repeat.txt -returnCodes error - testuncompress xz testfile-xz.zip {unsupported compression method} abac-repeat.txt -returnCodes error - testuncompress zstd testfile-zstd.zip {unsupported compression method} abac-repeat.txt -returnCodes error + # Test open modes - see bug [4645658689] + testuncompress stored-rw teststored.zip aaaaaaaaaaaaaa abac-repeat.txt r+ + testuncompress deflate-rw testdeflated2.zip aaaaaaaaaaaaaa abac-repeat.txt r+ + testuncompress stored-wr teststored.zip {} abac-repeat.txt w+ -constraints bug-00018ec7a0 + testuncompress deflate-wr testdeflated2.zip {} abac-repeat.txt w+ -constraints bug-00018ec7a0 + testuncompress stored-ar teststored.zip {} abac-repeat.txt a+ + testuncompress deflate-ar testdeflated2.zip {} abac-repeat.txt a+ + + testuncompress deflate-error broken.zip {decompression error} deflatezliberror {} -returnCodes error + testuncompress bzip2 testbzip2.zip {unsupported compression method} abac-repeat.txt {} -returnCodes error + testuncompress lzma testfile-lzma.zip {unsupported compression method} abac-repeat.txt {} -returnCodes error + testuncompress xz testfile-xz.zip {unsupported compression method} abac-repeat.txt {} -returnCodes error + testuncompress zstd testfile-zstd.zip {unsupported compression method} abac-repeat.txt {} -returnCodes error proc testpassword {id filename password result args} { variable defaultMountPoint @@ -1066,12 +1093,22 @@ namespace eval test_ns_zipfs { 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}] - test zipfs-file-stat-root-subdir-mount "Read stat of root when mount is subdir" -setup { + test zipfs-file-stat-root-subdir-mount "Read stat of root when mount is subdir" -constraints { + bug-02acab5aea + } -setup { mount [zippath test.zip] - } -cleanup cleanup -constraints bug-02acab5aea -body { + } -cleanup cleanup -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}] + test zipfs-file-stat-level3 "Stat on a directory that is intermediary in a mount point" -constraints { + bug-02acab5aea + } -setup { + mount [zippath test.zip] [file join $defaultMountPoint mt2] + } -cleanup cleanup -body { + lsort -stride 2 [file stat $defaultMountPoint] + } + # # glob of zipfs file proc testzipfsglob {id mountpoint pat result {globopt {}} args} { -- cgit v0.12