Skip to content

Commit

Permalink
Handle symlink attacks with an absolute target.
Browse files Browse the repository at this point in the history
In cases where a symlink has an absolute target like `/../secret`, we want to remove these symlinks instead of creating or operating on them.

PiperOrigin-RevId: 681420060
  • Loading branch information
Yousef Alowayed authored and copybara-github committed Oct 2, 2024
1 parent 8c96911 commit a157133
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 2 deletions.
8 changes: 6 additions & 2 deletions artifact/image/symlink/symlink.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,11 +199,15 @@ func removeLayerPathPrefix(path, layerPath string) string {
// this function would return true because the target file is outside of the root directory.
func TargetOutsideRoot(path, target string) bool {

// Create a marker directory as root to check if the target path is outside of the root directory.
markerDir := uuid.New().String()
if filepath.IsAbs(target) {
return false
// Absolute paths may still point outside of the root directory.
// e.g. "/../file.txt"
markerTarget := filepath.Join(markerDir, target)
return !strings.Contains(markerTarget, markerDir)
}

markerDir := uuid.New().String()
markerTargetAbs := filepath.Join(markerDir, filepath.Dir(path), target)
return !strings.Contains(markerTargetAbs, markerDir)
}
18 changes: 18 additions & 0 deletions artifact/image/symlink/symlink_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,14 @@
package symlink_test

import (
"runtime"
"testing"

"github.com/google/osv-scalibr/artifact/image/symlink"
)

var ()

func TestTargetOutsideRoot(t *testing.T) {

tests := []struct {
Expand Down Expand Up @@ -52,6 +55,21 @@ func TestTargetOutsideRoot(t *testing.T) {
path: "a/f.txt",
target: "../../t.txt",
want: true,
}, {
name: "absolute target outside root",
path: "a/f.txt",
target: func() string {
if runtime.GOOS == "windows" {
return "\\\\..\\t.txt"
}
return "/../t.txt"
}(),
want: true,
}, {
name: "absolute target inside root",
path: "a/b/f.txt",
target: "/a/../c/t.txt",
want: false,
}}

for _, tc := range tests {
Expand Down

0 comments on commit a157133

Please sign in to comment.