Skip to content

Commit 709c573

Browse files
committed
common: fix path containment corner cases in passdriver
With the current implementation of getPath, it's possible to return a path outside the root directory, matching a sibling directory instead through a crafted/specific id. Signed-off-by: Caleb Xu <caxu@redhat.com>
1 parent 3592bda commit 709c573

2 files changed

Lines changed: 29 additions & 3 deletions

File tree

common/pkg/secrets/passdriver/passdriver.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ func (d *Driver) getPath(id string) (string, error) {
171171
if err != nil {
172172
return "", define.ErrInvalidKey
173173
}
174-
if !strings.HasPrefix(path, d.Root) {
174+
if !strings.HasPrefix(path, d.Root+string(filepath.Separator)) {
175175
return "", define.ErrInvalidKey
176176
}
177177
return path + ".gpg", nil

common/pkg/secrets/passdriver/passdriver_test.go

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package passdriver
33
import (
44
"context"
55
"fmt"
6+
"os"
7+
"path/filepath"
68
"testing"
79

810
"github.com/stretchr/testify/require"
@@ -11,9 +13,17 @@ import (
1113

1214
const gpgTestID = "testing@passdriver"
1315

16+
// setupDriver configures a driver for tests under a single temp directory, including
17+
// the root, sibling directory (storex) for testing prefix collisions, and gpg homedir.
1418
func setupDriver(t *testing.T) *Driver {
15-
base := t.TempDir()
16-
gpghomedir := t.TempDir()
19+
t.Helper()
20+
tmpdir := t.TempDir()
21+
base := filepath.Join(tmpdir, "store")
22+
sibling := filepath.Join(tmpdir, "storex")
23+
gpghomedir := filepath.Join(tmpdir, "gpg")
24+
require.NoError(t, os.MkdirAll(base, 0o755))
25+
require.NoError(t, os.MkdirAll(sibling, 0o755))
26+
require.NoError(t, os.MkdirAll(gpghomedir, 0o755))
1727

1828
driver, err := NewDriver(map[string]string{
1929
"root": base,
@@ -57,6 +67,12 @@ func TestStoreAndLookup(t *testing.T) {
5767
value: []byte("abc"),
5868
expStoreErr: define.ErrInvalidKey,
5969
},
70+
{
71+
name: "storing with root path prefix collision fails",
72+
key: "../storex/marker",
73+
value: []byte("abc"),
74+
expStoreErr: define.ErrInvalidKey,
75+
},
6076
}
6177

6278
for _, tc := range cases {
@@ -109,6 +125,11 @@ func TestLookup(t *testing.T) {
109125
key: "../../../etc/shadow",
110126
expErr: define.ErrInvalidKey,
111127
},
128+
{
129+
name: "lookup with root path prefix collision fails",
130+
key: "../storex/marker",
131+
expErr: define.ErrInvalidKey,
132+
},
112133
}
113134

114135
for _, tc := range cases {
@@ -157,6 +178,11 @@ func TestDelete(t *testing.T) {
157178
key: "../../../etc/shadow",
158179
expErr: define.ErrInvalidKey,
159180
},
181+
{
182+
name: "delete with root path prefix collision fails",
183+
key: "../storex/marker",
184+
expErr: define.ErrInvalidKey,
185+
},
160186
}
161187

162188
for _, tc := range cases {

0 commit comments

Comments
 (0)