Validate secret name in GetSecretVersion to prevent path traversal (closes #13) #15
96
internal/vault/path_traversal_test.go
Normal file
96
internal/vault/path_traversal_test.go
Normal file
@ -0,0 +1,96 @@
|
|||||||
|
package vault
|
||||||
|
|
||||||
|
import (
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"git.eeqj.de/sneak/secret/internal/secret"
|
||||||
|
"github.com/awnumar/memguard"
|
||||||
|
"github.com/spf13/afero"
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
)
|
||||||
|
|
||||||
|
// TestGetSecretVersionRejectsPathTraversal verifies that GetSecretVersion
|
||||||
|
// validates the secret name and rejects path traversal attempts.
|
||||||
|
// This is a regression test for https://git.eeqj.de/sneak/secret/issues/13
|
||||||
|
func TestGetSecretVersionRejectsPathTraversal(t *testing.T) {
|
||||||
|
testMnemonic := "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about"
|
||||||
|
t.Setenv(secret.EnvMnemonic, testMnemonic)
|
||||||
|
t.Setenv(secret.EnvUnlockPassphrase, "test-passphrase")
|
||||||
|
|
||||||
|
fs := afero.NewMemMapFs()
|
||||||
|
stateDir := "/test/state"
|
||||||
|
|
||||||
|
vlt, err := CreateVault(fs, stateDir, "test-vault")
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
// Add a legitimate secret so the vault is set up
|
||||||
|
value := memguard.NewBufferFromBytes([]byte("legitimate-secret"))
|
||||||
|
err = vlt.AddSecret("legit", value, false)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
// These names contain path traversal and should be rejected
|
||||||
|
maliciousNames := []string{
|
||||||
|
"../../../etc/passwd",
|
||||||
|
"..%2f..%2fetc/passwd",
|
||||||
|
".secret",
|
||||||
|
"../sibling-vault/secrets.d/target",
|
||||||
|
"foo/../bar",
|
||||||
|
"a/../../etc/passwd",
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, name := range maliciousNames {
|
||||||
|
t.Run(name, func(t *testing.T) {
|
||||||
|
_, err := vlt.GetSecretVersion(name, "")
|
||||||
|
assert.Error(t, err, "GetSecretVersion should reject malicious name: %s", name)
|
||||||
|
assert.Contains(t, err.Error(), "invalid secret name",
|
||||||
|
"error should indicate invalid name for: %s", name)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestGetSecretRejectsPathTraversal verifies GetSecret (which calls GetSecretVersion)
|
||||||
|
// also rejects path traversal names.
|
||||||
|
func TestGetSecretRejectsPathTraversal(t *testing.T) {
|
||||||
|
testMnemonic := "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about"
|
||||||
|
t.Setenv(secret.EnvMnemonic, testMnemonic)
|
||||||
|
t.Setenv(secret.EnvUnlockPassphrase, "test-passphrase")
|
||||||
|
|
||||||
|
fs := afero.NewMemMapFs()
|
||||||
|
stateDir := "/test/state"
|
||||||
|
|
||||||
|
vlt, err := CreateVault(fs, stateDir, "test-vault")
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
_, err = vlt.GetSecret("../../../etc/passwd")
|
||||||
|
assert.Error(t, err)
|
||||||
|
assert.Contains(t, err.Error(), "invalid secret name")
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestGetSecretObjectRejectsPathTraversal verifies GetSecretObject
|
||||||
|
// also validates names and rejects path traversal attempts.
|
||||||
|
func TestGetSecretObjectRejectsPathTraversal(t *testing.T) {
|
||||||
|
testMnemonic := "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about"
|
||||||
|
t.Setenv(secret.EnvMnemonic, testMnemonic)
|
||||||
|
t.Setenv(secret.EnvUnlockPassphrase, "test-passphrase")
|
||||||
|
|
||||||
|
fs := afero.NewMemMapFs()
|
||||||
|
stateDir := "/test/state"
|
||||||
|
|
||||||
|
vlt, err := CreateVault(fs, stateDir, "test-vault")
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
maliciousNames := []string{
|
||||||
|
"../../../etc/passwd",
|
||||||
|
"foo/../bar",
|
||||||
|
"a/../../etc/passwd",
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, name := range maliciousNames {
|
||||||
|
t.Run(name, func(t *testing.T) {
|
||||||
|
_, err := vlt.GetSecretObject(name)
|
||||||
|
assert.Error(t, err, "GetSecretObject should reject: %s", name)
|
||||||
|
assert.Contains(t, err.Error(), "invalid secret name")
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
@ -92,6 +92,13 @@ func isValidSecretName(name string) bool {
|
|||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Check for path traversal via ".." components
|
||||||
|
for _, part := range strings.Split(name, "/") {
|
||||||
|
if part == ".." {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Check the basic pattern
|
// Check the basic pattern
|
||||||
matched, _ := regexp.MatchString(`^[a-z0-9\.\-\_\/]+$`, name)
|
matched, _ := regexp.MatchString(`^[a-z0-9\.\-\_\/]+$`, name)
|
||||||
|
|
||||||
@ -319,6 +326,13 @@ func (v *Vault) GetSecretVersion(name string, version string) ([]byte, error) {
|
|||||||
slog.String("version", version),
|
slog.String("version", version),
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// Validate secret name to prevent path traversal
|
||||||
|
if !isValidSecretName(name) {
|
||||||
|
secret.Debug("Invalid secret name provided", "secret_name", name)
|
||||||
|
|
||||||
|
return nil, fmt.Errorf("invalid secret name '%s': must match pattern [a-z0-9.\\-_/]+", name)
|
||||||
|
}
|
||||||
|
|
||||||
// Get vault directory
|
// Get vault directory
|
||||||
vaultDir, err := v.GetDirectory()
|
vaultDir, err := v.GetDirectory()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@ -454,6 +468,10 @@ func (v *Vault) UnlockVault() (*age.X25519Identity, error) {
|
|||||||
|
|
||||||
// GetSecretObject retrieves a Secret object with metadata loaded from this vault
|
// GetSecretObject retrieves a Secret object with metadata loaded from this vault
|
||||||
func (v *Vault) GetSecretObject(name string) (*secret.Secret, error) {
|
func (v *Vault) GetSecretObject(name string) (*secret.Secret, error) {
|
||||||
|
if !isValidSecretName(name) {
|
||||||
|
return nil, fmt.Errorf("invalid secret name: %s", name)
|
||||||
|
}
|
||||||
|
|
||||||
// First check if the secret exists by checking for the metadata file
|
// First check if the secret exists by checking for the metadata file
|
||||||
vaultDir, err := v.GetDirectory()
|
vaultDir, err := v.GetDirectory()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user