security: add escape to arbitrary file access (#23319)

This commit is contained in:
Deniz Onur Duzgun
2024-07-08 14:00:09 -04:00
committed by GitHub
parent 21818843f0
commit ef6cdec884
4 changed files with 72 additions and 6 deletions

3
.changelog/23319.txt Normal file
View File

@@ -0,0 +1,3 @@
```release-note:security
migration: Added a check for relative paths escaping the allocation directory when unpacking archive during migration, to harden clients against compromised peer clients sending malicious archives
```

View File

@@ -587,6 +587,12 @@ func (p *remotePrevAlloc) streamAllocDir(ctx context.Context, resp io.ReadCloser
p.prevAllocID, p.allocID, err)
}
if escapes, err := escapingfs.PathEscapesAllocDir(dest, "", hdr.Name); err != nil {
return fmt.Errorf("error evaluating object: %w", err)
} else if escapes {
return fmt.Errorf("archive contains object that escapes alloc dir")
}
if hdr.Name == errorFilename {
// Error snapshotting on the remote side, try to read
// the message out of the file and return it.
@@ -618,12 +624,12 @@ func (p *remotePrevAlloc) streamAllocDir(ctx context.Context, resp io.ReadCloser
return fmt.Errorf("error creating symlink: %w", err)
}
escapes, err := escapingfs.PathEscapesAllocDir(dest, "", hdr.Name)
if err != nil {
return fmt.Errorf("error evaluating symlink: %w", err)
}
if escapes {
return fmt.Errorf("archive contains symlink that escapes alloc dir")
for _, path := range []string{hdr.Name, hdr.Linkname} {
if escapes, err := escapingfs.PathEscapesAllocDir(dest, "", path); err != nil {
return fmt.Errorf("error evaluating symlink: %w", err)
} else if escapes {
return fmt.Errorf("archive contains symlink that escapes alloc dir")
}
}
continue

View File

@@ -23,6 +23,7 @@ import (
"github.com/hashicorp/nomad/nomad/mock"
"github.com/hashicorp/nomad/nomad/structs"
"github.com/hashicorp/nomad/testutil"
"github.com/shoenig/test/must"
"github.com/stretchr/testify/require"
)
@@ -276,3 +277,38 @@ func TestPrevAlloc_StreamAllocDir_Error(t *testing.T) {
t.Fatalf("expected foo.txt to be size 1 but found %d", fi.Size())
}
}
// TestPrevAlloc_StreamAllocDir_FileEscape asserts that an error is returned
// when the tar archive contains a file that escapes the allocation directory.
func TestPrevAlloc_StreamAllocDir_FileEscape(t *testing.T) {
ci.Parallel(t)
// This test only unit tests streamAllocDir so we only need a partially
// complete remotePrevAlloc
prevAlloc := &remotePrevAlloc{
logger: testlog.HCLogger(t),
allocID: "123",
prevAllocID: "abc",
migrate: true,
}
// Create a tar archive with a file that escapes the allocation directory
tarBuf := bytes.NewBuffer(nil)
tw := tar.NewWriter(tarBuf)
err := tw.WriteHeader(&tar.Header{
Name: "../escape.txt",
Mode: 0666,
Size: 1,
ModTime: time.Now(),
Typeflag: tar.TypeReg,
})
must.NoError(t, err)
_, err = tw.Write([]byte{'a'})
t.Cleanup(func() { tw.Close() })
must.NoError(t, err)
// Attempt to stream the allocation directory
dest := t.TempDir()
err = prevAlloc.streamAllocDir(context.Background(), io.NopCloser(tarBuf), dest)
must.EqError(t, err, "archive contains object that escapes alloc dir")
}

View File

@@ -115,6 +115,27 @@ func TestPrevAlloc_StreamAllocDir_BadSymlink(t *testing.T) {
must.EqError(t, err, "archive contains symlink that escapes alloc dir")
}
func TestPrevAlloc_StreamAllocDir_BadSymlink_Linkname(t *testing.T) {
ci.Parallel(t)
// Create a tar archive with a symlink that attempts to escape the allocation directory
var buf bytes.Buffer
tw := tar.NewWriter(&buf)
t.Cleanup(func() { tw.Close() })
must.NoError(t, tw.WriteHeader(&tar.Header{
Typeflag: tar.TypeSymlink,
Name: "symlink",
Linkname: "../escape_attempt",
Mode: 0600,
}))
newDir := t.TempDir()
prevAlloc := &remotePrevAlloc{logger: testlog.HCLogger(t)}
err := prevAlloc.streamAllocDir(context.Background(), io.NopCloser(&buf), newDir)
must.EqError(t, err, "archive contains symlink that escapes alloc dir")
}
func testTar(dir string) (*bytes.Buffer, error) {
buf := new(bytes.Buffer)