From a15713329251b758d2c72a9a7a29d707a5d6e533 Mon Sep 17 00:00:00 2001
From: Yousef Alowayed <yalo@google.com>
Date: Wed, 2 Oct 2024 05:55:49 -0700
Subject: [PATCH] Handle symlink attacks with an absolute target.

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
---
 artifact/image/symlink/symlink.go      |  8 ++++++--
 artifact/image/symlink/symlink_test.go | 18 ++++++++++++++++++
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/artifact/image/symlink/symlink.go b/artifact/image/symlink/symlink.go
index 47383c4..64d7bfd 100644
--- a/artifact/image/symlink/symlink.go
+++ b/artifact/image/symlink/symlink.go
@@ -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)
 }
diff --git a/artifact/image/symlink/symlink_test.go b/artifact/image/symlink/symlink_test.go
index b1a1c0a..0ca803b 100644
--- a/artifact/image/symlink/symlink_test.go
+++ b/artifact/image/symlink/symlink_test.go
@@ -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 {
@@ -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 {