-
Notifications
You must be signed in to change notification settings - Fork 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: add new util file 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
df79d7d
commit d36d95d
Showing
4 changed files
with
198 additions
and
9 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) | ||
| }) | ||
| } | ||
| } |