From 3d34c087a1e446738a6285c04bbde1125082b20e Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Fri, 12 May 2017 13:52:33 -0400 Subject: [PATCH] Don't allow parent references in file paths --- helper/consts/error.go | 3 +++ physical/file.go | 26 ++++++++++++++++++++++++++ physical/file_test.go | 9 +++++++++ vault/expiration.go | 4 ++++ vault/expiration_test.go | 5 +++++ vault/plugin_catalog.go | 8 ++++++++ vault/token_store.go | 5 +++++ 7 files changed, 60 insertions(+) diff --git a/helper/consts/error.go b/helper/consts/error.go index d96ba4fe84..06977d5d5a 100644 --- a/helper/consts/error.go +++ b/helper/consts/error.go @@ -10,4 +10,7 @@ var ( // ErrStandby is returned if an operation is performed on a standby Vault. // No operation is expected to succeed until active. ErrStandby = errors.New("Vault is in standby mode") + + // Used when .. is used in a path + ErrPathContainsParentReferences = errors.New("path cannot contain parent references") ) diff --git a/physical/file.go b/physical/file.go index 2e1e12c1c5..cf22b83aa0 100644 --- a/physical/file.go +++ b/physical/file.go @@ -11,6 +11,7 @@ import ( log "github.com/mgutz/logxi/v1" + "github.com/hashicorp/vault/helper/consts" "github.com/hashicorp/vault/helper/jsonutil" ) @@ -77,6 +78,10 @@ func (b *FileBackend) DeleteInternal(path string) error { return nil } + if err := b.validatePath(path); err != nil { + return err + } + basePath, key := b.expandPath(path) fullPath := filepath.Join(basePath, key) @@ -138,6 +143,10 @@ func (b *FileBackend) Get(k string) (*Entry, error) { } func (b *FileBackend) GetInternal(k string) (*Entry, error) { + if err := b.validatePath(k); err != nil { + return nil, err + } + path, key := b.expandPath(k) path = filepath.Join(path, key) @@ -172,6 +181,10 @@ func (b *FileBackend) Put(entry *Entry) error { } func (b *FileBackend) PutInternal(entry *Entry) error { + if err := b.validatePath(entry.Key); err != nil { + return err + } + path, key := b.expandPath(entry.Key) // Make the parent tree @@ -205,6 +218,10 @@ func (b *FileBackend) List(prefix string) ([]string, error) { } func (b *FileBackend) ListInternal(prefix string) ([]string, error) { + if err := b.validatePath(prefix); err != nil { + return nil, err + } + path := b.path if prefix != "" { path = filepath.Join(path, prefix) @@ -246,6 +263,15 @@ func (b *FileBackend) expandPath(k string) (string, string) { return path, "_" + key } +func (b *FileBackend) validatePath(path string) error { + switch { + case strings.Contains(path, ".."): + return consts.ErrPathContainsParentReferences + } + + return nil +} + func (b *TransactionalFileBackend) Transaction(txns []TxnEntry) error { b.permitPool.Acquire() defer b.permitPool.Release() diff --git a/physical/file_test.go b/physical/file_test.go index 9810f4beb3..8cd6e3436e 100644 --- a/physical/file_test.go +++ b/physical/file_test.go @@ -131,6 +131,15 @@ func TestFileBackend_Base64URLEncoding(t *testing.T) { } } +func TestFileBackend_ValidatePath(t *testing.T) { + if validatePath("foo/bar/../zip") { + t.Fatal("expected error") + } + if !validatePath("foo/bar/zip") { + t.Fatal("did not expect error") + } +} + func TestFileBackend(t *testing.T) { dir, err := ioutil.TempDir("", "vault") if err != nil { diff --git a/vault/expiration.go b/vault/expiration.go index 73b47b4090..a1f9fff9ba 100644 --- a/vault/expiration.go +++ b/vault/expiration.go @@ -696,6 +696,10 @@ func (m *ExpirationManager) RegisterAuth(source string, auth *logical.Auth) erro return fmt.Errorf("expiration: cannot register an auth lease with an empty token") } + if strings.Contains(source, "..") { + return fmt.Errorf("expiration: %s", consts.ErrPathContainsParentReferences) + } + // Create a lease entry le := leaseEntry{ LeaseID: path.Join(source, m.tokenStore.SaltID(auth.ClientToken)), diff --git a/vault/expiration_test.go b/vault/expiration_test.go index 7598611d9e..f5f4ff3f86 100644 --- a/vault/expiration_test.go +++ b/vault/expiration_test.go @@ -454,6 +454,11 @@ func TestExpiration_RegisterAuth(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } + + err = exp.RegisterAuth("auth/github/../login", auth) + if err == nil { + t.Fatal("expected error") + } } func TestExpiration_RegisterAuth_NoLease(t *testing.T) { diff --git a/vault/plugin_catalog.go b/vault/plugin_catalog.go index 79474e601f..09f612cc87 100644 --- a/vault/plugin_catalog.go +++ b/vault/plugin_catalog.go @@ -10,6 +10,7 @@ import ( "sync" "github.com/hashicorp/vault/helper/builtinplugins" + "github.com/hashicorp/vault/helper/consts" "github.com/hashicorp/vault/helper/jsonutil" "github.com/hashicorp/vault/helper/pluginutil" "github.com/hashicorp/vault/logical" @@ -84,6 +85,13 @@ func (c *PluginCatalog) Set(name, command string, sha256 []byte) error { return ErrDirectoryNotConfigured } + switch { + case strings.Contains(name, ".."): + fallthrough + case strings.Contains(command, ".."): + return consts.ErrPathContainsParentReferences + } + c.lock.Lock() defer c.lock.Unlock() diff --git a/vault/token_store.go b/vault/token_store.go index c0e3dec0e2..34d2b692ab 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -14,6 +14,7 @@ import ( "github.com/armon/go-metrics" "github.com/hashicorp/go-multierror" "github.com/hashicorp/go-uuid" + "github.com/hashicorp/vault/helper/consts" "github.com/hashicorp/vault/helper/jsonutil" "github.com/hashicorp/vault/helper/locksutil" "github.com/hashicorp/vault/helper/parseutil" @@ -2246,6 +2247,10 @@ func (ts *TokenStore) tokenStoreRoleCreateUpdate( entry.PathSuffix = data.Get("path_suffix").(string) } + if strings.Contains(entry.PathSuffix, "..") { + return logical.ErrorResponse(fmt.Sprintf("error registering path suffix: %s", consts.ErrPathContainsParentReferences)), nil + } + allowedPoliciesStr, ok := data.GetOk("allowed_policies") if ok { entry.AllowedPolicies = policyutil.SanitizePolicies(strings.Split(allowedPoliciesStr.(string), ","), policyutil.DoNotAddDefaultPolicy)