CVE-2015-10024: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: A vulnerability classified as critical was found in hoffie larasync. This vulnerability affects unknown code of the file repository/content/file_storage.go. The manipulation leads to path traversal. The name of the patch is 776bad422f4bd4930d09491711246bbeb1be9ba5. It is recommended to apply a patch to fix this issue. The identifier of this vulnerability is VDB-217612.
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
CVSS v3 base score: 9.8
Published: 2023-01-07T13:15Z
Repository: https://github.com/hoffie/larasync
Language: Go
Files changed: repository/content/file_storage.go, repository/content/file_storage_test.go
Commit date: 2015-01-20 23:21:00 +0100
Commit message: repository/content: fix potential (authenticated) path traversal (#195)

Diff: diff --git a/repository/content/file_storage.go b/repository/content/file_storage.go
index 99344dc..dfead0f 100644
--- a/repository/content/file_storage.go
+++ b/repository/content/file_storage.go
@@ -1,21 +1,28 @@
 package content
 
 import (
+	"errors"
 	"io"
 	"os"
 	"path"
+	"path/filepath"
+	"strings"
 
 	"github.com/hoffie/larasync/helpers/atomic"
 )
 
 const (
 	// default permissions
 	defaultFilePerms = 0600
 	defaultDirPerms  = 0700
 )
 
+// ErrInvalidPath is returned if storage at a path not rooted at the FileStorage's
+// root path is attempted.
+var ErrInvalidPath = errors.New("invalid path")
+
 // FileStorage is the basic implementation of the Storage
 // implementation which stores the data into the file system.
 type FileStorage struct {
 	path string
 }
@@ -37,27 +44,44 @@ func (f *FileStorage) CreateDir() error {
 	}
 	return nil
 }
 
 // storagePathFor returns the storage path for the data entry.
-func (f *FileStorage) storagePathFor(contentID string) string {
-	return path.Join(f.path, contentID)
+func (f *FileStorage) storagePathFor(contentID string) (string, error) {
+	p := path.Join(f.path, contentID)
+	p = filepath.Clean(p)
+	root := f.path
+	/*if len(root) > 1 && root[len(root)-1] != filepath.Separator {
+		root += filepath.Separator
+	}*/
+	if !strings.HasPrefix(p, root) {
+		return "", ErrInvalidPath
+	}
+	return p, nil
 }
 
 // Get returns the file handle for the given contentID.
 // If there is no data stored for the Id it should return a
 // os.ErrNotExists error.
 func (f *FileStorage) Get(contentID string) (io.ReadCloser, error) {
-	if f.Exists(contentID) {
-		return os.Open(f.storagePathFor(contentID))
+	if !f.Exists(contentID) {
+		return nil, os.ErrNotExist
+	}
+	// FIXME TOCTU race
+	p, err := f.storagePathFor(contentID)
+	if err != nil {
+		return nil, err
 	}
-	return nil, os.ErrNotExist
+	return os.Open(p)
 }
 
 // Set sets the data of the given contentID in the blob storage.
 func (f *FileStorage) Set(contentID string, reader io.Reader) error {
-	blobStoragePath := f.storagePathFor(contentID)
+	blobStoragePath, err := f.storagePathFor(contentID)
+	if err != nil {
+		return err
+	}
 
 	writer, err := atomic.NewStandardWriter(blobStoragePath, defaultFilePerms)
 	if err != nil {
 		return err
 	}
@@ -77,16 +101,25 @@ func (f *FileStorage) Set(contentID string, reader io.Reader) error {
 	return nil
 }
 
 // Exists checks if the given entry is stored in the database.
 func (f *FileStorage) Exists(contentID string) bool {
-	_, err := os.Stat(f.storagePathFor(contentID))
+	p, err := f.storagePathFor(contentID)
+	if err != nil {
+		// FIXME maybe return error instead?
+		return false
+	}
+	_, err = os.Stat(p)
 	if err != nil {
 		return !os.IsNotExist(err)
 	}
 	return true
 }
 
 // Delete removes the data with the given contentID from the store.
 func (f *FileStorage) Delete(contentID string) error {
-	return os.Remove(f.storagePathFor(contentID))
+	p, err := f.storagePathFor(contentID)
+	if err != nil {
+		return err
+	}
+	return os.Remove(p)
 }
diff --git a/repository/content/file_storage_test.go b/repository/content/file_storage_test.go
index 99a880d..fead6e9 100644
--- a/repository/content/file_storage_test.go
+++ b/repository/content/file_storage_test.go
@@ -18,10 +18,12 @@ type FileStorageTests struct {
 	data    []byte
 }
 
 var _ = Suite(&FileStorageTests{})
 
+var dangerousNames = []string{"..", "../xyz"}
+
 func (t *FileStorageTests) SetUpTest(c *C) {
 	t.dir = c.MkDir()
 	t.storage = NewFileStorage(t.dir)
 	t.data = []byte("This is a test blob storage file input.")
 }
@@ -41,10 +43,18 @@ func (t *FileStorageTests) testReader() io.Reader {
 
 func (t *FileStorageTests) setData() error {
 	return t.storage.Set(t.blobID(), t.testReader())
 }
 
+func (t *FileStorageTests) TestSetDangerousName(c *C) {
+	r := t.testReader()
+	for _, id := range dangerousNames {
+		err := t.storage.Set(id, r)
+		c.Assert(err, NotNil)
+	}
+}
+
 func (t *FileStorageTests) TestSet(c *C) {
 	err := t.setData()
 	c.Assert(err, IsNil)
 	_, err = os.Stat(t.blobPath())
 	c.Assert(err, IsNil)
@@ -70,10 +80,17 @@ func (t *FileStorageTests) TestGet(c *C) {
 	t.storage.Set(t.blobID(), t.testReader())
 	_, err := t.storage.Get(t.blobID())
 	c.Assert(err, IsNil)
 }
 
+func (t *FileStorageTests) TestGetDangerousName(c *C) {
+	for _, id := range dangerousNames {
+		_, err := t.storage.Get(id)
+		c.Assert(err, NotNil)
+	}
+}
+
 func (t *FileStorageTests) TestGetData(c *C) {
 	t.setData()
 	file, _ := t.storage.Get(t.blobID())
 	fileData, _ := ioutil.ReadAll(file)
 	c.Assert(fileData[:], DeepEquals, t.data)

Vulnerable code: func (f *FileStorage) storagePathFor(contentID string) string {
	return path.Join(f.path, contentID)
	if f.Exists(contentID) {
		return os.Open(f.storagePathFor(contentID))
	return nil, os.ErrNotExist
	blobStoragePath := f.storagePathFor(contentID)
	_, err := os.Stat(f.storagePathFor(contentID))
	return os.Remove(f.storagePathFor(contentID))
Fixed code: 	"errors"
	"path/filepath"
	"strings"
// ErrInvalidPath is returned if storage at a path not rooted at the FileStorage's
// root path is attempted.
var ErrInvalidPath = errors.New("invalid path")

func (f *FileStorage) storagePathFor(contentID string) (string, error) {
	p := path.Join(f.path, contentID)
	p = filepath.Clean(p)
	root := f.path
	/*if len(root) > 1 && root[len(root)-1] != filepath.Separator {
		root += filepath.Separator
	}*/
	if !strings.HasPrefix(p, root) {
		return "", ErrInvalidPath
	}
	return p, nil
	if !f.Exists(contentID) {
		return nil, os.ErrNotExist
	}
	// FIXME TOCTU race
	p, err := f.storagePathFor(contentID)
	if err != nil {
		return nil, err
	return os.Open(p)
	blobStoragePath, err := f.storagePathFor(contentID)
	if err != nil {
		return err
	}
	p, err := f.storagePathFor(contentID)
	if err != nil {
		// FIXME maybe return error instead?
		return false
	}
	_, err = os.Stat(p)
	p, err := f.storagePathFor(contentID)
	if err != nil {
		return err
	}
	return os.Remove(p)
var dangerousNames = []string{"..", "../xyz"}

func (t *FileStorageTests) TestSetDangerousName(c *C) {
	r := t.testReader()
	for _, id := range dangerousNames {
		err := t.storage.Set(id, r)
		c.Assert(err, NotNil)
	}
}

func (t *FileStorageTests) TestGetDangerousName(c *C) {
	for _, id := range dangerousNames {
		_, err := t.storage.Get(id)
		c.Assert(err, NotNil)
	}
}