Nomad Vulnerable to Arbitrary Write Through Symlink Attack

@@ -0,0 +1,3 @@
+```release-note:security
+migration: Fixed a bug where archives used for migration were not checked for symlinks that escaped the allocation directory
+```

@@ -20,6 +20,7 @@ import (
 	"github.com/hashicorp/nomad/client/config"
 	cstructs "github.com/hashicorp/nomad/client/structs"
 	"github.com/hashicorp/nomad/helper"
+	"github.com/hashicorp/nomad/helper/escapingfs"
 	"github.com/hashicorp/nomad/nomad/structs"
 )
 
@@ -515,7 +516,7 @@ func (p *remotePrevAlloc) migrateAllocDir(ctx context.Context, nodeAddr string)
 	// Create the previous alloc dir
 	prevAllocDir := allocdir.NewAllocDir(p.logger, p.config.AllocDir, p.prevAllocID)
 	if err := prevAllocDir.Build(); err != nil {
-		return nil, fmt.Errorf("error building alloc dir for previous alloc %q: %v", p.prevAllocID, err)
+		return nil, fmt.Errorf("error building alloc dir for previous alloc %q: %w", p.prevAllocID, err)
 	}
 
 	// Create an API client
@@ -537,7 +538,7 @@ func (p *remotePrevAlloc) migrateAllocDir(ctx context.Context, nodeAddr string)
 	resp, err := apiClient.Raw().Response(url, qo)
 	if err != nil {
 		prevAllocDir.Destroy()
-		return nil, fmt.Errorf("error getting snapshot from previous alloc %q: %v", p.prevAllocID, err)
+		return nil, fmt.Errorf("error getting snapshot from previous alloc %q: %w", p.prevAllocID, err)
 	}
 
 	if err := p.streamAllocDir(ctx, resp, prevAllocDir.AllocDir); err != nil {
@@ -582,7 +583,7 @@ func (p *remotePrevAlloc) streamAllocDir(ctx context.Context, resp io.ReadCloser
 		}
 
 		if err != nil {
-			return fmt.Errorf("error streaming previous alloc %q for new alloc %q: %v",
+			return fmt.Errorf("error streaming previous alloc %q for new alloc %q: %w",
 				p.prevAllocID, p.allocID, err)
 		}
 
@@ -591,7 +592,7 @@ func (p *remotePrevAlloc) streamAllocDir(ctx context.Context, resp io.ReadCloser
 			// the message out of the file and return it.
 			errBuf := make([]byte, int(hdr.Size))
 			if _, err := tr.Read(errBuf); err != nil && err != io.EOF {
-				return fmt.Errorf("error streaming previous alloc %q for new alloc %q; failed reading error message: %v",
+				return fmt.Errorf("error streaming previous alloc %q for new alloc %q; failed reading error message: %w",
 					p.prevAllocID, p.allocID, err)
 			}
 			return fmt.Errorf("error streaming previous alloc %q for new alloc %q: %s",
@@ -606,36 +607,45 @@ func (p *remotePrevAlloc) streamAllocDir(ctx context.Context, resp io.ReadCloser
 			// Can't change owner if not root or on Windows.
 			if euid == 0 {
 				if err := os.Chown(name, hdr.Uid, hdr.Gid); err != nil {
-					return fmt.Errorf("error chowning directory %v", err)
+					return fmt.Errorf("error chowning directory %w", err)
 				}
 			}
 			continue
 		}
 		// If the header is for a symlink we create the symlink
 		if hdr.Typeflag == tar.TypeSymlink {
 			if err = os.Symlink(hdr.Linkname, filepath.Join(dest, hdr.Name)); err != nil {
-				return fmt.Errorf("error creating symlink: %v", err)
+				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")
+			}
+
 			continue
 		}
 		// 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))
 			if err != nil {
-				return fmt.Errorf("error creating file: %v", err)
+				return fmt.Errorf("error creating file: %w", err)
 			}
 
 			// Setting the permissions of the file as the origin.
 			if err := f.Chmod(os.FileMode(hdr.Mode)); err != nil {
 				f.Close()
-				return fmt.Errorf("error chmoding file %v", err)
+				return fmt.Errorf("error chmoding file %w", err)
 			}
 
 			// Can't change owner if not root or on Windows.
 			if euid == 0 {
 				if err := f.Chown(hdr.Uid, hdr.Gid); err != nil {
 					f.Close()
-					return fmt.Errorf("error chowning file %v", err)
+					return fmt.Errorf("error chowning file %w", err)
 				}
 			}
 
@@ -646,14 +656,14 @@ func (p *remotePrevAlloc) streamAllocDir(ctx context.Context, resp io.ReadCloser
 				if n > 0 && (err == nil || err == io.EOF) {
 					if _, err := f.Write(buf[:n]); err != nil {
 						f.Close()
-						return fmt.Errorf("error writing to file %q: %v", f.Name(), err)
+						return fmt.Errorf("error writing to file %q: %w", f.Name(), err)
 					}
 				}
 
 				if err != nil {
 					f.Close()
 					if err != io.EOF {
-						return fmt.Errorf("error reading snapshot: %v", err)
+						return fmt.Errorf("error reading snapshot: %w", err)
 					}
 					break
 				}

@@ -20,6 +20,7 @@ import (
 	"github.com/hashicorp/nomad/ci"
 	ctestutil "github.com/hashicorp/nomad/client/testutil"
 	"github.com/hashicorp/nomad/helper/testlog"
+	"github.com/shoenig/test/must"
 )
 
 // TestPrevAlloc_StreamAllocDir_Ok asserts that streaming a tar to an alloc dir
@@ -32,47 +33,90 @@ func TestPrevAlloc_StreamAllocDir_Ok(t *testing.T) {
 
 	// Create foo/
 	fooDir := filepath.Join(dir, "foo")
-	if err := os.Mkdir(fooDir, 0777); err != nil {
-		t.Fatalf("err: %v", err)
-	}
+	must.NoError(t, os.Mkdir(fooDir, 0777))
 
 	// Change ownership of foo/ to test #3702 (any non-root user is fine)
 	const uid, gid = 1, 1
-	if err := os.Chown(fooDir, uid, gid); err != nil {
-		t.Fatalf("err : %v", err)
-	}
+	must.NoError(t, os.Chown(fooDir, uid, gid))
 
 	dirInfo, err := os.Stat(fooDir)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
+	must.NoError(t, err)
 
 	// Create foo/bar
 	f, err := os.Create(filepath.Join(fooDir, "bar"))
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	if _, err := f.WriteString("123"); err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	if err := f.Chmod(0644); err != nil {
-		t.Fatalf("err: %v", err)
-	}
+	must.NoError(t, err)
+
+	_, err = f.WriteString("123")
+	must.NoError(t, err)
+
+	err = f.Chmod(0644)
+	must.NoError(t, err)
+
 	fInfo, err := f.Stat()
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
+	must.NoError(t, err)
 	f.Close()
 
 	// Create foo/baz -> bar symlink
-	if err := os.Symlink("bar", filepath.Join(dir, "foo", "baz")); err != nil {
-		t.Fatalf("err: %v", err)
-	}
+	err = os.Symlink("bar", filepath.Join(dir, "foo", "baz"))
+	must.NoError(t, err)
+
 	linkInfo, err := os.Lstat(filepath.Join(dir, "foo", "baz"))
-	if err != nil {
-		t.Fatalf("err: %v", err)
+	must.NoError(t, err)
+
+	buf, err := testTar(dir)
+
+	dir1 := t.TempDir()
+
+	rc := io.NopCloser(buf)
+	prevAlloc := &remotePrevAlloc{logger: testlog.HCLogger(t)}
+	err = prevAlloc.streamAllocDir(context.Background(), rc, dir1)
+	must.NoError(t, err)
+
+	// Ensure foo is present
+	fi, err := os.Stat(filepath.Join(dir1, "foo"))
+	must.NoError(t, err)
+	must.Eq(t, dirInfo.Mode(), fi.Mode(), must.Sprintf("unexpected file mode"))
+
+	stat := fi.Sys().(*syscall.Stat_t)
+	if stat.Uid != uid || stat.Gid != gid {
+		t.Fatalf("foo/ has incorrect ownership: expected %d:%d found %d:%d",
+			uid, gid, stat.Uid, stat.Gid)
 	}
 
+	fi1, err := os.Stat(filepath.Join(dir1, "bar"))
+	must.NoError(t, err)
+	must.Eq(t, fInfo.Mode(), fi1.Mode(), must.Sprintf("unexpected file mode"))
+
+	fi2, err := os.Lstat(filepath.Join(dir1, "baz"))
+	must.NoError(t, err)
+	must.Eq(t, linkInfo.Mode(), fi2.Mode(), must.Sprintf("unexpected file mode"))
+}
+
+func TestPrevAlloc_StreamAllocDir_BadSymlink(t *testing.T) {
+	ci.Parallel(t)
+
+	dir := t.TempDir()
+	sensitiveDir := t.TempDir()
+
+	fooDir := filepath.Join(dir, "foo")
+	err := os.Mkdir(fooDir, 0777)
+	must.NoError(t, err)
+
+	// Create sensitive -> foo/bar symlink
+	err = os.Symlink(sensitiveDir, filepath.Join(dir, "foo", "baz"))
+	must.NoError(t, err)
+
+	buf, err := testTar(dir)
+	rc := io.NopCloser(buf)
+
+	dir1 := t.TempDir()
+	prevAlloc := &remotePrevAlloc{logger: testlog.HCLogger(t)}
+	err = prevAlloc.streamAllocDir(context.Background(), rc, dir1)
+	must.EqError(t, err, "archive contains symlink that escapes alloc dir")
+}
+
+func testTar(dir string) (*bytes.Buffer, error) {
+
 	buf := new(bytes.Buffer)
 	tw := tar.NewWriter(buf)
 
@@ -118,45 +162,9 @@ func TestPrevAlloc_StreamAllocDir_Ok(t *testing.T) {
 	}
 
 	if err := filepath.Walk(dir, walkFn); err != nil {
-		t.Fatalf("err: %v", err)
+		return nil, err
 	}
 	tw.Close()
 
-	dir1 := t.TempDir()
-
-	rc := io.NopCloser(buf)
-	prevAlloc := &remotePrevAlloc{logger: testlog.HCLogger(t)}
-	if err := prevAlloc.streamAllocDir(context.Background(), rc, dir1); err != nil {
-		t.Fatalf("err: %v", err)
-	}
-
-	// Ensure foo is present
-	fi, err := os.Stat(filepath.Join(dir1, "foo"))
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	if fi.Mode() != dirInfo.Mode() {
-		t.Fatalf("mode: %v", fi.Mode())
-	}
-	stat := fi.Sys().(*syscall.Stat_t)
-	if stat.Uid != uid || stat.Gid != gid {
-		t.Fatalf("foo/ has incorrect ownership: expected %d:%d found %d:%d",
-			uid, gid, stat.Uid, stat.Gid)
-	}
-
-	fi1, err := os.Stat(filepath.Join(dir1, "bar"))
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	if fi1.Mode() != fInfo.Mode() {
-		t.Fatalf("mode: %v", fi1.Mode())
-	}
-
-	fi2, err := os.Lstat(filepath.Join(dir1, "baz"))
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	if fi2.Mode() != linkInfo.Mode() {
-		t.Fatalf("mode: %v", fi2.Mode())
-	}
+	return buf, nil
 }

@@ -52,16 +52,19 @@ func pathEscapesBaseViaSymlink(base, full string) (bool, error) {
 		return false, err
 	}
 
-	rel, err := filepath.Rel(resolveSym, base)
-	if err != nil {
-		return true, nil
-	}
+	// Nomad owns most of the prefix path, which includes the alloc UUID, so
+	// it's safe to assume that we can do a case insensitive check regardless of
+	// filesystem, as even if the cluster admin remounted the datadir with a
+	// slightly different capitalization, you'd only be able to escape into that
+	// same directory.
+	return !hasPrefixCaseInsensitive(resolveSym, base), nil
+}
 
-	// note: this is not the same as !filesystem.IsAbs; we are asking if the relative
-	// path is descendent of the base path, indicating it does not escape.
-	isRelative := strings.HasPrefix(rel, "..") || rel == "."
-	escapes := !isRelative
-	return escapes, nil
+func hasPrefixCaseInsensitive(path, prefix string) bool {
+	if len(prefix) > len(path) {
+		return false
+	}
+	return strings.EqualFold(path[:len(prefix)], prefix)
 }
 
 // PathEscapesAllocDir returns true if base/prefix/path escapes the given base directory.

@@ -9,6 +9,7 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/shoenig/test/must"
 	"github.com/stretchr/testify/require"
 )
 
@@ -229,3 +230,55 @@ func TestPathEscapesSandbox(t *testing.T) {
 		})
 	}
 }
+
+func TestHasPrefixCaseInsensitive(t *testing.T) {
+	cases := []struct {
+		name     string
+		path     string
+		prefix   string
+		expected bool
+	}{
+		{
+			name:     "has prefix",
+			path:     "/foo/bar",
+			prefix:   "/foo",
+			expected: true,
+		},
+		{
+			name:     "has prefix different case",
+			path:     "/FOO/bar",
+			prefix:   "/foo",
+			expected: true,
+		},
+		{
+			name:     "short path",
+			path:     "/foo",
+			prefix:   "/foo/bar",
+			expected: false,
+		},
+		{
+			name:     "exact match",
+			path:     "/foo",
+			prefix:   "/foo",
+			expected: true,
+		},
+		{
+			name:     "no prefix",
+			path:     "/baz/bar",
+			prefix:   "/foo",
+			expected: false,
+		},
+		{
+			name:     "no prefix different case",
+			path:     "/BAZ/bar",
+			prefix:   "/foo",
+			expected: false,
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := hasPrefixCaseInsensitive(tc.path, tc.prefix)
+			must.Eq(t, tc.expected, got)
+		})
+	}
+}

@@ -0,0 +1,3 @@
+```release-note:security
+migration: Fixed a bug where archives used for migration were not checked for symlinks that escaped the allocation directory
+```

@@ -20,6 +20,7 @@ import (
 	"github.com/hashicorp/nomad/client/config"
 	cstructs "github.com/hashicorp/nomad/client/structs"
 	"github.com/hashicorp/nomad/helper"
+	"github.com/hashicorp/nomad/helper/escapingfs"
 	"github.com/hashicorp/nomad/nomad/structs"
 )
 
@@ -515,7 +516,7 @@ func (p *remotePrevAlloc) migrateAllocDir(ctx context.Context, nodeAddr string)
 	// Create the previous alloc dir
 	prevAllocDir := allocdir.NewAllocDir(p.logger, p.config.AllocDir, p.prevAllocID)
 	if err := prevAllocDir.Build(); err != nil {
-		return nil, fmt.Errorf("error building alloc dir for previous alloc %q: %v", p.prevAllocID, err)
+		return nil, fmt.Errorf("error building alloc dir for previous alloc %q: %w", p.prevAllocID, err)
 	}
 
 	// Create an API client
@@ -537,7 +538,7 @@ func (p *remotePrevAlloc) migrateAllocDir(ctx context.Context, nodeAddr string)
 	resp, err := apiClient.Raw().Response(url, qo)
 	if err != nil {
 		prevAllocDir.Destroy()
-		return nil, fmt.Errorf("error getting snapshot from previous alloc %q: %v", p.prevAllocID, err)
+		return nil, fmt.Errorf("error getting snapshot from previous alloc %q: %w", p.prevAllocID, err)
 	}
 
 	if err := p.streamAllocDir(ctx, resp, prevAllocDir.AllocDir); err != nil {
@@ -582,7 +583,7 @@ func (p *remotePrevAlloc) streamAllocDir(ctx context.Context, resp io.ReadCloser
 		}
 
 		if err != nil {
-			return fmt.Errorf("error streaming previous alloc %q for new alloc %q: %v",
+			return fmt.Errorf("error streaming previous alloc %q for new alloc %q: %w",
 				p.prevAllocID, p.allocID, err)
 		}
 
@@ -591,7 +592,7 @@ func (p *remotePrevAlloc) streamAllocDir(ctx context.Context, resp io.ReadCloser
 			// the message out of the file and return it.
 			errBuf := make([]byte, int(hdr.Size))
 			if _, err := tr.Read(errBuf); err != nil && err != io.EOF {
-				return fmt.Errorf("error streaming previous alloc %q for new alloc %q; failed reading error message: %v",
+				return fmt.Errorf("error streaming previous alloc %q for new alloc %q; failed reading error message: %w",
 					p.prevAllocID, p.allocID, err)
 			}
 			return fmt.Errorf("error streaming previous alloc %q for new alloc %q: %s",
@@ -606,36 +607,45 @@ func (p *remotePrevAlloc) streamAllocDir(ctx context.Context, resp io.ReadCloser
 			// Can't change owner if not root or on Windows.
 			if euid == 0 {
 				if err := os.Chown(name, hdr.Uid, hdr.Gid); err != nil {
-					return fmt.Errorf("error chowning directory %v", err)
+					return fmt.Errorf("error chowning directory %w", err)
 				}
 			}
 			continue
 		}
 		// If the header is for a symlink we create the symlink
 		if hdr.Typeflag == tar.TypeSymlink {
 			if err = os.Symlink(hdr.Linkname, filepath.Join(dest, hdr.Name)); err != nil {
-				return fmt.Errorf("error creating symlink: %v", err)
+				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")
+			}
+
 			continue
 		}
 		// 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))
 			if err != nil {
-				return fmt.Errorf("error creating file: %v", err)
+				return fmt.Errorf("error creating file: %w", err)
 			}
 
 			// Setting the permissions of the file as the origin.
 			if err := f.Chmod(os.FileMode(hdr.Mode)); err != nil {
 				f.Close()
-				return fmt.Errorf("error chmoding file %v", err)
+				return fmt.Errorf("error chmoding file %w", err)
 			}
 
 			// Can't change owner if not root or on Windows.
 			if euid == 0 {
 				if err := f.Chown(hdr.Uid, hdr.Gid); err != nil {
 					f.Close()
-					return fmt.Errorf("error chowning file %v", err)
+					return fmt.Errorf("error chowning file %w", err)
 				}
 			}
 
@@ -646,14 +656,14 @@ func (p *remotePrevAlloc) streamAllocDir(ctx context.Context, resp io.ReadCloser
 				if n > 0 && (err == nil || err == io.EOF) {
 					if _, err := f.Write(buf[:n]); err != nil {
 						f.Close()
-						return fmt.Errorf("error writing to file %q: %v", f.Name(), err)
+						return fmt.Errorf("error writing to file %q: %w", f.Name(), err)
 					}
 				}
 
 				if err != nil {
 					f.Close()
 					if err != io.EOF {
-						return fmt.Errorf("error reading snapshot: %v", err)
+						return fmt.Errorf("error reading snapshot: %w", err)
 					}
 					break
 				}

@@ -20,6 +20,7 @@ import (
 	"github.com/hashicorp/nomad/ci"
 	ctestutil "github.com/hashicorp/nomad/client/testutil"
 	"github.com/hashicorp/nomad/helper/testlog"
+	"github.com/shoenig/test/must"
 )
 
 // TestPrevAlloc_StreamAllocDir_Ok asserts that streaming a tar to an alloc dir
@@ -32,47 +33,90 @@ func TestPrevAlloc_StreamAllocDir_Ok(t *testing.T) {
 
 	// Create foo/
 	fooDir := filepath.Join(dir, "foo")
-	if err := os.Mkdir(fooDir, 0777); err != nil {
-		t.Fatalf("err: %v", err)
-	}
+	must.NoError(t, os.Mkdir(fooDir, 0777))
 
 	// Change ownership of foo/ to test #3702 (any non-root user is fine)
 	const uid, gid = 1, 1
-	if err := os.Chown(fooDir, uid, gid); err != nil {
-		t.Fatalf("err : %v", err)
-	}
+	must.NoError(t, os.Chown(fooDir, uid, gid))
 
 	dirInfo, err := os.Stat(fooDir)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
+	must.NoError(t, err)
 
 	// Create foo/bar
 	f, err := os.Create(filepath.Join(fooDir, "bar"))
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	if _, err := f.WriteString("123"); err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	if err := f.Chmod(0644); err != nil {
-		t.Fatalf("err: %v", err)
-	}
+	must.NoError(t, err)
+
+	_, err = f.WriteString("123")
+	must.NoError(t, err)
+
+	err = f.Chmod(0644)
+	must.NoError(t, err)
+
 	fInfo, err := f.Stat()
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
+	must.NoError(t, err)
 	f.Close()
 
 	// Create foo/baz -> bar symlink
-	if err := os.Symlink("bar", filepath.Join(dir, "foo", "baz")); err != nil {
-		t.Fatalf("err: %v", err)
-	}
+	err = os.Symlink("bar", filepath.Join(dir, "foo", "baz"))
+	must.NoError(t, err)
+
 	linkInfo, err := os.Lstat(filepath.Join(dir, "foo", "baz"))
-	if err != nil {
-		t.Fatalf("err: %v", err)
+	must.NoError(t, err)
+
+	buf, err := testTar(dir)
+
+	dir1 := t.TempDir()
+
+	rc := io.NopCloser(buf)
+	prevAlloc := &remotePrevAlloc{logger: testlog.HCLogger(t)}
+	err = prevAlloc.streamAllocDir(context.Background(), rc, dir1)
+	must.NoError(t, err)
+
+	// Ensure foo is present
+	fi, err := os.Stat(filepath.Join(dir1, "foo"))
+	must.NoError(t, err)
+	must.Eq(t, dirInfo.Mode(), fi.Mode(), must.Sprintf("unexpected file mode"))
+
+	stat := fi.Sys().(*syscall.Stat_t)
+	if stat.Uid != uid || stat.Gid != gid {
+		t.Fatalf("foo/ has incorrect ownership: expected %d:%d found %d:%d",
+			uid, gid, stat.Uid, stat.Gid)
 	}
 
+	fi1, err := os.Stat(filepath.Join(dir1, "bar"))
+	must.NoError(t, err)
+	must.Eq(t, fInfo.Mode(), fi1.Mode(), must.Sprintf("unexpected file mode"))
+
+	fi2, err := os.Lstat(filepath.Join(dir1, "baz"))
+	must.NoError(t, err)
+	must.Eq(t, linkInfo.Mode(), fi2.Mode(), must.Sprintf("unexpected file mode"))
+}
+
+func TestPrevAlloc_StreamAllocDir_BadSymlink(t *testing.T) {
+	ci.Parallel(t)
+
+	dir := t.TempDir()
+	sensitiveDir := t.TempDir()
+
+	fooDir := filepath.Join(dir, "foo")
+	err := os.Mkdir(fooDir, 0777)
+	must.NoError(t, err)
+
+	// Create sensitive -> foo/bar symlink
+	err = os.Symlink(sensitiveDir, filepath.Join(dir, "foo", "baz"))
+	must.NoError(t, err)
+
+	buf, err := testTar(dir)
+	rc := io.NopCloser(buf)
+
+	dir1 := t.TempDir()
+	prevAlloc := &remotePrevAlloc{logger: testlog.HCLogger(t)}
+	err = prevAlloc.streamAllocDir(context.Background(), rc, dir1)
+	must.EqError(t, err, "archive contains symlink that escapes alloc dir")
+}
+
+func testTar(dir string) (*bytes.Buffer, error) {
+
 	buf := new(bytes.Buffer)
 	tw := tar.NewWriter(buf)
 
@@ -118,45 +162,9 @@ func TestPrevAlloc_StreamAllocDir_Ok(t *testing.T) {
 	}
 
 	if err := filepath.Walk(dir, walkFn); err != nil {
-		t.Fatalf("err: %v", err)
+		return nil, err
 	}
 	tw.Close()
 
-	dir1 := t.TempDir()
-
-	rc := io.NopCloser(buf)
-	prevAlloc := &remotePrevAlloc{logger: testlog.HCLogger(t)}
-	if err := prevAlloc.streamAllocDir(context.Background(), rc, dir1); err != nil {
-		t.Fatalf("err: %v", err)
-	}
-
-	// Ensure foo is present
-	fi, err := os.Stat(filepath.Join(dir1, "foo"))
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	if fi.Mode() != dirInfo.Mode() {
-		t.Fatalf("mode: %v", fi.Mode())
-	}
-	stat := fi.Sys().(*syscall.Stat_t)
-	if stat.Uid != uid || stat.Gid != gid {
-		t.Fatalf("foo/ has incorrect ownership: expected %d:%d found %d:%d",
-			uid, gid, stat.Uid, stat.Gid)
-	}
-
-	fi1, err := os.Stat(filepath.Join(dir1, "bar"))
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	if fi1.Mode() != fInfo.Mode() {
-		t.Fatalf("mode: %v", fi1.Mode())
-	}
-
-	fi2, err := os.Lstat(filepath.Join(dir1, "baz"))
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	if fi2.Mode() != linkInfo.Mode() {
-		t.Fatalf("mode: %v", fi2.Mode())
-	}
+	return buf, nil
 }

@@ -52,16 +52,19 @@ func pathEscapesBaseViaSymlink(base, full string) (bool, error) {
 		return false, err
 	}
 
-	rel, err := filepath.Rel(resolveSym, base)
-	if err != nil {
-		return true, nil
-	}
+	// Nomad owns most of the prefix path, which includes the alloc UUID, so
+	// it's safe to assume that we can do a case insensitive check regardless of
+	// filesystem, as even if the cluster admin remounted the datadir with a
+	// slightly different capitalization, you'd only be able to escape into that
+	// same directory.
+	return !hasPrefixCaseInsensitive(resolveSym, base), nil
+}
 
-	// note: this is not the same as !filesystem.IsAbs; we are asking if the relative
-	// path is descendent of the base path, indicating it does not escape.
-	isRelative := strings.HasPrefix(rel, "..") || rel == "."
-	escapes := !isRelative
-	return escapes, nil
+func hasPrefixCaseInsensitive(path, prefix string) bool {
+	if len(prefix) > len(path) {
+		return false
+	}
+	return strings.EqualFold(path[:len(prefix)], prefix)
 }
 
 // PathEscapesAllocDir returns true if base/prefix/path escapes the given base directory.

@@ -9,6 +9,7 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/shoenig/test/must"
 	"github.com/stretchr/testify/require"
 )
 
@@ -229,3 +230,55 @@ func TestPathEscapesSandbox(t *testing.T) {
 		})
 	}
 }
+
+func TestHasPrefixCaseInsensitive(t *testing.T) {
+	cases := []struct {
+		name     string
+		path     string
+		prefix   string
+		expected bool
+	}{
+		{
+			name:     "has prefix",
+			path:     "/foo/bar",
+			prefix:   "/foo",
+			expected: true,
+		},
+		{
+			name:     "has prefix different case",
+			path:     "/FOO/bar",
+			prefix:   "/foo",
+			expected: true,
+		},
+		{
+			name:     "short path",
+			path:     "/foo",
+			prefix:   "/foo/bar",
+			expected: false,
+		},
+		{
+			name:     "exact match",
+			path:     "/foo",
+			prefix:   "/foo",
+			expected: true,
+		},
+		{
+			name:     "no prefix",
+			path:     "/baz/bar",
+			prefix:   "/foo",
+			expected: false,
+		},
+		{
+			name:     "no prefix different case",
+			path:     "/BAZ/bar",
+			prefix:   "/foo",
+			expected: false,
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := hasPrefixCaseInsensitive(tc.path, tc.prefix)
+			must.Eq(t, tc.expected, got)
+		})
+	}
+}

@@ -0,0 +1,3 @@
+```release-note:security
+migration: Fixed a bug where archives used for migration were not checked for symlinks that escaped the allocation directory
+```

@@ -20,6 +20,7 @@ import (
 	"github.com/hashicorp/nomad/client/config"
 	cstructs "github.com/hashicorp/nomad/client/structs"
 	"github.com/hashicorp/nomad/helper"
+	"github.com/hashicorp/nomad/helper/escapingfs"
 	"github.com/hashicorp/nomad/nomad/structs"
 )
 
@@ -515,7 +516,7 @@ func (p *remotePrevAlloc) migrateAllocDir(ctx context.Context, nodeAddr string)
 	// Create the previous alloc dir
 	prevAllocDir := allocdir.NewAllocDir(p.logger, p.config.AllocDir, p.prevAllocID)
 	if err := prevAllocDir.Build(); err != nil {
-		return nil, fmt.Errorf("error building alloc dir for previous alloc %q: %v", p.prevAllocID, err)
+		return nil, fmt.Errorf("error building alloc dir for previous alloc %q: %w", p.prevAllocID, err)
 	}
 
 	// Create an API client
@@ -537,7 +538,7 @@ func (p *remotePrevAlloc) migrateAllocDir(ctx context.Context, nodeAddr string)
 	resp, err := apiClient.Raw().Response(url, qo)
 	if err != nil {
 		prevAllocDir.Destroy()
-		return nil, fmt.Errorf("error getting snapshot from previous alloc %q: %v", p.prevAllocID, err)
+		return nil, fmt.Errorf("error getting snapshot from previous alloc %q: %w", p.prevAllocID, err)
 	}
 
 	if err := p.streamAllocDir(ctx, resp, prevAllocDir.AllocDir); err != nil {
@@ -582,7 +583,7 @@ func (p *remotePrevAlloc) streamAllocDir(ctx context.Context, resp io.ReadCloser
 		}
 
 		if err != nil {
-			return fmt.Errorf("error streaming previous alloc %q for new alloc %q: %v",
+			return fmt.Errorf("error streaming previous alloc %q for new alloc %q: %w",
 				p.prevAllocID, p.allocID, err)
 		}
 
@@ -591,7 +592,7 @@ func (p *remotePrevAlloc) streamAllocDir(ctx context.Context, resp io.ReadCloser
 			// the message out of the file and return it.
 			errBuf := make([]byte, int(hdr.Size))
 			if _, err := tr.Read(errBuf); err != nil && err != io.EOF {
-				return fmt.Errorf("error streaming previous alloc %q for new alloc %q; failed reading error message: %v",
+				return fmt.Errorf("error streaming previous alloc %q for new alloc %q; failed reading error message: %w",
 					p.prevAllocID, p.allocID, err)
 			}
 			return fmt.Errorf("error streaming previous alloc %q for new alloc %q: %s",
@@ -606,36 +607,45 @@ func (p *remotePrevAlloc) streamAllocDir(ctx context.Context, resp io.ReadCloser
 			// Can't change owner if not root or on Windows.
 			if euid == 0 {
 				if err := os.Chown(name, hdr.Uid, hdr.Gid); err != nil {
-					return fmt.Errorf("error chowning directory %v", err)
+					return fmt.Errorf("error chowning directory %w", err)
 				}
 			}
 			continue
 		}
 		// If the header is for a symlink we create the symlink
 		if hdr.Typeflag == tar.TypeSymlink {
 			if err = os.Symlink(hdr.Linkname, filepath.Join(dest, hdr.Name)); err != nil {
-				return fmt.Errorf("error creating symlink: %v", err)
+				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")
+			}
+
 			continue
 		}
 		// 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))
 			if err != nil {
-				return fmt.Errorf("error creating file: %v", err)
+				return fmt.Errorf("error creating file: %w", err)
 			}
 
 			// Setting the permissions of the file as the origin.
 			if err := f.Chmod(os.FileMode(hdr.Mode)); err != nil {
 				f.Close()
-				return fmt.Errorf("error chmoding file %v", err)
+				return fmt.Errorf("error chmoding file %w", err)
 			}
 
 			// Can't change owner if not root or on Windows.
 			if euid == 0 {
 				if err := f.Chown(hdr.Uid, hdr.Gid); err != nil {
 					f.Close()
-					return fmt.Errorf("error chowning file %v", err)
+					return fmt.Errorf("error chowning file %w", err)
 				}
 			}
 
@@ -646,14 +656,14 @@ func (p *remotePrevAlloc) streamAllocDir(ctx context.Context, resp io.ReadCloser
 				if n > 0 && (err == nil || err == io.EOF) {
 					if _, err := f.Write(buf[:n]); err != nil {
 						f.Close()
-						return fmt.Errorf("error writing to file %q: %v", f.Name(), err)
+						return fmt.Errorf("error writing to file %q: %w", f.Name(), err)
 					}
 				}
 
 				if err != nil {
 					f.Close()
 					if err != io.EOF {
-						return fmt.Errorf("error reading snapshot: %v", err)
+						return fmt.Errorf("error reading snapshot: %w", err)
 					}
 					break
 				}

@@ -20,6 +20,7 @@ import (
 	"github.com/hashicorp/nomad/ci"
 	ctestutil "github.com/hashicorp/nomad/client/testutil"
 	"github.com/hashicorp/nomad/helper/testlog"
+	"github.com/shoenig/test/must"
 )
 
 // TestPrevAlloc_StreamAllocDir_Ok asserts that streaming a tar to an alloc dir
@@ -32,47 +33,90 @@ func TestPrevAlloc_StreamAllocDir_Ok(t *testing.T) {
 
 	// Create foo/
 	fooDir := filepath.Join(dir, "foo")
-	if err := os.Mkdir(fooDir, 0777); err != nil {
-		t.Fatalf("err: %v", err)
-	}
+	must.NoError(t, os.Mkdir(fooDir, 0777))
 
 	// Change ownership of foo/ to test #3702 (any non-root user is fine)
 	const uid, gid = 1, 1
-	if err := os.Chown(fooDir, uid, gid); err != nil {
-		t.Fatalf("err : %v", err)
-	}
+	must.NoError(t, os.Chown(fooDir, uid, gid))
 
 	dirInfo, err := os.Stat(fooDir)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
+	must.NoError(t, err)
 
 	// Create foo/bar
 	f, err := os.Create(filepath.Join(fooDir, "bar"))
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	if _, err := f.WriteString("123"); err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	if err := f.Chmod(0644); err != nil {
-		t.Fatalf("err: %v", err)
-	}
+	must.NoError(t, err)
+
+	_, err = f.WriteString("123")
+	must.NoError(t, err)
+
+	err = f.Chmod(0644)
+	must.NoError(t, err)
+
 	fInfo, err := f.Stat()
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
+	must.NoError(t, err)
 	f.Close()
 
 	// Create foo/baz -> bar symlink
-	if err := os.Symlink("bar", filepath.Join(dir, "foo", "baz")); err != nil {
-		t.Fatalf("err: %v", err)
-	}
+	err = os.Symlink("bar", filepath.Join(dir, "foo", "baz"))
+	must.NoError(t, err)
+
 	linkInfo, err := os.Lstat(filepath.Join(dir, "foo", "baz"))
-	if err != nil {
-		t.Fatalf("err: %v", err)
+	must.NoError(t, err)
+
+	buf, err := testTar(dir)
+
+	dir1 := t.TempDir()
+
+	rc := io.NopCloser(buf)
+	prevAlloc := &remotePrevAlloc{logger: testlog.HCLogger(t)}
+	err = prevAlloc.streamAllocDir(context.Background(), rc, dir1)
+	must.NoError(t, err)
+
+	// Ensure foo is present
+	fi, err := os.Stat(filepath.Join(dir1, "foo"))
+	must.NoError(t, err)
+	must.Eq(t, dirInfo.Mode(), fi.Mode(), must.Sprintf("unexpected file mode"))
+
+	stat := fi.Sys().(*syscall.Stat_t)
+	if stat.Uid != uid || stat.Gid != gid {
+		t.Fatalf("foo/ has incorrect ownership: expected %d:%d found %d:%d",
+			uid, gid, stat.Uid, stat.Gid)
 	}
 
+	fi1, err := os.Stat(filepath.Join(dir1, "bar"))
+	must.NoError(t, err)
+	must.Eq(t, fInfo.Mode(), fi1.Mode(), must.Sprintf("unexpected file mode"))
+
+	fi2, err := os.Lstat(filepath.Join(dir1, "baz"))
+	must.NoError(t, err)
+	must.Eq(t, linkInfo.Mode(), fi2.Mode(), must.Sprintf("unexpected file mode"))
+}
+
+func TestPrevAlloc_StreamAllocDir_BadSymlink(t *testing.T) {
+	ci.Parallel(t)
+
+	dir := t.TempDir()
+	sensitiveDir := t.TempDir()
+
+	fooDir := filepath.Join(dir, "foo")
+	err := os.Mkdir(fooDir, 0777)
+	must.NoError(t, err)
+
+	// Create sensitive -> foo/bar symlink
+	err = os.Symlink(sensitiveDir, filepath.Join(dir, "foo", "baz"))
+	must.NoError(t, err)
+
+	buf, err := testTar(dir)
+	rc := io.NopCloser(buf)
+
+	dir1 := t.TempDir()
+	prevAlloc := &remotePrevAlloc{logger: testlog.HCLogger(t)}
+	err = prevAlloc.streamAllocDir(context.Background(), rc, dir1)
+	must.EqError(t, err, "archive contains symlink that escapes alloc dir")
+}
+
+func testTar(dir string) (*bytes.Buffer, error) {
+
 	buf := new(bytes.Buffer)
 	tw := tar.NewWriter(buf)
 
@@ -118,45 +162,9 @@ func TestPrevAlloc_StreamAllocDir_Ok(t *testing.T) {
 	}
 
 	if err := filepath.Walk(dir, walkFn); err != nil {
-		t.Fatalf("err: %v", err)
+		return nil, err
 	}
 	tw.Close()
 
-	dir1 := t.TempDir()
-
-	rc := io.NopCloser(buf)
-	prevAlloc := &remotePrevAlloc{logger: testlog.HCLogger(t)}
-	if err := prevAlloc.streamAllocDir(context.Background(), rc, dir1); err != nil {
-		t.Fatalf("err: %v", err)
-	}
-
-	// Ensure foo is present
-	fi, err := os.Stat(filepath.Join(dir1, "foo"))
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	if fi.Mode() != dirInfo.Mode() {
-		t.Fatalf("mode: %v", fi.Mode())
-	}
-	stat := fi.Sys().(*syscall.Stat_t)
-	if stat.Uid != uid || stat.Gid != gid {
-		t.Fatalf("foo/ has incorrect ownership: expected %d:%d found %d:%d",
-			uid, gid, stat.Uid, stat.Gid)
-	}
-
-	fi1, err := os.Stat(filepath.Join(dir1, "bar"))
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	if fi1.Mode() != fInfo.Mode() {
-		t.Fatalf("mode: %v", fi1.Mode())
-	}
-
-	fi2, err := os.Lstat(filepath.Join(dir1, "baz"))
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	if fi2.Mode() != linkInfo.Mode() {
-		t.Fatalf("mode: %v", fi2.Mode())
-	}
+	return buf, nil
 }

@@ -52,16 +52,19 @@ func pathEscapesBaseViaSymlink(base, full string) (bool, error) {
 		return false, err
 	}
 
-	rel, err := filepath.Rel(resolveSym, base)
-	if err != nil {
-		return true, nil
-	}
+	// Nomad owns most of the prefix path, which includes the alloc UUID, so
+	// it's safe to assume that we can do a case insensitive check regardless of
+	// filesystem, as even if the cluster admin remounted the datadir with a
+	// slightly different capitalization, you'd only be able to escape into that
+	// same directory.
+	return !hasPrefixCaseInsensitive(resolveSym, base), nil
+}
 
-	// note: this is not the same as !filesystem.IsAbs; we are asking if the relative
-	// path is descendent of the base path, indicating it does not escape.
-	isRelative := strings.HasPrefix(rel, "..") || rel == "."
-	escapes := !isRelative
-	return escapes, nil
+func hasPrefixCaseInsensitive(path, prefix string) bool {
+	if len(prefix) > len(path) {
+		return false
+	}
+	return strings.EqualFold(path[:len(prefix)], prefix)
 }
 
 // PathEscapesAllocDir returns true if base/prefix/path escapes the given base directory.

@@ -9,6 +9,7 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/shoenig/test/must"
 	"github.com/stretchr/testify/require"
 )
 
@@ -229,3 +230,55 @@ func TestPathEscapesSandbox(t *testing.T) {
 		})
 	}
 }
+
+func TestHasPrefixCaseInsensitive(t *testing.T) {
+	cases := []struct {
+		name     string
+		path     string
+		prefix   string
+		expected bool
+	}{
+		{
+			name:     "has prefix",
+			path:     "/foo/bar",
+			prefix:   "/foo",
+			expected: true,
+		},
+		{
+			name:     "has prefix different case",
+			path:     "/FOO/bar",
+			prefix:   "/foo",
+			expected: true,
+		},
+		{
+			name:     "short path",
+			path:     "/foo",
+			prefix:   "/foo/bar",
+			expected: false,
+		},
+		{
+			name:     "exact match",
+			path:     "/foo",
+			prefix:   "/foo",
+			expected: true,
+		},
+		{
+			name:     "no prefix",
+			path:     "/baz/bar",
+			prefix:   "/foo",
+			expected: false,
+		},
+		{
+			name:     "no prefix different case",
+			path:     "/BAZ/bar",
+			prefix:   "/foo",
+			expected: false,
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := hasPrefixCaseInsensitive(tc.path, tc.prefix)
+			must.Eq(t, tc.expected, got)
+		})
+	}
+}