chore(filewriter): cleanup writes

filewriter.go

@@ -1,19 +1,28 @@
 package files
 
 import (
+	"errors"
 	"fmt"
 	"io"
 	"os"
 	"path/filepath"
 )
 
+var ErrInvalidDirectoryEntry = errors.New("invalid directory entry name")
+var ErrPathExistsOverwrite = errors.New("path already exists and overwriting is not allowed")
+
 // WriteTo writes the given node to the local filesystem at fpath.
 func WriteTo(nd Node, fpath string) error {
+	if _, err := os.Lstat(fpath); err == nil {
+		return ErrPathExistsOverwrite
+	} else if !os.IsNotExist(err) {
+		return err
+	}
 	switch nd := nd.(type) {
 	case *Symlink:
 		return os.Symlink(nd.Target, fpath)
 	case File:
-		f, err := os.Create(fpath)
+		f, err := createNewFile(fpath)
 		defer f.Close()
 		if err != nil {
 			return err
@@ -31,7 +40,14 @@ func WriteTo(nd Node, fpath string) error {
 
 		entries := nd.Entries()
 		for entries.Next() {
-			child := filepath.Join(fpath, entries.Name())
+			entryName := entries.Name()
+			if entryName == "" ||
+				entryName == "." ||
+				entryName == ".." ||
+				!isValidFilename(entryName) {
+				return ErrInvalidDirectoryEntry
+			}
+			child := filepath.Join(fpath, entryName)
 			if err := WriteTo(entries.Node(), child); err != nil {
 				return err
 			}

filewriter_test.go

@@ -6,6 +6,8 @@ import (
 	"os"
 	"path/filepath"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestWriteTo(t *testing.T) {
@@ -75,3 +77,25 @@ func TestWriteTo(t *testing.T) {
 		t.Fatalf("failed to find: %#v", expected)
 	}
 }
+
+func TestDontAllowOverwrite(t *testing.T) {
+	tmppath, err := ioutil.TempDir("", "files-test")
+	assert.NoError(t, err)
+	defer os.RemoveAll(tmppath)
+
+	path := filepath.Join(tmppath, "output")
+
+	// Check we can actually write to the output path before trying invalid entries
+	// and leave an existing entry to test overwrite protection.
+	assert.NoError(t, WriteTo(NewMapDirectory(map[string]Node{
+		"exisiting-entry": NewBytesFile(nil),
+	}), path))
+
+	assert.Equal(t, ErrPathExistsOverwrite, WriteTo(NewBytesFile(nil), filepath.Join(path)))
+	assert.Equal(t, ErrPathExistsOverwrite, WriteTo(NewBytesFile(nil), filepath.Join(path, "exisiting-entry")))
+	// The directory in `path` has already been created so this should fail too:
+	assert.Equal(t, ErrPathExistsOverwrite, WriteTo(NewMapDirectory(map[string]Node{
+		"any-name": NewBytesFile(nil),
+	}), filepath.Join(path)))
+	os.RemoveAll(path)
+}

filewriter_unix.go

@@ -0,0 +1,20 @@
+//go:build darwin || linux || netbsd || openbsd
+// +build darwin linux netbsd openbsd
+
+package files
+
+import (
+	"os"
+	"strings"
+	"syscall"
+)
+
+var invalidChars = `/` + "\x00"
+
+func isValidFilename(filename string) bool {
+	return !strings.ContainsAny(filename, invalidChars)
+}
+
+func createNewFile(path string) (*os.File, error) {
+	return os.OpenFile(path, os.O_EXCL|os.O_CREATE|os.O_WRONLY|syscall.O_NOFOLLOW, 0666)
+}

filewriter_unix_test.go

@@ -0,0 +1,35 @@
+//go:build darwin || linux || netbsd || openbsd
+// +build darwin linux netbsd openbsd
+
+package files
+
+import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestWriteToInvalidPaths(t *testing.T) {
+	tmppath, err := ioutil.TempDir("", "files-test")
+	assert.NoError(t, err)
+	defer os.RemoveAll(tmppath)
+
+	path := filepath.Join(tmppath, "output")
+
+	// Check we can actually write to the output path before trying invalid entries.
+	assert.NoError(t, WriteTo(NewMapDirectory(map[string]Node{
+		"valid-entry": NewBytesFile(nil),
+	}), path))
+	os.RemoveAll(path)
+
+	// Now try all invalid entry names
+	for _, entryName := range []string{"", ".", "..", "/", "", "not/a/base/path"} {
+		assert.Equal(t, ErrInvalidDirectoryEntry, WriteTo(NewMapDirectory(map[string]Node{
+			entryName: NewBytesFile(nil),
+		}), filepath.Join(path)))
+		os.RemoveAll(path)
+	}
+}

filewriter_windows.go

@@ -0,0 +1,46 @@
+//go:build windows
+// +build windows
+
+package files
+
+import (
+	"os"
+	"strings"
+)
+
+var invalidChars = `<>:"/\|?*` + "\x00"
+
+var reservedNames = map[string]struct{}{
+	"CON":  {},
+	"PRN":  {},
+	"AUX":  {},
+	"NUL":  {},
+	"COM1": {},
+	"COM2": {},
+	"COM3": {},
+	"COM4": {},
+	"COM5": {},
+	"COM6": {},
+	"COM7": {},
+	"COM8": {},
+	"COM9": {},
+	"LPT1": {},
+	"LPT2": {},
+	"LPT3": {},
+	"LPT4": {},
+	"LPT5": {},
+	"LPT6": {},
+	"LPT7": {},
+	"LPT8": {},
+	"LPT9": {},
+}
+
+func isValidFilename(filename string) bool {
+	_, isReservedName := reservedNames[filename]
+	return !strings.ContainsAny(filename, invalidChars) &&
+		!isReservedName
+}
+
+func createNewFile(path string) (*os.File, error) {
+	return os.OpenFile(path, os.O_EXCL|os.O_CREATE|os.O_WRONLY, 0666)
+}

filewriter_windows_test.go

@@ -0,0 +1,37 @@
+//go:build windows
+// +build windows
+
+package files
+
+import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestWriteToInvalidPaths(t *testing.T) {
+	tmppath, err := ioutil.TempDir("", "files-test")
+	assert.NoError(t, err)
+	defer os.RemoveAll(tmppath)
+
+	path := filepath.Join(tmppath, "output")
+
+	// Check we can actually write to the output path before trying invalid entries.
+	assert.NoError(t, WriteTo(NewMapDirectory(map[string]Node{
+		"valid-entry": NewBytesFile(nil),
+	}), path))
+	os.RemoveAll(path)
+
+	// Now try all invalid entry names
+	for _, entryName := range []string{"", ".", "..", "/", "", "not/a/base/path",
+		"<", ">", ":", "\"", "\\", "|", "?", "*", "\x00",
+		"CON", "PRN", "AUX", "NUL", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", "COM9", "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9"} {
+		assert.Equal(t, ErrInvalidDirectoryEntry, WriteTo(NewMapDirectory(map[string]Node{
+			entryName: NewBytesFile(nil),
+		}), filepath.Join(path)))
+		os.RemoveAll(path)
+	}
+}

go.mod

@@ -2,7 +2,7 @@ module github.com/ipfs/go-ipfs-files
 
 require (
 	github.com/crackcomm/go-gitignore v0.0.0-20170627025303-887ab5e44cc3
-	github.com/stretchr/testify v1.7.0 // indirect
+	github.com/stretchr/testify v1.7.0
 	golang.org/x/sys v0.0.0-20190302025703-b6889370fb10
 )
 

go.sum

@@ -9,6 +9,7 @@ github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5Cc
 github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 golang.org/x/sys v0.0.0-20190302025703-b6889370fb10 h1:xQJI9OEiErEQ++DoXOHqEpzsGMrAv2Q2jyCpi7DmfpQ=
 golang.org/x/sys v0.0.0-20190302025703-b6889370fb10/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
+gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo=
 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=