mirrors/velero
1
0
Fork 0
mirror of https://github.com/vmware-tanzu/velero.git synced 2026-08-15 11:46:06 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Verify extracted item paths stay inside the backup directory

Browse Source 
archive.GetItemFilePath/GetVersionedItemFilePath joined the group resource,
namespace and name into a path without checking the result against rootDir.
Those components can come from backup contents - the additional items a
RestoreItemAction returns are built from annotations on a backed up object -
so a component containing ".." resolved to an arbitrary file on the Velero
pod, which was then Stat'd, unmarshalled and restored as a Kubernetes object.

Both helpers now return an error when the joined path escapes rootDir, and all
callers handle it. rootDir is empty when building an entry path inside the
backup tarball, so "." is used as the containment base for that relative form.

Signed-off-by: chlins <chlins.zhang@gmail.com>
This commit is contained in:
chlins
2026-07-31 06:02:43 +00:00
committed by github-actions[bot]
parent 0c2b588f5a
commit 0f86521735
6 changed files with 168 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files
changelogs/unreleased/10102-chlins
+1
View File
@@ -0,0 +1 @@
Verify extracted item paths stay inside the backup directory
internal/delete/delete_item_action_handler.go
+4 -1
View File
@@ -114,7 +114,10 @@ func InvokeDeleteActions(ctx *Context) error {
// Process individual items from the backup
for _, item := range items {
itemPath := archive.GetItemFilePath(dir, resource, namespace, item)
itemPath, err := archive.GetItemFilePath(dir, resource, namespace, item)
if err != nil {
return errors.Wrapf(err, "could not build item path: %v", item)
}
// obj is the Unstructured item from the backup
obj, err := archive.Unmarshal(ctx.Filesystem, itemPath)
pkg/archive/filesystem.go
+29 -3
View File
@@ -19,7 +19,9 @@ package archive
import (
"encoding/json"
"path/filepath"
"strings"
"github.com/cockroachdb/errors"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
@@ -27,13 +29,37 @@ import (
)
// GetItemFilePath returns an item's file path once extracted from a Velero backup archive.
func GetItemFilePath(rootDir, groupResource, namespace, name string) string {
func GetItemFilePath(rootDir, groupResource, namespace, name string) (string, error) {
return GetVersionedItemFilePath(rootDir, groupResource, namespace, name, "")
}
// GetVersionedItemFilePath returns an item's file path once extracted from a Velero backup archive, with version included.
func GetVersionedItemFilePath(rootDir, groupResource, namespace, name, versionPath string) string {
return filepath.Join(rootDir, velerov1api.ResourcesDir, groupResource, versionPath, GetScopeDir(namespace), namespace, name+".json")
//
// The namespace and name components can originate from backup contents - for example the
// additional items a RestoreItemAction returns are built from annotations on a backed up
// object - so the joined path is verified to stay within rootDir. Without that check a
// component containing ".." escapes the extracted backup directory and addresses an
// arbitrary file on the Velero pod's filesystem.
func GetVersionedItemFilePath(rootDir, groupResource, namespace, name, versionPath string) (string, error) {
path := filepath.Join(rootDir, velerov1api.ResourcesDir, groupResource, versionPath, GetScopeDir(namespace), namespace, name+".json")
// rootDir is empty when building the path of an entry inside the backup tarball rather
// than of an extracted file on disk; "." is the containment base for that relative form.
base := rootDir
if base == "" {
base = "."
}
rel, err := filepath.Rel(base, path)
if err != nil {
return "", errors.Wrapf(err, "error resolving item path for %q/%q", namespace, name)
}
if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) {
return "", errors.Errorf("invalid item path for %q/%q: escapes the backup directory", namespace, name)
}
return path, nil
}
// GetScopeDir returns NamespaceScopedDir if namespace is present, or ClusterScopedDir if empty
pkg/archive/filesystem_test.go
+81 -8
View File
@@ -27,31 +27,104 @@ import (
)
func TestGetItemFilePath(t *testing.T) {
res := GetItemFilePath("root", "resource", "", "item")
res, err := GetItemFilePath("root", "resource", "", "item")
require.NoError(t, err)
assert.Equal(t, "root/resources/resource/cluster/item.json", res)
res = GetItemFilePath("root", "resource", "namespace", "item")
res, err = GetItemFilePath("root", "resource", "namespace", "item")
require.NoError(t, err)
assert.Equal(t, "root/resources/resource/namespaces/namespace/item.json", res)
res = GetItemFilePath("", "resource", "", "item")
res, err = GetItemFilePath("", "resource", "", "item")
require.NoError(t, err)
assert.Equal(t, "resources/resource/cluster/item.json", res)
res = GetVersionedItemFilePath("root", "resource", "", "item", "")
res, err = GetVersionedItemFilePath("root", "resource", "", "item", "")
require.NoError(t, err)
assert.Equal(t, "root/resources/resource/cluster/item.json", res)
res = GetVersionedItemFilePath("root", "resource", "namespace", "item", "")
res, err = GetVersionedItemFilePath("root", "resource", "namespace", "item", "")
require.NoError(t, err)
assert.Equal(t, "root/resources/resource/namespaces/namespace/item.json", res)
res = GetVersionedItemFilePath("root", "resource", "namespace", "item", "v1")
res, err = GetVersionedItemFilePath("root", "resource", "namespace", "item", "v1")
require.NoError(t, err)
assert.Equal(t, "root/resources/resource/v1/namespaces/namespace/item.json", res)
res = GetVersionedItemFilePath("root", "resource", "", "item", "v1")
res, err = GetVersionedItemFilePath("root", "resource", "", "item", "v1")
require.NoError(t, err)
assert.Equal(t, "root/resources/resource/v1/cluster/item.json", res)
res = GetVersionedItemFilePath("", "resource", "", "item", "")
res, err = GetVersionedItemFilePath("", "resource", "", "item", "")
require.NoError(t, err)
assert.Equal(t, "resources/resource/cluster/item.json", res)
}
// TestGetItemFilePathRejectsPathTraversal verifies that a name or namespace containing
// ".." cannot address a file outside the extracted backup directory. These components can
// come from backup contents, for example the additional items a RestoreItemAction builds
// from annotations on a backed up object.
func TestGetItemFilePathRejectsPathTraversal(t *testing.T) {
tests := []struct {
name string
rootDir string
groupResource string
namespace string
itemName string
}{
{
name: "traversal in name escapes root",
rootDir: "/tmp/restore-dir",
groupResource: "secrets",
namespace: "x",
itemName: "../../../../../../root/.docker/config",
},
{
name: "traversal in namespace escapes root",
rootDir: "/tmp/restore-dir",
groupResource: "secrets",
namespace: "../../../../../../etc",
itemName: "passwd",
},
{
name: "traversal in group resource escapes root",
rootDir: "/tmp/restore-dir",
groupResource: "../../../../../../etc",
namespace: "",
itemName: "passwd",
},
{
name: "traversal escapes archive-relative root",
rootDir: "",
groupResource: "secrets",
namespace: "x",
itemName: "../../../../../../escape",
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
res, err := GetItemFilePath(tc.rootDir, tc.groupResource, tc.namespace, tc.itemName)
require.Error(t, err)
assert.Contains(t, err.Error(), "escapes the backup directory")
assert.Empty(t, res)
res, err = GetVersionedItemFilePath(tc.rootDir, tc.groupResource, tc.namespace, tc.itemName, "v1")
require.Error(t, err)
assert.Contains(t, err.Error(), "escapes the backup directory")
assert.Empty(t, res)
})
}
}
// TestGetItemFilePathAllowsInnerDotDot verifies the containment check does not reject a
// path whose ".." segments resolve back inside the root directory.
func TestGetItemFilePathAllowsInnerDotDot(t *testing.T) {
res, err := GetItemFilePath("root", "resource", "namespaces/..", "item")
require.NoError(t, err)
assert.Equal(t, "root/resources/resource/namespaces/item.json", res)
}
func TestGetScopeDir(t *testing.T) {
res := GetScopeDir("")
assert.Equal(t, velerov1api.ClusterScopedDir, res)
pkg/backup/item_backupper.go
+17 -5
View File
@@ -351,16 +351,28 @@ func (ib *itemBackupper) backupItemInternal(logger logrus.FieldLogger, obj runti
if versionPath == preferredGVR.Version {
// backing up preferred version backup without API Group version - for backward compatibility
log.Debugf("Resource %s/%s, version= %s, preferredVersion=%s", groupResource.String(), name, versionPath, preferredGVR.Version)
itemFiles = append(itemFiles, getFileForArchive(namespace, name, groupResource.String(), "", itemBytes))
fileForArchive, err := getFileForArchive(namespace, name, groupResource.String(), "", itemBytes)
if err != nil {
return false, itemFiles, err
}
itemFiles = append(itemFiles, fileForArchive)
versionPath = versionPath + velerov1api.PreferredVersionDir
}
itemFiles = append(itemFiles, getFileForArchive(namespace, name, groupResource.String(), versionPath, itemBytes))
fileForArchive, err := getFileForArchive(namespace, name, groupResource.String(), versionPath, itemBytes)
if err != nil {
return false, itemFiles, err
}
itemFiles = append(itemFiles, fileForArchive)
return true, itemFiles, nil
}
func getFileForArchive(namespace, name, groupResource, versionPath string, itemBytes []byte) FileForArchive {
filePath := archive.GetVersionedItemFilePath("", groupResource, namespace, name, versionPath)
func getFileForArchive(namespace, name, groupResource, versionPath string, itemBytes []byte) (FileForArchive, error) {
filePath, err := archive.GetVersionedItemFilePath("", groupResource, namespace, name, versionPath)
if err != nil {
return FileForArchive{}, err
}
hdr := &tar.Header{
Name: filePath,
Size: int64(len(itemBytes)),
@@ -368,7 +380,7 @@ func getFileForArchive(namespace, name, groupResource, versionPath string, itemB
Mode: 0755,
ModTime: time.Now(),
}
return FileForArchive{FilePath: filePath, Header: hdr, FileBytes: itemBytes}
return FileForArchive{FilePath: filePath, Header: hdr, FileBytes: itemBytes}, nil
}
// backupPodVolumes triggers pod volume backups of the specified pod volumes, and returns a list of PodVolumeBackups
pkg/restore/restore.go
+36 -14
View File
@@ -1011,11 +1011,13 @@ func (ctx *restoreContext) processSelectedResource(
if namespace != "" && !existingNamespaces.Has(targetNS) {
logger := ctx.log.WithField("namespace", namespace)
ns := getNamespace(
logger,
archive.GetItemFilePath(ctx.restoreDir, "namespaces", "", namespace),
targetNS,
)
nsPath, err := archive.GetItemFilePath(ctx.restoreDir, "namespaces", "", namespace)
if err != nil {
errs.AddVeleroError(err)
continue
}
ns := getNamespace(logger, nsPath, targetNS)
_, nsCreated, err := kube.EnsureNamespaceExistsAndIsReady(
ns,
ctx.namespaceClient,
@@ -1440,7 +1442,13 @@ func (ctx *restoreContext) restoreItem(obj *unstructured.Unstructured, groupReso
// If the namespace scoped resource should be restored, ensure that the
// namespace into which the resource is being restored into exists.
// This is the *remapped* namespace that we are ensuring exists.
nsToEnsure := getNamespace(restoreLogger, archive.GetItemFilePath(ctx.restoreDir, "namespaces", "", obj.GetNamespace()), namespace)
nsPath, err := archive.GetItemFilePath(ctx.restoreDir, "namespaces", "", obj.GetNamespace())
if err != nil {
errs.AddVeleroError(err)
return warnings, errs, itemExists
}
nsToEnsure := getNamespace(restoreLogger, nsPath, namespace)
_, nsCreated, err := kube.EnsureNamespaceExistsAndIsReady(nsToEnsure, ctx.namespaceClient, ctx.resourceTerminatingTimeout, ctx.resourceDeletionStatusTracker)
if err != nil {
errs.AddVeleroError(err)
@@ -1693,7 +1701,17 @@ func (ctx *restoreContext) restoreItem(obj *unstructured.Unstructured, groupReso
var filteredAdditionalItems []velero.ResourceIdentifier
for _, additionalItem := range executeOutput.AdditionalItems {
itemPath := archive.GetItemFilePath(ctx.restoreDir, additionalItem.GroupResource.String(), additionalItem.Namespace, additionalItem.Name)
itemPath, err := archive.GetItemFilePath(ctx.restoreDir, additionalItem.GroupResource.String(), additionalItem.Namespace, additionalItem.Name)
if err != nil {
restoreLogger.WithError(err).WithFields(logrus.Fields{
"additionalResource": additionalItem.GroupResource.String(),
"additionalResourceNamespace": additionalItem.Namespace,
"additionalResourceName": additionalItem.Name,
}).Warn("unable to restore additional item")
warnings.Add(additionalItem.Namespace, err)
continue
}
if _, err := ctx.fileSystem.Stat(itemPath); err != nil {
restoreLogger.WithError(err).WithFields(logrus.Fields{
@@ -2671,9 +2689,9 @@ func (ctx *restoreContext) getSelectedRestoreableItems(resource string, original
// Peek-and-map logic for unresolvable kinds
if rf == nil && len(items) > 0 {
peekPath := archive.GetItemFilePath(ctx.restoreDir, resourceForPath, originalNamespace, items[0])
// Ignore unmarshal errors during peek; the main restore loop will catch and report them
if obj, err := archive.Unmarshal(ctx.fileSystem, peekPath); err == nil {
peekPath, pathErr := archive.GetItemFilePath(ctx.restoreDir, resourceForPath, originalNamespace, items[0])
// Ignore path and unmarshal errors during peek; the main restore loop will catch and report them
if obj, err := archive.Unmarshal(ctx.fileSystem, peekPath); pathErr == nil && err == nil {
actualKind := obj.GroupVersionKind().Kind
for _, filter := range nsFilter.resourceFilterMap {
for _, k := range filter.originalKinds {
@@ -2714,9 +2732,9 @@ func (ctx *restoreContext) getSelectedRestoreableItems(resource string, original
// Note: Unlike the namespaced path, this fallback is always reachable
// because the main restore loop does not have a fast-path skip for
// unlisted cluster-scoped resources.
peekPath := archive.GetItemFilePath(ctx.restoreDir, resourceForPath, originalNamespace, items[0])
// Ignore unmarshal errors during peek; the main restore loop will catch and report them
if obj, err := archive.Unmarshal(ctx.fileSystem, peekPath); err == nil {
peekPath, pathErr := archive.GetItemFilePath(ctx.restoreDir, resourceForPath, originalNamespace, items[0])
// Ignore path and unmarshal errors during peek; the main restore loop will catch and report them
if obj, err := archive.Unmarshal(ctx.fileSystem, peekPath); pathErr == nil && err == nil {
actualKind := obj.GroupVersionKind().Kind
for _, filter := range ctx.clusterScopedFilterMap {
for _, k := range filter.originalKinds {
@@ -2742,7 +2760,11 @@ func (ctx *restoreContext) getSelectedRestoreableItems(resource string, original
}
for _, item := range items {
itemPath := archive.GetItemFilePath(ctx.restoreDir, resourceForPath, originalNamespace, item)
itemPath, err := archive.GetItemFilePath(ctx.restoreDir, resourceForPath, originalNamespace, item)
if err != nil {
errs.Add(targetNamespace, err)
continue
}
obj, err := archive.Unmarshal(ctx.fileSystem, itemPath)
if err != nil {