archive/zip: potential bug with zip64 check #56249
Comments
|
@dsnet can you give this a look? |
dr2chase
added
the
NeedsInvestigation
|
I'll take a look next week. This week is a bit busier. |
|
According to section 4.4.23 of https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT:
@kleeon is correct that it should compare with The typo does not cause any issue because:
That said, the typo should be fixed anyway. I used this code to generate a zip file that has a large size of the central directory. Both package main
import (
"archive/zip"
"fmt"
"os"
"strings"
)
func main() {
// The file size will be 4.1G.
archive, err := os.Create("z.zip")
if err != nil {
panic(err)
}
defer archive.Close()
zipWriter := zip.NewWriter(archive)
defer zipWriter.Close()
comment := strings.Repeat("1", 64<<10-1)
for i := 0; i < 65428; i++ {
name := fmt.Sprintf("%060x.txt", i)
h := &zip.FileHeader{
Name: name,
Comment: comment,
}
_, err := zipWriter.CreateHeader(h)
if err != nil {
panic(err)
}
}
} |
|
@ZekeLu I decided to take another look at this issue.
You could actually do some weird stuff by putting a spurious EOCD64 and EOCD64 locator into the comment of the last file, then using the same comment to pad Central Directory size to 0xFFFF bytes. Like so: package main
import (
"archive/zip"
"fmt"
"os"
"strings"
)
func createFile() {
file, _ := os.Create("arch.zip")
defer file.Close()
arch := zip.NewWriter(file)
defer arch.Close()
name := "real_file.txt"
arch.CreateHeader(&zip.FileHeader{
Name: name,
})
zip64Locator := "\x50\x4B\x06\x07" + // signature
"\x00\x00\x00\x00" + // disk number
"\x2C\x00\x01\x00\x00\x00\x00\x00" + // EOCD64 location
"\x01\x00\x00\x00" // number of disks
zip64Eocd := "\x50\x4B\x06\x06" + // signature
"\x00\x00\x00\x00\x00\x00\x00\x00" + // size of eocd64, ignored
"\x00\x00\x00\x00" + // version info
"\x00\x00\x00\x00" + // disk number
"\x00\x00\x00\x00" + // central directory start disk number
"\x01\x00\x00\x00\x00\x00\x00\x00" + // number of CD records on this disk
"\x01\x00\x00\x00\x00\x00\x00\x00" + // number of CD records
"\x3B\x00\x00\x00\x00\x00\x00\x00" + // size of CD
"\xF1\xFF\x00\x00\x00\x00\x00\x00" // offset to fake CD
fakeFileCDRecord := "\x50\x4b\x01\x02" +
"\x00\x00\x00\x00" + // version info
"\x00\x00" + // flags
"\x00\x00" + // compression method, no compression
"\x00\x00\x00\x00" + // modification time and date
"\x00\x00\x00\x00" + // crc-32
"\x00\x00\x00\x00" + // compressed size, empty file
"\x00\x00\x00\x00" + // uncompressed size
"\x0D\x00" + // file name length
"\x00\x00" + // extra field length
"\x00\x00" + // comment length
"\x00\x00" + // disk number
"\x00\x00\x00\x00\x00\x00" + // file attributes
"\x00\x00\x00\x00" + // local file header offset, empty file so don't care
"fake_file.txt"
padding := strings.Repeat("\x00", 65279)
arch.CreateHeader(&zip.FileHeader{
Name: "payload_file.txt",
Comment: padding + fakeFileCDRecord + zip64Eocd + zip64Locator,
})
}
func readFile() {
file, _ := os.Open("arch.zip")
stat, _ := file.Stat()
arch, err := zip.NewReader(file, stat.Size())
if err != nil {
panic(err)
}
fmt.Println("Files in archive:")
for _, f := range arch.File {
fmt.Println(f.Name)
}
}
func main() {
createFile()
readFile()
}In this example I put a fake file into the comment of While opening this same archive with WinRAR looks like this:
Probably not a huge security concern, but definitely should be fixed. |
What version of Go are you using (
go version)?There seems to be a bug with zip64 check on this line:
go/src/archive/zip/reader.go
Line 557 in 9ddb8ea
Specifically the
d.directorySize == 0xffffpart. Size of central directory is a 32-bit value so it should be compared to 0xffffffff.The text was updated successfully, but these errors were encountered: