Skip to content

Commit

Permalink
Scanners fixes (#720)
Browse files Browse the repository at this point in the history
* Fix Arbitrary file access during archive extraction Zip Slip findings

* Fix Incorrect conversion between integer types finding

* Write test for SanitizePath

* gofmt the code
  • Loading branch information
burov authored Dec 18, 2024
1 parent c232087 commit cb8e6c8
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 5 deletions.
4 changes: 2 additions & 2 deletions policies/recipes/recipe.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ func convertVersion(version string) ([]int, error) {
if idx > 3 {
return nil, fmt.Errorf("invalid Version string")
}
val, err := strconv.ParseUint(element, 10, 0)
if err != nil {
val, err := strconv.ParseInt(element, 10, 0)
if err != nil || val < 0 {
return nil, fmt.Errorf("invalid Version string")
}
ret = append(ret, int(val))
Expand Down
6 changes: 3 additions & 3 deletions policies/recipes/steps.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func extractZip(zipPath string, dst string) error {

// Check for conflicts
for _, f := range zr.File {
filen, err := util.NormPath(filepath.Join(dst, f.Name))
filen, err := util.NormPath(util.SanitizePath(filepath.Join(dst, f.Name)))
if err != nil {
return err
}
Expand All @@ -156,7 +156,7 @@ func extractZip(zipPath string, dst string) error {

// Create files.
for _, f := range zr.File {
filen, err := util.NormPath(filepath.Join(dst, f.Name))
filen, err := util.NormPath(util.SanitizePath(filepath.Join(dst, f.Name)))
if err != nil {
return err
}
Expand Down Expand Up @@ -289,7 +289,7 @@ func extractTar(ctx context.Context, tarName string, dst string, archiveType age
if err != nil {
return err
}
filen, err := util.NormPath(filepath.Join(dst, header.Name))
filen, err := util.NormPath(filepath.Join(dst, util.SanitizePath(header.Name)))
if err != nil {
return err
}
Expand Down
8 changes: 8 additions & 0 deletions util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,14 @@ type Logger struct {
Fatalf func(string, ...any)
}

// SanitizePath ensures that relative path does not contains ".." to avoid directory traversal attacks.
// As well run filepath.Clean to remove redundant path segments.
func SanitizePath(path string) string {
sanitized := strings.ReplaceAll(path, "../", "")

return filepath.Clean(sanitized)
}

// NormPath transforms a windows path into an extended-length path as described in
// https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx#maxpath
// when not running on windows it will just return the input path.
Expand Down
46 changes: 46 additions & 0 deletions util/util_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package util

import (
"testing"
)

func TestSanitizePath(t *testing.T) {
tests := []struct {
name string
input string
expectedOutput string
}{
{
name: "Basic file name",
input: "test.yaml",
expectedOutput: "test.yaml",
},
{
name: "Basic full path",
input: "/x/test.yaml",
expectedOutput: "/x/test.yaml",
},
{
name: "Relative path",
input: "x/test.yaml",
expectedOutput: "x/test.yaml",
},
{
name: "Relative path with traversal segment",
input: "../x/test.yaml",
expectedOutput: "x/test.yaml",
},
{
name: "Relative path with traversal segment",
input: "/../x/test.yaml",
expectedOutput: "/x/test.yaml",
},
}

for _, tt := range tests {
if result := SanitizePath(tt.input); result != tt.expectedOutput {
t.Errorf("Test %q failed, expectedOutput %q, got %q", tt.name, tt.expectedOutput, result)
}

}
}

0 comments on commit cb8e6c8

Please sign in to comment.