From 0f7b8698ec70ea88dcea98468669de18f64a2687 Mon Sep 17 00:00:00 2001
From: Deniz Onur Duzgun <59659739+dduzgun-security@users.noreply.github.com>
Date: Mon, 5 Aug 2024 16:23:27 -0400
Subject: [PATCH] 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
---
 .changelog/23738.txt                          |  3 +
 client/allocwatcher/alloc_watcher.go          |  8 ++-
 .../allocwatcher/alloc_watcher_unix_test.go   | 56 +++++++++++++++++++
 3 files changed, 66 insertions(+), 1 deletion(-)
 create mode 100644 .changelog/23738.txt

diff --git a/.changelog/23738.txt b/.changelog/23738.txt
new file mode 100644
index 000000000..0b04dc6fe
--- /dev/null
+++ b/.changelog/23738.txt
@@ -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.
+```
diff --git a/client/allocwatcher/alloc_watcher.go b/client/allocwatcher/alloc_watcher.go
index 5f5ac776a..b8fe48d04 100644
--- a/client/allocwatcher/alloc_watcher.go
+++ b/client/allocwatcher/alloc_watcher.go
@@ -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)
 			}
diff --git a/client/allocwatcher/alloc_watcher_unix_test.go b/client/allocwatcher/alloc_watcher_unix_test.go
index 78f93e708..70f4b536b 100644
--- a/client/allocwatcher/alloc_watcher_unix_test.go
+++ b/client/allocwatcher/alloc_watcher_unix_test.go
@@ -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)