golang/go

archive/zip: unexpected EOF

dvyukov opened this issue · 5 comments

The following program fails with the panic:

package main

import (
    "archive/zip"
    "bytes"
    "io/ioutil"
)

func main() {
    headers := []zip.FileHeader{
        {Name:"0", Method:8},
        {Name:"1", Method:8},
    }
    contents := [][]byte{
        []byte("a"),
        []byte("abcdefgh01234567890"),
    }
    buf := new(bytes.Buffer)
    w := zip.NewWriter(buf)
    for i, h := range headers {
        w1, err := w.CreateHeader(&h)
        if err != nil {
            panic(err)
        }
        _, err = w1.Write(contents[i])
        if err != nil {
            panic(err)
        }
    }
    err := w.Close()
    if err != nil {
        panic(err)
    }

    z1, err := zip.NewReader(bytes.NewReader(buf.Bytes()), int64(len(buf.Bytes())))
    if err != nil {
        panic(err)
    }
    var headers1 []zip.FileHeader
    var contents1 [][]byte
    for _, f := range z1.File {
        r, err := f.Open()
        if err != nil {
            panic(err)
        }
        c, err := ioutil.ReadAll(r)
        if err != nil {
            panic(err)
        }
        headers1 = append(headers1, f.FileHeader)
        contents1 = append(contents1, c)
        r.Close()
    }
}
panic: unexpected EOF

goroutine 1 [running]:
main.main()
    zip.go:48 +0x8f8

go version devel +b0532a9 Mon Jun 8 05:13:15 2015 +0000 linux/amd64

Oh, I see that CreateHeader retains the &h pointer passed to it. As the result all files in the archive share the same header. This explains everything.
But it is quite confusing and at least must be documented.

This user bug has quite obscure failure mode that is discovered only during reading. Writing finishes successfully producing a corrupted archive. So probably it makes sense to diagnose this misuse. Note that we don't need to remember/compare all header pointers for this, checking only the previous pointer would be enough.

Sure, but I get to take your go-fuzz trophy back.

Well, this is discovered due to go-fuzz, one way or another :)

CL https://golang.org/cl/10883 mentions this issue.