Skip to content

Commit 45b038c

Browse files
tonistiigicrazy-max
authored andcommitted
git: normalize and validate subdir paths
Normalize Git subdir fragments and validate checkout subdir components so each segment must be a real directory, preventing traversal and symlink escapes. Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com> (cherry picked from commit 8c994eb561a2646b35352e5663afecd225306214) (cherry picked from commit f756b3f8aa2f32b0986c1cbb6449a29fe22715d7)
1 parent f5462c2 commit 45b038c

File tree

5 files changed

+110
-12
lines changed

5 files changed

+110
-12
lines changed

client/llb/source.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,8 @@ func Git(url, fragment string, opts ...GitOption) State {
398398
AuthTokenSecret: GitAuthTokenKey,
399399
}
400400
ref, subdir, ok := strings.Cut(fragment, ":")
401+
subdir = path.Join("/", subdir)
402+
subdir = strings.TrimPrefix(subdir, "/")
401403
if ref != "" {
402404
GitRef(ref).SetGitOption(gi)
403405
}

source/git/identifier.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
package git
22

33
import (
4-
"path"
5-
64
"github.com/moby/buildkit/solver/llbsolver/provenance"
75
provenancetypes "github.com/moby/buildkit/solver/llbsolver/provenance/types"
86
"github.com/moby/buildkit/source"
@@ -46,9 +44,6 @@ func NewGitIdentifier(remoteURL string) (*GitIdentifier, error) {
4644
repo.Ref = u.Opts.Ref
4745
repo.Subdir = u.Opts.Subdir
4846
}
49-
if sd := path.Clean(repo.Subdir); sd == "/" || sd == "." {
50-
repo.Subdir = ""
51-
}
5247
return &repo, nil
5348
}
5449

source/git/identifier_test.go

Lines changed: 69 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,14 @@ func TestNewGitIdentifier(t *testing.T) {
4848
expected: GitIdentifier{
4949
Remote: "git://github.com/user/repo.git",
5050
Ref: "mybranch",
51-
Subdir: "mydir/mysubdir/",
51+
Subdir: "mydir/mysubdir",
5252
},
5353
},
5454
{
5555
url: "git://github.com/user/repo.git#:mydir/mysubdir/",
5656
expected: GitIdentifier{
5757
Remote: "git://github.com/user/repo.git",
58-
Subdir: "mydir/mysubdir/",
58+
Subdir: "mydir/mysubdir",
5959
},
6060
},
6161
{
@@ -69,7 +69,7 @@ func TestNewGitIdentifier(t *testing.T) {
6969
expected: GitIdentifier{
7070
Remote: "https://github.com/user/repo.git",
7171
Ref: "mybranch",
72-
Subdir: "mydir/mysubdir/",
72+
Subdir: "mydir/mysubdir",
7373
},
7474
},
7575
{
@@ -83,7 +83,7 @@ func TestNewGitIdentifier(t *testing.T) {
8383
expected: GitIdentifier{
8484
Remote: "git@github.com:user/repo.git",
8585
Ref: "mybranch",
86-
Subdir: "mydir/mysubdir/",
86+
Subdir: "mydir/mysubdir",
8787
},
8888
},
8989
{
@@ -97,15 +97,78 @@ func TestNewGitIdentifier(t *testing.T) {
9797
expected: GitIdentifier{
9898
Remote: "ssh://github.com/user/repo.git",
9999
Ref: "mybranch",
100-
Subdir: "mydir/mysubdir/",
100+
Subdir: "mydir/mysubdir",
101101
},
102102
},
103103
{
104104
url: "ssh://foo%40barcorp.com@github.com/user/repo.git#mybranch:mydir/mysubdir/",
105105
expected: GitIdentifier{
106106
Remote: "ssh://foo%40barcorp.com@github.com/user/repo.git",
107107
Ref: "mybranch",
108-
Subdir: "mydir/mysubdir/",
108+
Subdir: "mydir/mysubdir",
109+
},
110+
},
111+
{
112+
url: "https://github.com/user/repo.git#main:../../escape",
113+
expected: GitIdentifier{
114+
Remote: "https://github.com/user/repo.git",
115+
Ref: "main",
116+
Subdir: "escape",
117+
},
118+
},
119+
{
120+
url: "https://github.com/user/repo.git#main:dir/../../escape",
121+
expected: GitIdentifier{
122+
Remote: "https://github.com/user/repo.git",
123+
Ref: "main",
124+
Subdir: "escape",
125+
},
126+
},
127+
{
128+
url: "https://github.com/user/repo.git#main:/absolute/path",
129+
expected: GitIdentifier{
130+
Remote: "https://github.com/user/repo.git",
131+
Ref: "main",
132+
Subdir: "absolute/path",
133+
},
134+
},
135+
{
136+
url: "https://github.com/user/repo.git#main:../",
137+
expected: GitIdentifier{
138+
Remote: "https://github.com/user/repo.git",
139+
Ref: "main",
140+
},
141+
},
142+
{
143+
url: "ssh://github.com/user/repo.git#main:../../escape",
144+
expected: GitIdentifier{
145+
Remote: "ssh://github.com/user/repo.git",
146+
Ref: "main",
147+
Subdir: "escape",
148+
},
149+
},
150+
{
151+
url: "ssh://github.com/user/repo.git#main:/absolute/path",
152+
expected: GitIdentifier{
153+
Remote: "ssh://github.com/user/repo.git",
154+
Ref: "main",
155+
Subdir: "absolute/path",
156+
},
157+
},
158+
{
159+
url: "git@github.com:user/repo.git#main:../../escape",
160+
expected: GitIdentifier{
161+
Remote: "git@github.com:user/repo.git",
162+
Ref: "main",
163+
Subdir: "escape",
164+
},
165+
},
166+
{
167+
url: "git@github.com:user/repo.git#main:/absolute/path",
168+
expected: GitIdentifier{
169+
Remote: "git@github.com:user/repo.git",
170+
Ref: "main",
171+
Subdir: "absolute/path",
109172
},
110173
},
111174
}

source/git/source.go

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -994,7 +994,7 @@ func (gs *gitSourceHandler) checkout(ctx context.Context, repo *gitRepo, g sessi
994994
}
995995
}()
996996

997-
subdir := path.Clean(gs.src.Subdir)
997+
subdir := path.Join("/", gs.src.Subdir)
998998
if subdir == "/" {
999999
subdir = "."
10001000
}
@@ -1087,6 +1087,11 @@ func (gs *gitSourceHandler) checkout(ctx context.Context, repo *gitRepo, g sessi
10871087
}
10881088

10891089
if subdir != "." {
1090+
subdir = filepath.FromSlash(subdir)
1091+
if err := validateDirsOnly(cd, subdir); err != nil {
1092+
return nil, errors.Wrapf(err, "invalid subdir %v", subdir)
1093+
}
1094+
10901095
d, err := os.Open(filepath.Join(cd, subdir))
10911096
if err != nil {
10921097
return nil, errors.Wrapf(err, "failed to open subdir %v", subdir)
@@ -1298,3 +1303,33 @@ func gitCLI(opts ...gitutil.Option) *gitutil.GitCLI {
12981303
}, opts...)
12991304
return gitutil.NewGitCLI(opts...)
13001305
}
1306+
1307+
// validateDirsOnly checks that the given subpath in the repository
1308+
// only contains directories without any symlinks or files.
1309+
func validateDirsOnly(root string, subpath string) error {
1310+
rel := filepath.Clean(subpath)
1311+
rel = strings.TrimPrefix(rel, string(filepath.Separator))
1312+
if rel == "" || rel == "." {
1313+
return nil
1314+
}
1315+
1316+
r, err := os.OpenRoot(root)
1317+
if err != nil {
1318+
return errors.Wrapf(err, "failed to open root %q", root)
1319+
}
1320+
defer r.Close()
1321+
1322+
p := ""
1323+
for part := range strings.SplitSeq(rel, string(filepath.Separator)) {
1324+
p = filepath.Join(p, part)
1325+
1326+
fi, err := r.Lstat(p)
1327+
if err != nil {
1328+
return errors.Wrapf(err, "failed to lstat %q", p)
1329+
}
1330+
if !fi.IsDir() {
1331+
return errors.Errorf("git subpath %q contains non-directory %q", subpath, p)
1332+
}
1333+
}
1334+
return nil
1335+
}

util/gitutil/git_url.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package gitutil
22

33
import (
44
"net/url"
5+
"path"
56
"regexp"
67
"strings"
78

@@ -72,6 +73,8 @@ func parseOpts(fragment string) *GitURLOpts {
7273
return nil
7374
}
7475
ref, subdir, _ := strings.Cut(fragment, ":")
76+
subdir = path.Join("/", subdir)
77+
subdir = strings.TrimPrefix(subdir, "/")
7578
return &GitURLOpts{Ref: ref, Subdir: subdir}
7679
}
7780

0 commit comments

Comments
 (0)