Skip to content

Commit

Permalink
archive/tar, archive/zip: disable insecure file name checks with GODEBUG
Browse files Browse the repository at this point in the history
Add GODEBUG=tarinsecurepath=1 and GODEBUG=zipinsecurepath=1 settings
to disable file name validation.

For #55356.

Change-Id: Iaacdc629189493e7ea3537a81660215a59dd40a4
Reviewed-on: https://go-review.googlesource.com/c/go/+/452495
Reviewed-by: Bryan Mills <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Damien Neil <[email protected]>
Reviewed-by: Russ Cox <[email protected]>
Reviewed-by: Heschi Kreinick <[email protected]>
  • Loading branch information
neild committed Nov 21, 2022
1 parent f60c770 commit 85a2c19
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 1 deletion.
8 changes: 8 additions & 0 deletions doc/go1.20.html
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,10 @@ <h3 id="minor_library_changes">Minor changes to the library</h3>
Programs that want to operate on archives containing insecure file names may
ignore this error.
</p>
<p>
Insecure tar file name checks may be entirely disabled by setting the
<code>GODEBUG=tarinsecurepath=1</code> environment variable.
</p>
</dd>
</dl><!-- archive/tar -->

Expand All @@ -308,6 +312,10 @@ <h3 id="minor_library_changes">Minor changes to the library</h3>
Programs that want to operate on archives containing insecure file names may
ignore this error.
</p>
<p>
Insecure zip file name checks may be entirely disabled by setting the
<code>GODEBUG=zipinsecurepath=1</code> environment variable.
</p>
<p><!-- CL 449955 -->
Reading from a directory file that contains file data will now return an error.
The zip specification does not permit directory files to contain file data,
Expand Down
3 changes: 3 additions & 0 deletions src/archive/tar/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ package tar
import (
"errors"
"fmt"
"internal/godebug"
"io/fs"
"math"
"path"
Expand All @@ -26,6 +27,8 @@ import (
// architectures. If a large value is encountered when decoding, the result
// stored in Header will be the truncated version.

var tarinsecurepath = godebug.New("tarinsecurepath")

var (
ErrHeader = errors.New("archive/tar: invalid tar header")
ErrWriteTooLong = errors.New("archive/tar: write too long")
Expand Down
2 changes: 1 addition & 1 deletion src/archive/tar/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func (tr *Reader) Next() (*Header, error) {
}
hdr, err := tr.next()
tr.err = err
if err == nil && !filepath.IsLocal(hdr.Name) {
if err == nil && tarinsecurepath.Value() != "1" && !filepath.IsLocal(hdr.Name) {
err = ErrInsecurePath
}
return hdr, err
Expand Down
20 changes: 20 additions & 0 deletions src/archive/tar/reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1617,6 +1617,7 @@ func TestFileReader(t *testing.T) {
}

func TestInsecurePaths(t *testing.T) {
t.Setenv("GODEBUG", "tarinsecurepath=0")
for _, path := range []string{
"../foo",
"/foo",
Expand Down Expand Up @@ -1652,3 +1653,22 @@ func TestInsecurePaths(t *testing.T) {
}
}
}

func TestDisableInsecurePathCheck(t *testing.T) {
t.Setenv("GODEBUG", "tarinsecurepath=1")
var buf bytes.Buffer
tw := NewWriter(&buf)
const name = "/foo"
tw.WriteHeader(&Header{
Name: name,
})
tw.Close()
tr := NewReader(&buf)
h, err := tr.Next()
if err != nil {
t.Fatalf("tr.Next with tarinsecurepath=1: got err %v, want nil", err)
}
if h.Name != name {
t.Fatalf("tr.Next with tarinsecurepath=1: got name %q, want %q", h.Name, name)
}
}
6 changes: 6 additions & 0 deletions src/archive/zip/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"errors"
"hash"
"hash/crc32"
"internal/godebug"
"io"
"io/fs"
"os"
Expand All @@ -21,6 +22,8 @@ import (
"time"
)

var zipinsecurepath = godebug.New("zipinsecurepath")

var (
ErrFormat = errors.New("zip: not a valid zip file")
ErrAlgorithm = errors.New("zip: unsupported compression algorithm")
Expand Down Expand Up @@ -108,6 +111,9 @@ func NewReader(r io.ReaderAt, size int64) (*Reader, error) {
// Zip permits an empty file name field.
continue
}
if zipinsecurepath.Value() == "1" {
continue
}
// The zip specification states that names must use forward slashes,
// so consider any backslashes in the name insecure.
if !filepath.IsLocal(f.Name) || strings.Contains(f.Name, `\`) {
Expand Down
26 changes: 26 additions & 0 deletions src/archive/zip/reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1290,6 +1290,7 @@ func TestFSModTime(t *testing.T) {
}

func TestCVE202127919(t *testing.T) {
t.Setenv("GODEBUG", "zipinsecurepath=0")
// Archive containing only the file "../test.txt"
data := []byte{
0x50, 0x4b, 0x03, 0x04, 0x14, 0x00, 0x08, 0x00,
Expand Down Expand Up @@ -1411,6 +1412,7 @@ func TestCVE202139293(t *testing.T) {
}

func TestCVE202141772(t *testing.T) {
t.Setenv("GODEBUG", "zipinsecurepath=0")
// Archive contains a file whose name is exclusively made up of '/', '\'
// characters, or "../", "..\" paths, which would previously cause a panic.
//
Expand Down Expand Up @@ -1586,6 +1588,7 @@ func TestIssue54801(t *testing.T) {
}

func TestInsecurePaths(t *testing.T) {
t.Setenv("GODEBUG", "zipinsecurepath=0")
for _, path := range []string{
"../foo",
"/foo",
Expand Down Expand Up @@ -1616,3 +1619,26 @@ func TestInsecurePaths(t *testing.T) {
}
}
}

func TestDisableInsecurePathCheck(t *testing.T) {
t.Setenv("GODEBUG", "zipinsecurepath=1")
var buf bytes.Buffer
zw := NewWriter(&buf)
const name = "/foo"
_, err := zw.Create(name)
if err != nil {
t.Fatalf("zw.Create(%q) = %v", name, err)
}
zw.Close()
zr, err := NewReader(bytes.NewReader(buf.Bytes()), int64(buf.Len()))
if err != nil {
t.Fatalf("NewReader with zipinsecurepath=1: got err %v, want nil", err)
}
var gotPaths []string
for _, f := range zr.File {
gotPaths = append(gotPaths, f.Name)
}
if want := []string{name}; !reflect.DeepEqual(gotPaths, want) {
t.Errorf("NewReader with zipinsecurepath=1: got files %q, want %q", gotPaths, want)
}
}

0 comments on commit 85a2c19

Please sign in to comment.