diff --git a/libs/filer/local_root_path.go b/libs/filer/local_root_path.go index 3f8843093aa..f2bbe93be3c 100644 --- a/libs/filer/local_root_path.go +++ b/libs/filer/local_root_path.go @@ -19,7 +19,21 @@ func NewLocalRootPath(root string) localRootPath { func (rp *localRootPath) Join(name string) (string, error) { absPath := filepath.Join(rp.rootPath, name) - if !strings.HasPrefix(absPath, rp.rootPath) { + + // An empty root carries no restriction (cmd/fs constructs unrooted local filers). + if rp.rootPath == "" { + return absPath, nil + } + + // Joining exactly the root must stay allowed: calls like ReadDir(".") resolve to it. + // Any other path must extend the root past a separator boundary; a plain prefix + // check would also accept siblings like "/root-evil" for root "/root". + // Cleaned roots like "/" or `C:\` already end in a separator. + root := rp.rootPath + if !strings.HasSuffix(root, string(filepath.Separator)) { + root += string(filepath.Separator) + } + if absPath != rp.rootPath && !strings.HasPrefix(absPath, root) { return "", fmt.Errorf("relative path escapes root: %s", name) } return absPath, nil diff --git a/libs/filer/local_root_path_test.go b/libs/filer/local_root_path_test.go index 1a39c4463b3..a665a9de7bc 100644 --- a/libs/filer/local_root_path_test.go +++ b/libs/filer/local_root_path_test.go @@ -52,6 +52,11 @@ func testUnixLocalRootPath(t *testing.T, uncleanRoot string) { assert.NoError(t, err) assert.Equal(t, cleanRoot, remotePath) + // All roots in this test end in the path element "path". + remotePath, err = rp.Join("../path/x") + assert.NoError(t, err) + assert.Equal(t, cleanRoot+"/x", remotePath) + _, err = rp.Join("..") assert.ErrorContains(t, err, `relative path escapes root: ..`) @@ -78,6 +83,16 @@ func testUnixLocalRootPath(t *testing.T, uncleanRoot string) { _, err = rp.Join("../..") assert.ErrorContains(t, err, `relative path escapes root: ../..`) + + // Siblings that share the root as a string prefix must be rejected. + _, err = rp.Join("../path-evil") + assert.ErrorContains(t, err, `relative path escapes root: ../path-evil`) + + _, err = rp.Join("../path-evil/x") + assert.ErrorContains(t, err, `relative path escapes root: ../path-evil/x`) + + _, err = rp.Join("../pathx") + assert.ErrorContains(t, err, `relative path escapes root: ../pathx`) } func TestUnixLocalRootPath(t *testing.T) { @@ -128,6 +143,21 @@ func testWindowsLocalRootPath(t *testing.T, uncleanRoot string) { _, err = rp.Join(`a\..\..`) assert.ErrorContains(t, err, `relative path escapes root`) + + // All roots in this test end in the path element "path". + remotePath, err = rp.Join(`..\path\x`) + assert.NoError(t, err) + assert.Equal(t, cleanRoot+`\x`, remotePath) + + // Siblings that share the root as a string prefix must be rejected. + _, err = rp.Join(`..\path-evil`) + assert.ErrorContains(t, err, `relative path escapes root`) + + _, err = rp.Join(`..\path-evil\x`) + assert.ErrorContains(t, err, `relative path escapes root`) + + _, err = rp.Join(`..\pathx`) + assert.ErrorContains(t, err, `relative path escapes root`) } func TestWindowsLocalRootPath(t *testing.T) { @@ -140,3 +170,16 @@ func TestWindowsLocalRootPath(t *testing.T) { testWindowsLocalRootPath(t, `c:\some\root\path\.`) testWindowsLocalRootPath(t, `C:\some\root\..\path\`) } + +func TestLocalRootPathEmptyRoot(t *testing.T) { + // An empty root is the unrooted local filer used by cmd/fs. + rp := NewLocalRootPath("") + + p, err := rp.Join("a/b") + assert.NoError(t, err) + assert.Equal(t, filepath.Clean("a/b"), p) + + p, err = rp.Join("../a") + assert.NoError(t, err) + assert.Equal(t, filepath.Clean("../a"), p) +} diff --git a/libs/filer/workspace_root_path.go b/libs/filer/workspace_root_path.go index d5163924a15..5da25c86ae8 100644 --- a/libs/filer/workspace_root_path.go +++ b/libs/filer/workspace_root_path.go @@ -26,7 +26,15 @@ func (p *WorkspaceRootPath) Join(name string) (string, error) { absPath := path.Join(p.rootPath, name) // Don't allow escaping the specified root using relative paths. - if !strings.HasPrefix(absPath, p.rootPath) { + // Joining exactly the root must stay allowed: calls like ReadDir(".") resolve to it. + // Any other path must extend the root past a separator boundary; a plain prefix + // check would also accept siblings like "/root-evil" for root "/root". + // Cleaned roots like "/" (see cmd/fs) already end in a separator. + root := p.rootPath + if !strings.HasSuffix(root, "/") { + root += "/" + } + if absPath != p.rootPath && !strings.HasPrefix(absPath, root) { return "", fmt.Errorf("relative path escapes root: %s", name) } diff --git a/libs/filer/workspace_root_path_test.go b/libs/filer/workspace_root_path_test.go index 73746d274f8..beadf833b5b 100644 --- a/libs/filer/workspace_root_path_test.go +++ b/libs/filer/workspace_root_path_test.go @@ -51,6 +51,11 @@ func testRootPath(t *testing.T, uncleanRoot string) { assert.NoError(t, err) assert.Equal(t, cleanRoot, remotePath) + // All roots in this test end in the path element "path". + remotePath, err = rp.Join("../path/x") + assert.NoError(t, err) + assert.Equal(t, cleanRoot+"/x", remotePath) + _, err = rp.Join("..") assert.ErrorContains(t, err, `relative path escapes root: ..`) @@ -77,6 +82,16 @@ func testRootPath(t *testing.T, uncleanRoot string) { _, err = rp.Join("../..") assert.ErrorContains(t, err, `relative path escapes root: ../..`) + + // Siblings that share the root as a string prefix must be rejected. + _, err = rp.Join("../path-evil") + assert.ErrorContains(t, err, `relative path escapes root: ../path-evil`) + + _, err = rp.Join("../path-evil/x") + assert.ErrorContains(t, err, `relative path escapes root: ../path-evil/x`) + + _, err = rp.Join("../pathx") + assert.ErrorContains(t, err, `relative path escapes root: ../pathx`) } func TestRootPathClean(t *testing.T) {