Harden mapping from request path to FS path

Put strict checks in place to avoid authenticated users accessing files
outside of their actual storage directory. These checks will need
updating if multiple address books are to be supported.

storage/filesystem.go

@@ -9,6 +9,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"regexp"
 
 	"github.com/emersion/go-vcard"
 	"github.com/emersion/go-webdav/carddav"
@@ -38,7 +39,7 @@ func NewFilesystem(path string) (carddav.Backend, error) {
 func (b *filesystemBackend) pathForContext(ctx context.Context) (string, error) {
 	raw := ctx.Value(auth.AuthCtxKey)
 	if raw == nil {
-		return b.path, nil
+		return "", fmt.Errorf("unauthenticated requests are not supported")
 	}
 	authCtx, ok := raw.(*auth.AuthContext)
 	if !ok {
@@ -57,6 +58,20 @@ func (b *filesystemBackend) pathForContext(ctx context.Context) (string, error)
 	return path, nil
 }
 
+func (b *filesystemBackend) safePath(ctx context.Context, path string) (string, error) {
+	basePath, err := b.pathForContext(ctx)
+	if err != nil {
+		return "", err
+	}
+	// We are mapping to local filesystem path, so be conservative about what to accept
+	// TODO this changes once multiple addess books are supported
+	var valid = regexp.MustCompile(`^/[A-Za-z0-9_-]+(.[a-zA-Z]+)?$`)
+	if !valid.MatchString(path) {
+		return "", fmt.Errorf("invalid request path")
+	}
+	return filepath.Join(basePath, path), nil
+}
+
 func etagForFile(path string) (string, error) {
 	data, err := ioutil.ReadFile(path)
 	if err != nil {
@@ -153,13 +168,12 @@ func (b *filesystemBackend) AddressBook(ctx context.Context) (*carddav.AddressBo
 }
 
 func (b *filesystemBackend) GetAddressObject(ctx context.Context, path string, req *carddav.AddressDataRequest) (*carddav.AddressObject, error) {
-	basePath, err := b.pathForContext(ctx)
+	localPath, err := b.safePath(ctx, path)
 	if err != nil {
 		return nil, err
 	}
-	path = filepath.Join(basePath, path)
 
-	info, err := os.Stat(path)
+	info, err := os.Stat(localPath)
 	if err != nil {
 		return nil, err
 	}
@@ -169,18 +183,18 @@ func (b *filesystemBackend) GetAddressObject(ctx context.Context, path string, r
 		propFilter = req.Props
 	}
 
-	card, err := vcardFromFile(path, propFilter)
+	card, err := vcardFromFile(localPath, propFilter)
 	if err != nil {
 		return nil, err
 	}
 
-	etag, err := etagForFile(path)
+	etag, err := etagForFile(localPath)
 	if err != nil {
 		return nil, err
 	}
 
 	obj := carddav.AddressObject{
-		Path: "/" + filepath.Base(path),
+		Path: path,
 		ModTime: info.ModTime(),
 		ETag: etag,
 		Card: *card,
@@ -249,12 +263,11 @@ func (b *filesystemBackend) QueryAddressObjects(ctx context.Context, query *card
 }
 
 func (b *filesystemBackend) PutAddressObject(ctx context.Context, path string, card vcard.Card) (loc string, err error) {
-	basePath, err := b.pathForContext(ctx)
+	localPath, err := b.safePath(ctx, path)
 	if err != nil {
 		return "", err
 	}
-	internalPath := filepath.Join(basePath, path)
+	f, err := os.OpenFile(localPath, os.O_RDWR|os.O_CREATE, 0644)
-	f, err := os.OpenFile(internalPath, os.O_RDWR|os.O_CREATE, 0644)
 	if err != nil {
 		return "", err
 	}
@@ -270,13 +283,11 @@ func (b *filesystemBackend) PutAddressObject(ctx context.Context, path string, c
 }
 
 func (b *filesystemBackend) DeleteAddressObject(ctx context.Context, path string) error {
-	basePath, err := b.pathForContext(ctx)
+	localPath, err := b.safePath(ctx, path)
 	if err != nil {
 		return err
 	}
-	path = filepath.Join(basePath, path)
+	err = os.Remove(localPath)
-	//TODO does this need more security/sanity checks?
-	err = os.Remove(path)
 	if err != nil {
 		return err
 	}