c20ff56e4a
manifest.AppName / HDDSubdirs / VolumeNames are attacker-controlled JSON inside an imported .fab and reach filepath.Join+MkdirAll/extractTar with a trusted base (restore.go:339/606/678). UnmarshalManifest did zero validation, so '../..' in any of them escaped the stacks / HDD destination dir. - New appexport.ValidateSegment + validateManifestPaths; UnmarshalManifest now fails the parse on a traversal segment (the chokepoint). - Defence-in-depth ValidateSegment guards at the HDD-subdir and volume-name join loops in restore.go. - ConfigFiles deliberately NOT validated (holds dotfiles like .felhom.yml; never used in a restore join). - Permanent regression test (was the deep-sweep failing audit test) now asserts rejection of traversal + acceptance of legit names. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
67 lines
2.5 KiB
Go
67 lines
2.5 KiB
Go
package appexport
|
|
|
|
import (
|
|
"fmt"
|
|
"path/filepath"
|
|
"regexp"
|
|
"strings"
|
|
)
|
|
|
|
// safeSegment matches a single safe path component: starts with an
|
|
// alphanumeric, then alphanumerics / dot / dash / underscore. It cannot be
|
|
// "." or ".." (must start alnum), cannot contain a path separator, and cannot
|
|
// be an absolute path. This covers the legitimate values these fields hold —
|
|
// app slugs (e.g. "paperless-ngx"), HDD mount basenames (e.g. "felhom-usb"),
|
|
// and docker volume names (e.g. "adventurelog_postgres_data").
|
|
var safeSegment = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9._-]*$`)
|
|
|
|
// ValidateSegment rejects any value that is not a single safe path component.
|
|
// It is the guard for [CTRL-001]: manifest fields that reach filepath.Join with
|
|
// a trusted base (AppName, HDDSubdirs, VolumeNames) are fully attacker-controlled
|
|
// JSON inside an imported .fab, so a value like "../../etc/cron.d/x" must be
|
|
// refused before it can escape the stacks / HDD destination directory.
|
|
//
|
|
// NOTE: this is deliberately NOT applied to manifest.ConfigFiles — those are
|
|
// dotfile-bearing names (e.g. ".felhom.yml") that are never used in a restore
|
|
// join (restoreConfig enumerates the extracted dir via os.ReadDir, whose names
|
|
// are already single components).
|
|
func ValidateSegment(kind, s string) error {
|
|
if s == "" {
|
|
return fmt.Errorf("appexport: empty %s", kind)
|
|
}
|
|
if s == "." || s == ".." {
|
|
return fmt.Errorf("appexport: %s %q is a path-traversal segment", kind, s)
|
|
}
|
|
if strings.ContainsAny(s, `/\`) || strings.ContainsRune(s, filepath.Separator) {
|
|
return fmt.Errorf("appexport: %s %q must not contain a path separator", kind, s)
|
|
}
|
|
if filepath.IsAbs(s) {
|
|
return fmt.Errorf("appexport: %s %q must not be an absolute path", kind, s)
|
|
}
|
|
if !safeSegment.MatchString(s) {
|
|
return fmt.Errorf("appexport: %s %q is not a safe single-segment name", kind, s)
|
|
}
|
|
return nil
|
|
}
|
|
|
|
// validateManifestPaths checks every manifest field that is later used as a
|
|
// path segment in a filepath.Join against a trusted base. Called from
|
|
// UnmarshalManifest so a hostile bundle fails the parse, before executeImport
|
|
// can MkdirAll/extract into a traversed location.
|
|
func validateManifestPaths(m *Manifest) error {
|
|
if err := ValidateSegment("app_name", m.AppName); err != nil {
|
|
return err
|
|
}
|
|
for _, s := range m.HDDSubdirs {
|
|
if err := ValidateSegment("hdd_subdir", s); err != nil {
|
|
return err
|
|
}
|
|
}
|
|
for _, v := range m.VolumeNames {
|
|
if err := ValidateSegment("volume_name", v); err != nil {
|
|
return err
|
|
}
|
|
}
|
|
return nil
|
|
}
|