diff --git a/fetcher.go b/fetcher.go index 50b2726..b657113 100644 --- a/fetcher.go +++ b/fetcher.go @@ -5,7 +5,6 @@ import ( "fmt" "io" "net/http" - "os" "path/filepath" "strings" "sync" @@ -62,7 +61,7 @@ func (f *HTTPFetcher) FetchBytes(ctx context.Context, gav GAV) ([]byte, error) { if resp.StatusCode != http.StatusOK { return nil, fmt.Errorf("http %d for %s", resp.StatusCode, req.URL) } - return io.ReadAll(resp.Body) + return io.ReadAll(io.LimitReader(resp.Body, MaxPOMBytes+1)) } // POMURL builds the repository URL for gav's POM under base. @@ -81,12 +80,7 @@ type DirFetcher struct { } func (f *DirFetcher) Fetch(_ context.Context, gav GAV) (*POM, error) { - path := filepath.Join(f.Dir, FixtureName(gav)) - data, err := os.ReadFile(path) - if err != nil { - return nil, err - } - return ParsePOM(data) + return readPOMFile(filepath.Join(f.Dir, FixtureName(gav))) } // FixtureName returns the on-disk filename DirFetcher expects for gav. diff --git a/local.go b/local.go index d0bf6e8..6b9d2d2 100644 --- a/local.go +++ b/local.go @@ -3,23 +3,32 @@ package pom import ( "context" "fmt" + "io" "os" "path/filepath" + "strings" ) // LocalFetcher resolves parent POMs from the filesystem by following // (defaulting to ../pom.xml) from a root file. It // never touches the network; any GAV not found on disk returns an error, // which the resolver records as a warning and tags as unresolved_parent. +// +// The walk is jailed to the directory passed at construction time. An empty +// jail root disables the walk entirely so the fetcher only knows about the +// already-parsed root POM. This is the safe choice when the POM bytes came +// from an untrusted source and there is no on-disk checkout to consult. type LocalFetcher struct { + root string index map[GAV]*POM } // ResolveLocal is a convenience for the common case of resolving a pom.xml // found in a source checkout: it reads path, walks the relativePath chain -// on disk, and computes the effective POM with no network access. -func ResolveLocal(ctx context.Context, path string, opts Options) (*EffectivePOM, error) { - f, root, err := NewLocalFetcher(path) +// on disk within fsRoot, and computes the effective POM with no network +// access. +func ResolveLocal(ctx context.Context, path, fsRoot string, opts Options) (*EffectivePOM, error) { + f, root, err := NewLocalFetcher(path, fsRoot) if err != nil { return nil, err } @@ -27,25 +36,46 @@ func ResolveLocal(ctx context.Context, path string, opts Options) (*EffectivePOM } // NewLocalFetcher reads the POM at path and every reachable parent via -// , indexing each by GAV. It returns the fetcher and the -// parsed root so callers can pass it straight to Resolver.ResolvePOM. -func NewLocalFetcher(path string) (*LocalFetcher, *POM, error) { +// within fsRoot, indexing each by GAV. It returns the +// fetcher and the parsed root so callers can pass it straight to +// Resolver.ResolvePOM. +func NewLocalFetcher(path, fsRoot string) (*LocalFetcher, *POM, error) { root, err := readPOMFile(path) if err != nil { return nil, nil, err } - return NewLocalFetcherFrom(root, filepath.Dir(path)), root, nil + return NewLocalFetcherFrom(root, filepath.Dir(path), fsRoot), root, nil } // NewLocalFetcherFrom builds a LocalFetcher around an already-parsed root -// POM whose file lived in dir. Use this when the caller has the bytes in -// hand and wants to avoid re-reading the root. -func NewLocalFetcherFrom(root *POM, dir string) *LocalFetcher { +// POM whose file lived in dir. The relativePath walk is confined to fsRoot; +// pass an empty fsRoot to skip the walk entirely. +func NewLocalFetcherFrom(root *POM, dir, fsRoot string) *LocalFetcher { f := &LocalFetcher{index: map[GAV]*POM{root.EffectiveGAV(): root}} - f.walk(root, dir) + if fsRoot != "" { + if abs, err := filepath.Abs(fsRoot); err == nil { + f.root = filepath.Clean(abs) + f.walk(root, dir) + } + } return f } +func (f *LocalFetcher) within(path string) bool { + if f.root == "" { + return false + } + abs, err := filepath.Abs(path) + if err != nil { + return false + } + abs = filepath.Clean(abs) + if abs == f.root { + return true + } + return strings.HasPrefix(abs, f.root+string(filepath.Separator)) +} + func (f *LocalFetcher) walk(p *POM, dir string) { for range maxParentDepth { if p.Parent == nil { @@ -56,6 +86,9 @@ func (f *LocalFetcher) walk(p *POM, dir string) { return } path := filepath.Clean(filepath.Join(dir, rel)) + if !f.within(path) { + return + } fi, err := os.Lstat(path) if err != nil { return @@ -65,6 +98,9 @@ func (f *LocalFetcher) walk(p *POM, dir string) { } if fi.IsDir() { path = filepath.Join(path, "pom.xml") + if !f.within(path) { + return + } fi, err = os.Lstat(path) if err != nil || fi.Mode()&os.ModeSymlink != 0 { return @@ -96,7 +132,12 @@ func (f *LocalFetcher) Fetch(_ context.Context, gav GAV) (*POM, error) { } func readPOMFile(path string) (*POM, error) { - data, err := os.ReadFile(path) + fh, err := os.Open(path) + if err != nil { + return nil, err + } + defer func() { _ = fh.Close() }() + data, err := io.ReadAll(io.LimitReader(fh, MaxPOMBytes+1)) if err != nil { return nil, err } diff --git a/local_test.go b/local_test.go index 06b0f10..c4c071d 100644 --- a/local_test.go +++ b/local_test.go @@ -1,14 +1,16 @@ package pom import ( + "bytes" "context" + "errors" "os" "path/filepath" "testing" ) func TestResolveLocal(t *testing.T) { - ep, err := ResolveLocal(context.Background(), "testdata/local/child/pom.xml", Options{}) + ep, err := ResolveLocal(context.Background(), "testdata/local/child/pom.xml", "testdata/local", Options{}) if err != nil { t.Fatalf("ResolveLocal: %v", err) } @@ -42,7 +44,7 @@ func TestResolveLocal(t *testing.T) { } func TestResolveLocalEmptyRelativePath(t *testing.T) { - ep, err := ResolveLocal(context.Background(), "testdata/local/nested/pom.xml", Options{}) + ep, err := ResolveLocal(context.Background(), "testdata/local/nested/pom.xml", "testdata/local", Options{}) if err != nil { t.Fatalf("ResolveLocal: %v", err) } @@ -59,7 +61,7 @@ func TestResolveLocalEmptyRelativePath(t *testing.T) { } func TestResolveLocalMissingFile(t *testing.T) { - if _, err := ResolveLocal(context.Background(), "testdata/local/nope/pom.xml", Options{}); err == nil { + if _, err := ResolveLocal(context.Background(), "testdata/local/nope/pom.xml", "testdata/local", Options{}); err == nil { t.Error("expected error for missing root file") } } @@ -100,7 +102,7 @@ func TestLocalFetcherRejectsAbsoluteRelativePath(t *testing.T) { Parent: &Parent{GroupID: "org.evil", ArtifactID: "evil", Version: "1.0", RelativePath: &absPath}, } - f := NewLocalFetcherFrom(child, childDir) + f := NewLocalFetcherFrom(child, childDir, tmp) _, err := f.Fetch(context.Background(), GAV{"org.evil", "evil", "1.0"}) if err == nil { t.Error("expected error: absolute relativePath should be rejected") @@ -131,9 +133,97 @@ func TestLocalFetcherRejectsSymlink(t *testing.T) { Parent: &Parent{GroupID: "org.evil", ArtifactID: "evil", Version: "1.0", RelativePath: &rel}, } - f := NewLocalFetcherFrom(child, childDir) + f := NewLocalFetcherFrom(child, childDir, tmp) _, err := f.Fetch(context.Background(), GAV{"org.evil", "evil", "1.0"}) if err == nil { t.Error("expected error: symlink traversal should be rejected") } } + +func TestLocalFetcherRejectsRootEscape(t *testing.T) { + tmp := t.TempDir() + repo := filepath.Join(tmp, "repo") + childDir := filepath.Join(repo, "child") + _ = os.MkdirAll(childDir, 0o755) + + outside := filepath.Join(tmp, "outside", "pom.xml") + _ = os.MkdirAll(filepath.Dir(outside), 0o755) + _ = os.WriteFile(outside, []byte(`org.evilevil1.0`), 0o644) + + rel := "../../outside/pom.xml" + child := &POM{ + GroupID: "org.example", + ArtifactID: "child", + Version: "1.0", + Parent: &Parent{GroupID: "org.evil", ArtifactID: "evil", Version: "1.0", RelativePath: &rel}, + } + + f := NewLocalFetcherFrom(child, childDir, repo) + if _, err := f.Fetch(context.Background(), GAV{"org.evil", "evil", "1.0"}); err == nil { + t.Error("expected error: relativePath escaping fsRoot should be rejected") + } +} + +func TestLocalFetcherRejectsRootEscapePrefixSibling(t *testing.T) { + // Guard against the classic strings.HasPrefix bug where /tmp/repo-evil + // is treated as inside /tmp/repo because one is a string prefix of the + // other. + tmp := t.TempDir() + repo := filepath.Join(tmp, "repo") + sibling := filepath.Join(tmp, "repo-evil") + childDir := filepath.Join(repo, "child") + _ = os.MkdirAll(childDir, 0o755) + _ = os.MkdirAll(sibling, 0o755) + _ = os.WriteFile(filepath.Join(sibling, "pom.xml"), []byte(`org.evilevil1.0`), 0o644) + + rel := "../../repo-evil/pom.xml" + child := &POM{ + GroupID: "org.example", + ArtifactID: "child", + Version: "1.0", + Parent: &Parent{GroupID: "org.evil", ArtifactID: "evil", Version: "1.0", RelativePath: &rel}, + } + + f := NewLocalFetcherFrom(child, childDir, repo) + if _, err := f.Fetch(context.Background(), GAV{"org.evil", "evil", "1.0"}); err == nil { + t.Error("expected error: sibling directory with shared prefix should be rejected") + } +} + +func TestLocalFetcherEmptyRootSkipsWalk(t *testing.T) { + ep, err := ResolveLocal(context.Background(), "testdata/local/child/pom.xml", "", Options{}) + if err != nil { + t.Fatalf("ResolveLocal: %v", err) + } + if len(ep.Parents) != 0 { + t.Errorf("empty fsRoot should disable parent walk, got parents: %v", ep.Parents) + } + if len(ep.Warnings) == 0 { + t.Error("expected unresolved-parent warning when walk is disabled") + } +} + +func TestParsePOMRejectsOversize(t *testing.T) { + old := MaxPOMBytes + MaxPOMBytes = 1024 + t.Cleanup(func() { MaxPOMBytes = old }) + + data := append([]byte(""), bytes.Repeat([]byte("x"), 2000)...) + if _, err := ParsePOM(data); !errors.Is(err, ErrPOMTooLarge) { + t.Errorf("expected ErrPOMTooLarge, got %v", err) + } +} + +func TestReadPOMFileRejectsOversize(t *testing.T) { + old := MaxPOMBytes + MaxPOMBytes = 1024 + t.Cleanup(func() { MaxPOMBytes = old }) + + tmp := t.TempDir() + path := filepath.Join(tmp, "pom.xml") + _ = os.WriteFile(path, bytes.Repeat([]byte("x"), 2000), 0o644) + + if _, err := readPOMFile(path); !errors.Is(err, ErrPOMTooLarge) { + t.Errorf("expected ErrPOMTooLarge from readPOMFile, got %v", err) + } +} diff --git a/pom.go b/pom.go index 87e7b7d..06c6b77 100644 --- a/pom.go +++ b/pom.go @@ -10,6 +10,7 @@ package pom import ( "bytes" "encoding/xml" + "errors" "fmt" "io" "strings" @@ -244,10 +245,22 @@ func (p *Properties) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error return nil } +// MaxPOMBytes is the largest POM ParsePOM will accept and the read limit +// applied by the bundled fetchers. The largest POM on Maven Central is well +// under 1 MB; 10 MB leaves headroom for generated dependency lists while +// keeping a single hostile file from exhausting memory. +var MaxPOMBytes int64 = 10 << 20 + +// ErrPOMTooLarge is returned by ParsePOM when the input exceeds MaxPOMBytes. +var ErrPOMTooLarge = errors.New("pom: input exceeds MaxPOMBytes") + // ParsePOM decodes a POM from XML bytes. It is lenient about charset // declarations and strict-mode failures that would otherwise reject // real-world POMs published to Maven Central. func ParsePOM(data []byte) (*POM, error) { + if int64(len(data)) > MaxPOMBytes { + return nil, ErrPOMTooLarge + } dec := xml.NewDecoder(bytes.NewReader(data)) dec.Strict = false dec.CharsetReader = func(charset string, input io.Reader) (io.Reader, error) {