Validate secret name in GetSecretVersion to prevent path traversal
Add isValidSecretName() check at the top of GetSecretVersion(), matching the existing validation in AddSecret(). Without this, crafted secret names containing path traversal sequences (e.g. '../../../etc/passwd') could be used to read files outside the vault directory. Add regression tests for both GetSecretVersion and GetSecret. Closes #13
This commit is contained in:
parent
6ff00c696a
commit
3fd30bb9e6
66
internal/vault/path_traversal_test.go
Normal file
66
internal/vault/path_traversal_test.go
Normal file
@ -0,0 +1,66 @@
|
||||
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",
|
||||
}
|
||||
|
||||
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")
|
||||
}
|
||||
@ -319,6 +319,12 @@ func (v *Vault) GetSecretVersion(name string, version string) ([]byte, error) {
|
||||
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
|
||||
vaultDir, err := v.GetDirectory()
|
||||
if err != nil {
|
||||
|
||||
Loading…
Reference in New Issue
Block a user