Skip to content

Commit 3de9b0c

Browse files
idsulikglours
idsulik
authored and
glours
committed
fix(git): Add validation for Git subdirectory paths to prevent traversal
Signed-off-by: Suleiman Dibirov <idsulik@gmail.com> (cherry picked from commit 0d396bb) Signed-off-by: Guillaume Lours <705411+glours@users.noreply.github.com>
1 parent 1d4acfb commit 3de9b0c

File tree

2 files changed

+214
-0
lines changed

2 files changed

+214
-0
lines changed

‎pkg/remote/git.go‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"path/filepath"
2727
"regexp"
2828
"strconv"
29+
"strings"
2930

3031
"github.com/compose-spec/compose-go/v2/cli"
3132
"github.com/compose-spec/compose-go/v2/loader"
@@ -113,6 +114,9 @@ func (g gitRemoteLoader) Load(ctx context.Context, path string) (string, error)
113114
g.known[path] = local
114115
}
115116
if ref.SubDir != "" {
117+
if err := validateGitSubDir(local, ref.SubDir); err != nil {
118+
return "", err
119+
}
116120
local = filepath.Join(local, ref.SubDir)
117121
}
118122
stat, err := os.Stat(local)
@@ -129,6 +133,41 @@ func (g gitRemoteLoader) Dir(path string) string {
129133
return g.known[path]
130134
}
131135

136+
// validateGitSubDir ensures a subdirectory path is contained within the base directory
137+
// and doesn't escape via path traversal. Unlike validatePathInBase for OCI artifacts,
138+
// this allows nested directories but prevents traversal outside the base.
139+
func validateGitSubDir(base, subDir string) error {
140+
cleanSubDir := filepath.Clean(subDir)
141+
142+
if filepath.IsAbs(cleanSubDir) {
143+
return fmt.Errorf("git subdirectory must be relative, got: %s", subDir)
144+
}
145+
146+
if cleanSubDir == ".." || strings.HasPrefix(cleanSubDir, "../") || strings.HasPrefix(cleanSubDir, "..\\") {
147+
return fmt.Errorf("git subdirectory path traversal detected: %s", subDir)
148+
}
149+
150+
if len(cleanSubDir) >= 2 && cleanSubDir[1] == ':' {
151+
return fmt.Errorf("git subdirectory must be relative, got: %s", subDir)
152+
}
153+
154+
targetPath := filepath.Join(base, cleanSubDir)
155+
cleanBase := filepath.Clean(base)
156+
cleanTarget := filepath.Clean(targetPath)
157+
158+
// Ensure the target starts with the base path
159+
relPath, err := filepath.Rel(cleanBase, cleanTarget)
160+
if err != nil {
161+
return fmt.Errorf("invalid git subdirectory path: %w", err)
162+
}
163+
164+
if relPath == ".." || strings.HasPrefix(relPath, "../") || strings.HasPrefix(relPath, "..\\") {
165+
return fmt.Errorf("git subdirectory escapes base directory: %s", subDir)
166+
}
167+
168+
return nil
169+
}
170+
132171
func (g gitRemoteLoader) resolveGitRef(ctx context.Context, path string, ref *gitutil.GitRef) error {
133172
if !commitSHA.MatchString(ref.Ref) {
134173
cmd := exec.CommandContext(ctx, "git", "ls-remote", "--exit-code", ref.Remote, ref.Ref)

‎pkg/remote/git_test.go‎

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
/*
2+
Copyright 2020 Docker Compose CLI authors
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package remote
18+
19+
import (
20+
"testing"
21+
22+
"gotest.tools/v3/assert"
23+
)
24+
25+
func TestValidateGitSubDir(t *testing.T) {
26+
base := "/tmp/cache/compose/abc123def456"
27+
28+
tests := []struct {
29+
name string
30+
subDir string
31+
wantErr bool
32+
}{
33+
{
34+
name: "valid simple directory",
35+
subDir: "examples",
36+
wantErr: false,
37+
},
38+
{
39+
name: "valid nested directory",
40+
subDir: "examples/nginx",
41+
wantErr: false,
42+
},
43+
{
44+
name: "valid deeply nested directory",
45+
subDir: "examples/web/frontend/config",
46+
wantErr: false,
47+
},
48+
{
49+
name: "valid current directory",
50+
subDir: ".",
51+
wantErr: false,
52+
},
53+
{
54+
name: "valid directory with redundant separators",
55+
subDir: "examples//nginx",
56+
wantErr: false,
57+
},
58+
{
59+
name: "valid directory with dots in name",
60+
subDir: "examples/nginx.conf.d",
61+
wantErr: false,
62+
},
63+
{
64+
name: "path traversal - parent directory",
65+
subDir: "..",
66+
wantErr: true,
67+
},
68+
{
69+
name: "path traversal - multiple parent directories",
70+
subDir: "../../../etc/passwd",
71+
wantErr: true,
72+
},
73+
{
74+
name: "path traversal - deeply nested escape",
75+
subDir: "../../../../../../../tmp/pwned",
76+
wantErr: true,
77+
},
78+
{
79+
name: "path traversal - mixed with valid path",
80+
subDir: "examples/../../etc/passwd",
81+
wantErr: true,
82+
},
83+
{
84+
name: "path traversal - at the end",
85+
subDir: "examples/..",
86+
wantErr: false, // This resolves to "." which is the current directory, safe
87+
},
88+
{
89+
name: "path traversal - in the middle",
90+
subDir: "examples/../../../etc/passwd",
91+
wantErr: true,
92+
},
93+
{
94+
name: "path traversal - windows style",
95+
subDir: "..\\..\\..\\windows\\system32",
96+
wantErr: true,
97+
},
98+
{
99+
name: "absolute unix path",
100+
subDir: "/etc/passwd",
101+
wantErr: true,
102+
},
103+
{
104+
name: "absolute windows path",
105+
subDir: "C:\\windows\\system32\\config\\sam",
106+
wantErr: true,
107+
},
108+
{
109+
name: "absolute path with home directory",
110+
subDir: "/home/user/.ssh/id_rsa",
111+
wantErr: true,
112+
},
113+
{
114+
name: "normalized path that would escape",
115+
subDir: "./../../etc/passwd",
116+
wantErr: true,
117+
},
118+
{
119+
name: "directory name with three dots",
120+
subDir: ".../config",
121+
wantErr: false,
122+
},
123+
{
124+
name: "directory name with four dots",
125+
subDir: "..../config",
126+
wantErr: false,
127+
},
128+
{
129+
name: "directory name with five dots",
130+
subDir: "...../etc/passwd",
131+
wantErr: false, // ".....'' is a valid directory name, not path traversal
132+
},
133+
{
134+
name: "directory name starting with two dots and letter",
135+
subDir: "..foo/bar",
136+
wantErr: false,
137+
},
138+
}
139+
140+
for _, tt := range tests {
141+
t.Run(tt.name, func(t *testing.T) {
142+
err := validateGitSubDir(base, tt.subDir)
143+
if (err != nil) != tt.wantErr {
144+
t.Errorf("validateGitSubDir(%q, %q) error = %v, wantErr %v",
145+
base, tt.subDir, err, tt.wantErr)
146+
}
147+
})
148+
}
149+
}
150+
151+
// TestValidateGitSubDirSecurityScenarios tests specific security scenarios
152+
func TestValidateGitSubDirSecurityScenarios(t *testing.T) {
153+
base := "/var/cache/docker-compose/git/1234567890abcdef"
154+
155+
// Test the exact vulnerability scenario from the issue
156+
t.Run("CVE scenario - /tmp traversal", func(t *testing.T) {
157+
maliciousPath := "../../../../../../../tmp/pwned"
158+
err := validateGitSubDir(base, maliciousPath)
159+
assert.ErrorContains(t, err, "path traversal")
160+
})
161+
162+
// Test variations of the attack
163+
t.Run("CVE scenario - /etc traversal", func(t *testing.T) {
164+
maliciousPath := "../../../../../../../../etc/passwd"
165+
err := validateGitSubDir(base, maliciousPath)
166+
assert.ErrorContains(t, err, "path traversal")
167+
})
168+
169+
// Test that legitimate nested paths still work
170+
t.Run("legitimate nested path", func(t *testing.T) {
171+
validPath := "examples/docker-compose/nginx/config"
172+
err := validateGitSubDir(base, validPath)
173+
assert.NilError(t, err)
174+
})
175+
}

0 commit comments

Comments
 (0)