archive/tar: reader returns bogus headers
dvyukov opened this issue · 9 comments
The following program crashes:
package main
import (
"archive/tar"
"bytes"
"io"
)
func main() {
t := tar.NewReader(bytes.NewReader([]byte(data)))
var headers []*tar.Header
for {
hdr, err := t.Next()
if err == io.EOF {
break
}
if err != nil {
return
}
hdr1 := *hdr // make a copy to be safe
headers = append(headers, &hdr1)
}
buf := new(bytes.Buffer)
w := tar.NewWriter(buf)
for _, hdr := range headers {
err := w.WriteHeader(hdr)
if err != nil {
panic(err)
}
}
}
var data = "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" +
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" +
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" +
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" +
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01" +
"\xff\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" +
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" +
"\x00\x00\x00\x00\x00\x00\x00\x0000000000\x00\x00\x00\x00" +
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" +
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" +
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" +
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" +
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" +
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" +
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" +
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" +
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" +
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\t" +
"\xbf\x97KǸ\xd4\x03\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" +
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" +
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" +
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" +
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" +
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" +
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" +
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
panic: archive/tar: header field too long
The problem is that reader produces the following header:
tar.Header{Name:"", Mode:9151314442816847872, Uid:0, Gid:0, Size:0, ModTime:time.Time{sec:62135596800, nsec:0, loc:(*time.Location)(0x5d6f40)}, Typeflag:0x0, Linkname:"", Uname:"", Gname:"", Devmajor:0, Devminor:0, AccessTime:time.Time{sec:0, nsec:0, loc:(*time.Location)(nil)}, ChangeTime:time.Time{sec:0, nsec:0, loc:(*time.Location)(nil)}, Xattrs:map[string]string(nil)}
Mode is bogus. Then, cString tries to serialize that mode in octal format into 8-byte buffer. It does not fit.
I've inserted panic when this error is generated, and there is output:
b="\x00\x00\x00\x00\x00\x00\x00\x00"
s="774000000000000000000"
panic: archive/tar: header field too long
goroutine 1 [running]:
archive/tar.(*Writer).cString(0xc820090480, 0xc820090517, 0x8, 0x19c, 0xc8200ae0e0, 0x15, 0x505f00, 0x0, 0x0, 0x0)
src/archive/tar/writer.go:83 +0x333
archive/tar.(*Writer).octal(0xc820090480, 0xc820090517, 0x8, 0x19c, 0x7f00000000000000)
src/archive/tar/writer.go:102 +0xea
archive/tar.(*Writer).writeHeader(0xc820090480, 0xc8200b8f70, 0x517701, 0x0, 0x0)
src/archive/tar/writer.go:193 +0x651
archive/tar.(*Writer).WriteHeader(0xc820090480, 0xc8200b8f70, 0x0, 0x0)
src/archive/tar/writer.go:142 +0x3c
main.main()
/tmp/tar.go:26 +0x336
exit status 2
go version devel +9c514e1 Tue Sep 1 04:00:12 2015 +0000 linux/amd64
I've only tackled subsecond atime, ctime, and mtime. Don't let me stop you from working on the rest of those issues.
Copy. I'll tackle this, this weekend.
This should be renamed as: "archive/tar: writer does not support writing base-256 fields"
The issue at hand is not that the reader is giving bogus headers, but that the writer isn't able to encode the headers back since it doesn't support the base-256 format that GNU added.
I recommend closing this as "won't fix". Currently, the tar writer already outputs the Pax format when the header information extends beyond what the original tar header was able to encode. Thus, this prevents the use of any other format type, otherwise the output tar file will neither be Pax nor GNU nor BSD; it will be some weird and incompatible mix of two or more formats. It is fortunate (or unfortunate) that the writer already chose Pax as its default extended output format. Thus, it should not use extensions of any other format.
A peruse of the pax code shows that it does not support reading base256 encoded fields, thus proving that it is not safe to mix some of the exotic GNU/BSD extensions into Pax.
@dsymonds If he agrees.
EDIT: I misread the code and it seems that tar does dynamically choose what format to use based on the fields it needs to encode. As I was afraid, this does cause the writer to potentially write conflicting formats if not careful. Issue #9683 is caused by this phenomenon. This will probably be a good time to carefully look through tar and make sure it doesn't conflict with itself.
As a side note, I should mention that the reader incorrectly reads base256 encoded fields in that it does not handle negative numbers. I'll submit a CL for that. For reference:
The use case of negative numbers allows for timestamps before 1970, and also allows Go's tar implementation to properly reject Tar files with negative file sizes and what not.
CL https://golang.org/cl/14623 mentions this issue.
Still not working. Oh well.
CL https://golang.org/cl/17425 mentions this issue.