--- /dev/null
+pkg archive/tar, var ErrInsecurePath error #55356
+pkg archive/zip, var ErrInsecurePath error #55356
TODO: complete this section
</p>
+<dl id="archive/tar"><dt><a href="/pkg/archive/tar/">archive/tar</a></dt>
+ <dd>
+ <p><!-- https://go.dev/issue/55356 -->
+ <code>(*Reader).Next</code> will now return the error <code>ErrInsecurePath</code>
+ when opening an archive which contains file names that are absolute,
+ refer to a location outside the current directory, contain invalid
+ characters, or (on Windows) are reserved names such as <code>NUL</code>.
+ </p>
+ <p>
+ Programs that want to operate on archives containing insecure file names may
+ ignore this error.
+ </p>
+ </dd>
+</dl><!-- archive/tar -->
+
<dl id="archive/zip"><dt><a href="/pkg/archive/zip/">archive/zip</a></dt>
<dd>
+ <p><!-- https://go.dev/issue/55356 -->
+ <code>NewReader</code> will now return the error <code>ErrInsecurePath</code>
+ when opening an archive which contains file names that are absolute,
+ refer to a location outside the current directory, contain invalid
+ characters, or (on Windows) are reserved names such as <code>NUL</code>.
+ </p>
+ <p>
+ Programs that want to operate on archives containing insecure file names may
+ ignore this error.
+ </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,
ErrWriteTooLong = errors.New("archive/tar: write too long")
ErrFieldTooLong = errors.New("archive/tar: header field too long")
ErrWriteAfterClose = errors.New("archive/tar: write after close")
+ ErrInsecurePath = errors.New("archive/tar: insecure file path")
errMissData = errors.New("archive/tar: sparse file references non-existent data")
errUnrefData = errors.New("archive/tar: sparse file contains unreferenced data")
errWriteHole = errors.New("archive/tar: write non-NUL byte in sparse hole")
import (
"bytes"
"io"
+ "path/filepath"
"strconv"
"strings"
"time"
// Any remaining data in the current file is automatically discarded.
//
// io.EOF is returned at the end of the input.
+//
+// ErrInsecurePath and a valid *Header are returned if the next file's name is:
+//
+// - absolute;
+// - a relative path escaping the current directory, such as "../a"; or
+// - on Windows, a reserved file name such as "NUL".
+//
+// The caller may ignore the ErrInsecurePath error,
+// but is then responsible for sanitizing paths as appropriate.
func (tr *Reader) Next() (*Header, error) {
if tr.err != nil {
return nil, tr.err
}
hdr, err := tr.next()
tr.err = err
+ if err == nil && !filepath.IsLocal(hdr.Name) {
+ err = ErrInsecurePath
+ }
return hdr, err
}
}
}
}
+
+func TestInsecurePaths(t *testing.T) {
+ for _, path := range []string{
+ "../foo",
+ "/foo",
+ "a/b/../../../c",
+ } {
+ var buf bytes.Buffer
+ tw := NewWriter(&buf)
+ tw.WriteHeader(&Header{
+ Name: path,
+ })
+ const securePath = "secure"
+ tw.WriteHeader(&Header{
+ Name: securePath,
+ })
+ tw.Close()
+
+ tr := NewReader(&buf)
+ h, err := tr.Next()
+ if err != ErrInsecurePath {
+ t.Errorf("tr.Next for file %q: got err %v, want ErrInsecurePath", path, err)
+ continue
+ }
+ if h.Name != path {
+ t.Errorf("tr.Next for file %q: got name %q, want %q", path, h.Name, path)
+ }
+ // Error should not be sticky.
+ h, err = tr.Next()
+ if err != nil {
+ t.Errorf("tr.Next for file %q: got err %v, want nil", securePath, err)
+ }
+ if h.Name != securePath {
+ t.Errorf("tr.Next for file %q: got name %q, want %q", securePath, h.Name, securePath)
+ }
+ }
+}
// Test that we can get a long name back out of the archive.
reader := NewReader(&buf)
hdr, err = reader.Next()
- if err != nil {
+ if err != nil && err != ErrInsecurePath {
t.Fatal(err)
}
if hdr.Name != longName {
tr := NewReader(&b)
hdr, err := tr.Next()
- if err != nil {
+ if err != nil && err != ErrInsecurePath {
t.Errorf("test %d, unexpected Next error: %v", i, err)
}
if hdr.Name != name {
"io/fs"
"os"
"path"
+ "path/filepath"
"sort"
"strings"
"sync"
)
var (
- ErrFormat = errors.New("zip: not a valid zip file")
- ErrAlgorithm = errors.New("zip: unsupported compression algorithm")
- ErrChecksum = errors.New("zip: checksum error")
+ ErrFormat = errors.New("zip: not a valid zip file")
+ ErrAlgorithm = errors.New("zip: unsupported compression algorithm")
+ ErrChecksum = errors.New("zip: checksum error")
+ ErrInsecurePath = errors.New("zip: insecure file path")
)
// A Reader serves content from a ZIP archive.
// NewReader returns a new Reader reading from r, which is assumed to
// have the given size in bytes.
+//
+// ErrInsecurePath and a valid *Reader are returned if the names of any
+// files in the archive:
+//
+// - are absolute;
+// - are a relative path escaping the current directory, such as "../a";
+// - contain a backslash (\) character; or
+// - on Windows, are a reserved file name such as "NUL".
+//
+// The caller may ignore the ErrInsecurePath error,
+// but is then responsible for sanitizing paths as appropriate.
func NewReader(r io.ReaderAt, size int64) (*Reader, error) {
if size < 0 {
return nil, errors.New("zip: size cannot be negative")
if err := zr.init(r, size); err != nil {
return nil, err
}
+ for _, f := range zr.File {
+ if f.Name == "" {
+ // Zip permits an empty file name field.
+ 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, `\`) {
+ return zr, ErrInsecurePath
+ }
+ }
return zr, nil
}
0x00, 0x00, 0x59, 0x00, 0x00, 0x00, 0x00, 0x00,
}
r, err := NewReader(bytes.NewReader([]byte(data)), int64(len(data)))
- if err != nil {
+ if err != ErrInsecurePath {
t.Fatalf("Error reading the archive: %v", err)
}
_, err = r.Open("test.txt")
0x00, 0x90, 0x00, 0x00, 0x00, 0x00, 0x00,
}
r, err := NewReader(bytes.NewReader([]byte(data)), int64(len(data)))
- if err != nil {
+ if err != ErrInsecurePath {
t.Fatalf("Error reading the archive: %v", err)
}
entryNames := []string{`/`, `//`, `\`, `/test.txt`}
}
}
}
+
+func TestInsecurePaths(t *testing.T) {
+ for _, path := range []string{
+ "../foo",
+ "/foo",
+ "a/b/../../../c",
+ `a\b`,
+ } {
+ var buf bytes.Buffer
+ zw := NewWriter(&buf)
+ _, err := zw.Create(path)
+ if err != nil {
+ t.Errorf("zw.Create(%q) = %v", path, err)
+ continue
+ }
+ zw.Close()
+
+ zr, err := NewReader(bytes.NewReader(buf.Bytes()), int64(buf.Len()))
+ if err != ErrInsecurePath {
+ t.Errorf("NewReader for archive with file %q: got err %v, want ErrInsecurePath", path, err)
+ continue
+ }
+ var gotPaths []string
+ for _, f := range zr.File {
+ gotPaths = append(gotPaths, f.Name)
+ }
+ if !reflect.DeepEqual(gotPaths, []string{path}) {
+ t.Errorf("NewReader for archive with file %q: got files %q", path, gotPaths)
+ continue
+ }
+ }
+}
// It must be a relative path, not start with a drive letter (such as "C:"),
// and must use forward slashes instead of back slashes. A trailing slash
// indicates that this file is a directory and should have no data.
- //
- // When reading zip files, the Name field is populated from
- // the zip file directly and is not validated for correctness.
- // It is the caller's responsibility to sanitize it as
- // appropriate, including canonicalizing slash directions,
- // validating that paths are relative, and preventing path
- // traversal through filenames ("../../../").
Name string
// Comment is any arbitrary user-defined string shorter than 64KiB.