feat: add validations for symlink (#316)

Added validations to make sure singingkeys.json, trustpolicy.json and config.json doesnt read's symlink closes notaryproject/notation#634 Signed-off-by: Pritesh Bandi <[email protected]>
notaryproject · May 25, 2023 · 69cf11b · 69cf11b
1 parent 9b0e472
commit 69cf11b
Show file tree

Hide file tree

Showing 4 changed files with 139 additions and 62 deletions.
diff --git a/config/base.go b/config/base.go
@@ -2,6 +2,8 @@ package config
 
 import (
 	"encoding/json"
+	"fmt"
+	"io/fs"
 	"os"
 	"path/filepath"
 
@@ -28,7 +30,22 @@ func save(filePath string, cfg interface{}) error {
 
 // load reads file, parses json and stores in cfg struct
 func load(filePath string, cfg interface{}) error {
-	file, err := dir.ConfigFS().Open(filePath)
+	path, err := dir.ConfigFS().SysPath(filePath)
+	if err != nil {
+		return err
+	}
+
+	// throw error if path is a directory or is a symlink or does not exist.
+	fileInfo, err := os.Lstat(path)
+	if err != nil {
+		return err
+	}
+	mode := fileInfo.Mode()
+	if mode.IsDir() || mode&fs.ModeSymlink != 0 {
+		return fmt.Errorf("%q is not a regular file (symlinks are not supported)", path)
+	}
+
+	file, err := os.Open(path)
 	if err != nil {
 		return err
 	}

diff --git a/config/base_test.go b/config/base_test.go
@@ -0,0 +1,34 @@
+package config
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/notaryproject/notation-go/dir"
+)
+
+func TestLoadNonExistentFile(t *testing.T) {
+	dir.UserConfigDir = "testdata/valid"
+
+	var config string
+	err := load("non-existent", &config)
+	if err == nil {
+		t.Fatalf("load() expected error but not found")
+	}
+}
+
+func TestLoadSymlink(t *testing.T) {
+	root := t.TempDir()
+	dir.UserConfigDir = root
+	fileName := "symlink"
+	os.Symlink("testdata/valid/config.json", filepath.Join(root, fileName))
+
+	expectedError := fmt.Sprintf("\"%s/%s\" is not a regular file (symlinks are not supported)", dir.UserConfigDir, fileName)
+	var config string
+	err := load(fileName, &config)
+	if err != nil && err.Error() != expectedError {
+		t.Fatalf("load() expected error= %s but found= %v", expectedError, err)
+	}
+}
diff --git a/verifier/trustpolicy/trustpolicy.go b/verifier/trustpolicy/trustpolicy.go
@@ -6,6 +6,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"io/fs"
 	"os"
 	"path/filepath"
 	"regexp"
@@ -267,18 +268,34 @@ func (trustPolicyDoc *Document) GetApplicableTrustPolicy(artifactReference strin
 
 // LoadDocument loads a trust policy document from a local file system
 func LoadDocument() (*Document, error) {
-	jsonFile, err := dir.ConfigFS().Open(dir.PathTrustPolicy)
+	path, err := dir.ConfigFS().SysPath(dir.PathTrustPolicy)
 	if err != nil {
-		switch {
-		case errors.Is(err, os.ErrNotExist):
+		return nil, err
+	}
+
+	// throw error if path is a directory or a symlink or does not exist.
+	fileInfo, err := os.Lstat(path)
+	if err != nil {
+		if errors.Is(err, os.ErrNotExist) {
 			return nil, fmt.Errorf("trust policy is not present. To create a trust policy, see: %s", trustPolicyLink)
-		case errors.Is(err, os.ErrPermission):
+		}
+		return nil, err
+	}
+
+	mode := fileInfo.Mode()
+	if mode.IsDir() || mode&fs.ModeSymlink != 0 {
+		return nil, fmt.Errorf("trust policy is not a regular file (symlinks are not supported). To create a trust policy, see: %s", trustPolicyLink)
+	}
+
+	jsonFile, err := os.Open(path)
+	if err != nil {
+		if errors.Is(err, os.ErrPermission) {
 			return nil, fmt.Errorf("unable to read trust policy due to file permissions, please verify the permissions of %s", filepath.Join(dir.UserConfigDir, dir.PathTrustPolicy))
-		default:
-			return nil, err
 		}
+		return nil, err
 	}
 	defer jsonFile.Close()
+
 	policyDocument := &Document{}
 	err = json.NewDecoder(jsonFile).Decode(policyDocument)
 	if err != nil {

diff --git a/verifier/trustpolicy/trustpolicy_test.go b/verifier/trustpolicy/trustpolicy_test.go
@@ -7,6 +7,7 @@ import (
 	"path/filepath"
 	"runtime"
 	"strconv"
+	"strings"
 	"testing"
 
 	"github.com/notaryproject/notation-go/dir"
@@ -542,61 +543,69 @@ func TestApplicableTrustPolicy(t *testing.T) {
 }
 
 func TestLoadDocument(t *testing.T) {
-	// non-existing policy file
-	tempRoot := t.TempDir()
-	dir.UserConfigDir = tempRoot
-	_, err := LoadDocument()
-	if err == nil || err.Error() != fmt.Sprintf("trust policy is not present. To create a trust policy, see: %s", trustPolicyLink) {
-		t.Fatalf("TestLoadPolicyDocument should throw error for non existent policy")
-	}
-
-	// existing invalid json file
-	tempRoot = t.TempDir()
-	dir.UserConfigDir = tempRoot
-	path := filepath.Join(tempRoot, "invalid.json")
-	err = os.WriteFile(path, []byte(`{"invalid`), 0600)
-	if err != nil {
-		t.Fatalf("TestLoadPolicyDocument create invalid policy file failed. Error: %v", err)
-	}
-	_, err = LoadDocument()
-	if err == nil {
-		t.Fatalf("TestLoadPolicyDocument should throw error for invalid policy file. Error: %v", err)
-	}
 
-	// existing policy file
-	tempRoot = t.TempDir()
-	dir.UserConfigDir = tempRoot
-	path = filepath.Join(tempRoot, "trustpolicy.json")
-	policyDoc1 := dummyPolicyDocument()
-	policyJson, _ := json.Marshal(policyDoc1)
-	err = os.WriteFile(path, policyJson, 0600)
-	if err != nil {
-		t.Fatalf("TestLoadPolicyDocument create valid policy file failed. Error: %v", err)
-	}
-	_, err = LoadDocument()
-	if err != nil {
-		t.Fatalf("TestLoadPolicyDocument should not throw error for an existing policy file. Error: %v", err)
-	}
+	t.Run("non-existing policy file", func(t *testing.T) {
+		tempRoot := t.TempDir()
+		dir.UserConfigDir = tempRoot
+		if _, err := LoadDocument(); err == nil || err.Error() != fmt.Sprintf("trust policy is not present. To create a trust policy, see: %s", trustPolicyLink) {
+			t.Fatalf("TestLoadPolicyDocument should throw error for non existent policy")
+		}
+	})
+
+	t.Run("invalid json file", func(t *testing.T) {
+		tempRoot := t.TempDir()
+		dir.UserConfigDir = tempRoot
+		path := filepath.Join(tempRoot, "invalid.json")
+		if err := os.WriteFile(path, []byte(`{"invalid`), 0600); err != nil {
+			t.Fatalf("TestLoadPolicyDocument create invalid policy file failed. Error: %v", err)
+		}
+		if _, err := LoadDocument(); err == nil {
+			t.Fatalf("TestLoadPolicyDocument should throw error for invalid policy file. Error: %v", err)
+		}
+	})
+
+	t.Run("valid policy file", func(t *testing.T) {
+		tempRoot := t.TempDir()
+		dir.UserConfigDir = tempRoot
+		path := filepath.Join(tempRoot, "trustpolicy.json")
+		policyDoc1 := dummyPolicyDocument()
+		policyJson, _ := json.Marshal(policyDoc1)
+		if err := os.WriteFile(path, policyJson, 0600); err != nil {
+			t.Fatalf("TestLoadPolicyDocument create valid policy file failed. Error: %v", err)
+		}
+		if _, err := LoadDocument(); err != nil {
+			t.Fatalf("TestLoadPolicyDocument should not throw error for an existing policy file. Error: %v", err)
+		}
+	})
 
-	// existing policy file with bad permissions
-	if runtime.GOOS == "windows" {
-		t.Skip("skipping test on Windows")
-	}
-	tempRoot = t.TempDir()
-	dir.UserConfigDir = tempRoot
-	path = filepath.Join(tempRoot, "trustpolicy.json")
-	policyDoc2 := dummyPolicyDocument()
-	policyJson2, _ := json.Marshal(policyDoc2)
-	err = os.WriteFile(path, policyJson2, 0000)
-	if err != nil {
-		t.Fatalf("TestLoadPolicyDocument write policy file failed. Error: %v", err)
-	}
-	err = os.Chmod(path, 0000)
-	if err != nil {
-		t.Fatalf("TestLoadPolicyDocument create policy file with bad permissions failed. Error: %v", err)
-	}
-	_, err = LoadDocument()
-	if err == nil || err.Error() != fmt.Sprintf("unable to read trust policy due to file permissions, please verify the permissions of %s/trustpolicy.json", tempRoot) {
-		t.Fatalf("TestLoadPolicyDocument should throw error for a policy file with bad permissions. Error: %v", err)
-	}
+	t.Run("policy file with bad permissions", func(t *testing.T) {
+		if runtime.GOOS == "windows" {
+			t.Skip("skipping test on Windows")
+		}
+		tempRoot := t.TempDir()
+		dir.UserConfigDir = tempRoot
+		policyJson, _ := json.Marshal([]byte("Some String"))
+		path := filepath.Join(tempRoot, "trustpolicy.json")
+		if err := os.WriteFile(path, policyJson, 0000); err != nil {
+			t.Fatalf("TestLoadPolicyDocument write policy file failed. Error: %v", err)
+		}
+		_, err := LoadDocument()
+		if err == nil || err.Error() != fmt.Sprintf("unable to read trust policy due to file permissions, please verify the permissions of %s/trustpolicy.json", tempRoot) {
+			t.Fatalf("TestLoadPolicyDocument should throw error for a policy file with bad permissions. Error: %v", err)
+		}
+	})
+
+	t.Run("symlink policy file", func(t *testing.T) {
+		if runtime.GOOS == "windows" {
+			t.Skip("skipping test on Windows")
+		}
+		tempRoot := t.TempDir()
+		dir.UserConfigDir = tempRoot
+
+		os.Symlink("some/filepath", filepath.Join(tempRoot, "trustpolicy.json"))
+		_, err := LoadDocument()
+		if err == nil || !strings.HasPrefix(err.Error(), "trust policy is not a regular file (symlinks are not supported)") {
+			t.Fatalf("TestLoadPolicyDocument should throw error for a symlink policy file. Error: %v", err)
+		}
+	})
 }