Skip to content
This repository has been archived by the owner on Mar 29, 2023. It is now read-only.

Commit

Permalink
fix: error when TAR has files outside of root (#56)
Browse files Browse the repository at this point in the history
See "Security" section of IPIP-288 (ipfs/specs#288)
  • Loading branch information
hacdias authored Nov 9, 2022
1 parent 30b08ca commit e8cf9a3
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 1 deletion.
39 changes: 38 additions & 1 deletion tarwriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
64 changes: 64 additions & 0 deletions tarwriter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package files

import (
"archive/tar"
"errors"
"io"
"testing"
"time"
Expand Down Expand Up @@ -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)
}
}

0 comments on commit e8cf9a3

Please sign in to comment.