fix: error when TAR has files outside of root (#56)

See "Security" section of IPIP-288 (ipfs/specs#288)

tarwriter.go

@@ -2,14 +2,22 @@ package files
 
 import (
 	"archive/tar"
+	"errors"
 	"fmt"
 	"io"
 	"path"
+	"strings"
 	"time"
 )
 
+var (
+	ErrUnixFSPathOutsideRoot = errors.New("relative UnixFS paths outside the root are now allowed, use CAR instead")
+)
+
 type TarWriter struct {
-	TarW *tar.Writer
+	TarW       *tar.Writer
+	baseDirSet bool
+	baseDir    string
 }
 
 // NewTarWriter wraps given io.Writer into a new tar writer
@@ -50,8 +58,37 @@ func (w *TarWriter) writeFile(f File, fpath string) error {
 	return nil
 }
 
+func validateTarFilePath(baseDir, fpath string) bool {
+	// Ensure the filepath has no ".", "..", etc within the known root directory.
+	fpath = path.Clean(fpath)
+
+	// If we have a non-empty baseDir, check if the filepath starts with baseDir.
+	// If not, we can exclude it immediately. For 'ipfs get' and for the gateway,
+	// the baseDir would be '{cid}.tar'.
+	if baseDir != "" && !strings.HasPrefix(path.Clean(fpath), baseDir) {
+		return false
+	}
+
+	// Otherwise, check if the path starts with '..' which would make it fall
+	// outside the root path. This works since the path has already been cleaned.
+	if strings.HasPrefix(fpath, "..") {
+		return false
+	}
+
+	return true
+}
+
 // WriteNode adds a node to the archive.
 func (w *TarWriter) WriteFile(nd Node, fpath string) error {
+	if !w.baseDirSet {
+		w.baseDirSet = true // Use a variable for this as baseDir may be an empty string.
+		w.baseDir = fpath
+	}
+
+	if !validateTarFilePath(w.baseDir, fpath) {
+		return ErrUnixFSPathOutsideRoot
+	}
+
 	switch nd := nd.(type) {
 	case *Symlink:
 		return writeSymlinkHeader(w.TarW, nd.Target, fpath)

tarwriter_test.go

@@ -2,6 +2,7 @@ package files
 
 import (
 	"archive/tar"
+	"errors"
 	"io"
 	"testing"
 	"time"
@@ -83,3 +84,66 @@ func TestTarWriter(t *testing.T) {
 		t.Fatal(err)
 	}
 }
+
+func TestTarWriterRelativePathInsideRoot(t *testing.T) {
+	tf := NewMapDirectory(map[string]Node{
+		"file.txt": NewBytesFile([]byte(text)),
+		"boop": NewMapDirectory(map[string]Node{
+			"../a.txt": NewBytesFile([]byte("bleep")),
+			"b.txt":    NewBytesFile([]byte("bloop")),
+		}),
+		"beep.txt": NewBytesFile([]byte("beep")),
+	})
+
+	tw, err := NewTarWriter(io.Discard)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	defer tw.Close()
+	if err := tw.WriteFile(tf, ""); err != nil {
+		t.Error(err)
+	}
+}
+
+func TestTarWriterFailsFileOutsideRoot(t *testing.T) {
+	tf := NewMapDirectory(map[string]Node{
+		"file.txt": NewBytesFile([]byte(text)),
+		"boop": NewMapDirectory(map[string]Node{
+			"../../a.txt": NewBytesFile([]byte("bleep")),
+			"b.txt":       NewBytesFile([]byte("bloop")),
+		}),
+		"beep.txt": NewBytesFile([]byte("beep")),
+	})
+
+	tw, err := NewTarWriter(io.Discard)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	defer tw.Close()
+	if err := tw.WriteFile(tf, ""); !errors.Is(err, ErrUnixFSPathOutsideRoot) {
+		t.Errorf("unexpected error, wanted: %v; got: %v", ErrUnixFSPathOutsideRoot, err)
+	}
+}
+
+func TestTarWriterFailsFileOutsideRootWithBaseDir(t *testing.T) {
+	tf := NewMapDirectory(map[string]Node{
+		"../file.txt": NewBytesFile([]byte(text)),
+		"boop": NewMapDirectory(map[string]Node{
+			"a.txt": NewBytesFile([]byte("bleep")),
+			"b.txt": NewBytesFile([]byte("bloop")),
+		}),
+		"beep.txt": NewBytesFile([]byte("beep")),
+	})
+
+	tw, err := NewTarWriter(io.Discard)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	defer tw.Close()
+	if err := tw.WriteFile(tf, "test.tar"); !errors.Is(err, ErrUnixFSPathOutsideRoot) {
+		t.Errorf("unexpected error, wanted: %v; got: %v", ErrUnixFSPathOutsideRoot, err)
+	}
+}