security: fix write symlink escape on the same allocdir path (#23738)

Resolves symlink escape when unarchiving by removing existing paths within the same allocation directory which can occur by writing a header that points to a symlink that lives outside of the sandbox environment. This exploit requires first compromising the Nomad client agent at the source allocation.

Ref: https://hashicorp.atlassian.net/browse/NET-10607
Ref: https://github.com/hashicorp/nomad-enterprise/pull/1725
This commit is contained in:
Deniz Onur Duzgun
2024-08-05 16:23:27 -04:00
committed by GitHub
parent b25f1b66ce
commit 0f7b8698ec
3 changed files with 66 additions and 1 deletions

3
.changelog/23738.txt Normal file
View File

@@ -0,0 +1,3 @@
```release-note:security
security: Fix symlink escape during unarchiving by removing existing paths within the same allocdir. Compromising the Nomad client agent at the source allocation first is a prerequisite for leveraging this issue.
```

View File

@@ -636,7 +636,13 @@ func (p *remotePrevAlloc) streamAllocDir(ctx context.Context, resp io.ReadCloser
}
// If the header is a file, we write to a file
if hdr.Typeflag == tar.TypeReg {
f, err := os.Create(filepath.Join(dest, hdr.Name))
fPath := filepath.Join(dest, hdr.Name)
if _, err := os.Lstat(fPath); err == nil {
if err := os.Remove(fPath); err != nil {
return fmt.Errorf("error removing existing file: %w", err)
}
}
f, err := os.Create(fPath)
if err != nil {
return fmt.Errorf("error creating file: %w", err)
}

View File

@@ -136,6 +136,62 @@ func TestPrevAlloc_StreamAllocDir_BadSymlink_Linkname(t *testing.T) {
must.EqError(t, err, "archive contains symlink that escapes alloc dir")
}
func TestPrevAlloc_StreamAllocDir_SyminkWriteAttack(t *testing.T) {
ci.Parallel(t)
tmpDir := t.TempDir()
outsidePath := filepath.Join(tmpDir, "outside")
insidePath := "malformed_link"
content := "HelloWorld from outside"
// Create a tar archive with a symlink that attempts to escape the allocation directory
// by including a header that writes to the same path and follows the symlink target
// outside of the sandboxed environment.
var buf bytes.Buffer
tw := tar.NewWriter(&buf)
t.Cleanup(func() { tw.Close() })
must.NoError(t, tw.WriteHeader(&tar.Header{
Typeflag: tar.TypeSymlink,
Name: insidePath,
Linkname: outsidePath,
Mode: 0600,
}))
must.NoError(t, tw.WriteHeader(&tar.Header{
Typeflag: tar.TypeReg,
Name: insidePath,
Size: int64(len(content)),
Mode: 0600,
}))
_, err := tw.Write([]byte(content))
must.NoError(t, err)
newDir := t.TempDir()
prevAlloc := &remotePrevAlloc{logger: testlog.HCLogger(t)}
err = prevAlloc.streamAllocDir(context.Background(), io.NopCloser(&buf), newDir)
// No error expected
must.NoError(t, err)
// Check if the symlink target outside the alloc dir has not been written
_, err = os.Stat(outsidePath)
must.EqError(t, err, "stat "+outsidePath+": no such file or directory")
// Check if the symlink inside the alloc dir has been written
_, err = os.Stat(filepath.Join(newDir, insidePath))
must.NoError(t, err)
// Check if the content of the file inside the alloc dir is correct
contentBytes := make([]byte, len(content))
f, err := os.Open(filepath.Join(newDir, insidePath))
defer func() {
must.NoError(t, f.Close())
}()
must.NoError(t, err)
n, err := f.Read(contentBytes)
must.NoError(t, err)
must.Eq(t, content, string(contentBytes[:n]))
}
func testTar(dir string) (*bytes.Buffer, error) {
buf := new(bytes.Buffer)