-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request from GHSA-6gcg-hp2x-q54h
* fix: do not allow symlinks from directory-type applications Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * chore: fix imports and unnecessary parameters Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * chore: lint Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * chore: use t.TempDir for simpler tests Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * address comments Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>
- Loading branch information
1 parent
5cee8f8
commit 5e767a4
Showing
4 changed files
with
202 additions
and
14 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| package files | ||
|
|
||
| import ( | ||
| "io/fs" | ||
| "os" | ||
| "path/filepath" | ||
| "strings" | ||
| ) | ||
|
|
||
| // Inbound will validate if the given candidate path is inside the | ||
| // baseDir. This is useful to make sure that malicious candidates | ||
| // are not targeting a file outside of baseDir boundaries. | ||
| // Considerations: | ||
| // - baseDir must be absolute path. Will return false otherwise | ||
| // - candidate can be absolute or relative path | ||
| // - candidate should not be symlink as only syntatic validation is | ||
| // applied by this function | ||
| func Inbound(candidate, baseDir string) bool { | ||
| if !filepath.IsAbs(baseDir) { | ||
| return false | ||
| } | ||
| var target string | ||
| if filepath.IsAbs(candidate) { | ||
| target = filepath.Clean(candidate) | ||
| } else { | ||
| target = filepath.Join(baseDir, candidate) | ||
| } | ||
| return strings.HasPrefix(target, filepath.Clean(baseDir)+string(os.PathSeparator)) | ||
| } | ||
|
|
||
| // IsSymlink return true if the given FileInfo relates to a | ||
| // symlink file. Returns false otherwise. | ||
| func IsSymlink(fi os.FileInfo) bool { | ||
| return fi.Mode()&fs.ModeSymlink == fs.ModeSymlink | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| package files_test | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
|
|
||
| "github.com/argoproj/argo-cd/v2/util/io/files" | ||
| ) | ||
|
|
||
| func TestInbound(t *testing.T) { | ||
| type testcase struct { | ||
| name string | ||
| candidate string | ||
| basedir string | ||
| expected bool | ||
| } | ||
| cases := []testcase{ | ||
| { | ||
| name: "will return true if candidate is inbound", | ||
| candidate: "/home/test/app/readme.md", | ||
| basedir: "/home/test", | ||
| expected: true, | ||
| }, | ||
| { | ||
| name: "will return false if candidate is not inbound", | ||
| candidate: "/home/test/../readme.md", | ||
| basedir: "/home/test", | ||
| expected: false, | ||
| }, | ||
| { | ||
| name: "will return true if candidate is relative inbound", | ||
| candidate: "./readme.md", | ||
| basedir: "/home/test", | ||
| expected: true, | ||
| }, | ||
| { | ||
| name: "will return false if candidate is relative outbound", | ||
| candidate: "../readme.md", | ||
| basedir: "/home/test", | ||
| expected: false, | ||
| }, | ||
| { | ||
| name: "will return false if basedir is relative", | ||
| candidate: "/home/test/app/readme.md", | ||
| basedir: "./test", | ||
| expected: false, | ||
| }, | ||
| } | ||
| for _, c := range cases { | ||
| c := c | ||
| t.Run(c.name, func(t *testing.T) { | ||
| // given | ||
| t.Parallel() | ||
|
|
||
| // when | ||
| inbound := files.Inbound(c.candidate, c.basedir) | ||
|
|
||
| // then | ||
| assert.Equal(t, c.expected, inbound) | ||
| }) | ||
| } | ||
| } |