diff --git a/allowedpaths/sandbox.go b/allowedpaths/sandbox.go index 95b2315d..03e869c1 100644 --- a/allowedpaths/sandbox.go +++ b/allowedpaths/sandbox.go @@ -18,6 +18,11 @@ import ( "strings" ) +// MaxGlobEntries is the maximum number of directory entries read per single +// glob expansion step. ReadDirForGlob returns an error for directories that +// exceed this limit to prevent memory exhaustion during pattern matching. +const MaxGlobEntries = 100_000 + // root pairs an absolute directory path with its opened os.Root handle. type root struct { absPath string @@ -182,6 +187,24 @@ func (s *Sandbox) Open(path string, cwd string, flag int, perm os.FileMode) (io. // ReadDir implements the restricted directory-read policy. func (s *Sandbox) ReadDir(path string, cwd string) ([]fs.DirEntry, error) { + return s.readDirN(path, cwd, -1) +} + +// ReadDirForGlob reads directory entries for glob expansion, capped at +// MaxGlobEntries. The underlying ReadDir call is limited to MaxGlobEntries+1 +// so the kernel never materialises more entries than needed. If the directory +// exceeds the limit an error is returned before any pattern matching or +// sorting can occur, making the failure explicit rather than silently returning +// a partial listing that could miss valid matches. +func (s *Sandbox) ReadDirForGlob(path string, cwd string) ([]fs.DirEntry, error) { + return s.readDirN(path, cwd, MaxGlobEntries) +} + +// readDirN is the shared implementation for ReadDir and ReadDirForGlob. +// maxEntries <= 0 means unlimited. Otherwise f.ReadDir is called with +// maxEntries+1 to cap the read at the OS level; if the directory has more +// entries than the limit an error is returned. +func (s *Sandbox) readDirN(path string, cwd string, maxEntries int) ([]fs.DirEntry, error) { absPath := toAbs(path, cwd) root, relPath, ok := s.resolve(absPath) @@ -194,10 +217,23 @@ func (s *Sandbox) ReadDir(path string, cwd string) ([]fs.DirEntry, error) { return nil, PortablePathError(err) } defer f.Close() - entries, err := f.ReadDir(-1) - if err != nil { + + var entries []fs.DirEntry + if maxEntries <= 0 { + entries, err = f.ReadDir(-1) + } else { + entries, err = f.ReadDir(maxEntries + 1) + } + if err != nil && err != io.EOF { return nil, PortablePathError(err) } + if maxEntries > 0 && len(entries) > maxEntries { + return nil, &os.PathError{ + Op: "readdir", + Path: path, + Err: fmt.Errorf("directory has too many entries (cap: %d)", maxEntries), + } + } // os.Root's ReadDir does not guarantee sorted order like os.ReadDir. // Sort to match POSIX glob expansion expectations. slices.SortFunc(entries, func(a, b fs.DirEntry) int { diff --git a/allowedpaths/sandbox_test.go b/allowedpaths/sandbox_test.go index f9c55bd3..e36ccc14 100644 --- a/allowedpaths/sandbox_test.go +++ b/allowedpaths/sandbox_test.go @@ -185,6 +185,38 @@ func TestReadDirLimited(t *testing.T) { }) } +func TestReadDirNCapExceeded(t *testing.T) { + dir := t.TempDir() + + // Create 4 files so the directory exceeds a cap of 3. + for i := range 4 { + require.NoError(t, os.WriteFile(filepath.Join(dir, fmt.Sprintf("f%02d", i)), nil, 0644)) + } + + sb, err := New([]string{dir}) + require.NoError(t, err) + defer sb.Close() + + t.Run("returns error when entries exceed cap", func(t *testing.T) { + entries, err := sb.readDirN(".", dir, 3) + assert.Nil(t, entries) + assert.Error(t, err) + assert.Contains(t, err.Error(), "too many entries") + }) + + t.Run("returns entries when count equals cap", func(t *testing.T) { + entries, err := sb.readDirN(".", dir, 4) + require.NoError(t, err) + assert.Len(t, entries, 4) + }) + + t.Run("returns entries when count is below cap", func(t *testing.T) { + entries, err := sb.readDirN(".", dir, 10) + require.NoError(t, err) + assert.Len(t, entries, 4) + }) +} + func TestCollectDirEntries(t *testing.T) { makeEntries := func(names ...string) []fs.DirEntry { out := make([]fs.DirEntry, len(names)) diff --git a/builtins/tests/testcmd/testcmd_fuzz_test.go b/builtins/tests/testcmd/testcmd_fuzz_test.go index d7b4b873..9b902a62 100644 --- a/builtins/tests/testcmd/testcmd_fuzz_test.go +++ b/builtins/tests/testcmd/testcmd_fuzz_test.go @@ -79,7 +79,7 @@ func FuzzTestStringOps(f *testing.F) { dir := t.TempDir() - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) defer cancel() script := fmt.Sprintf("test '%s' %s '%s'", left, op, right) @@ -125,7 +125,7 @@ func FuzzTestIntegerOps(f *testing.F) { dir := t.TempDir() - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) defer cancel() script := fmt.Sprintf("test %d %s %d", left, op, right) @@ -167,7 +167,7 @@ func FuzzTestFileOps(f *testing.F) { } } - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) defer cancel() script := fmt.Sprintf("test %s %s", op, target) @@ -219,7 +219,7 @@ func FuzzTestStringUnary(f *testing.F) { dir := t.TempDir() - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) defer cancel() script := fmt.Sprintf("test %s '%s'", op, arg) @@ -288,7 +288,7 @@ func FuzzTestNesting(f *testing.F) { dir := t.TempDir() - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) defer cancel() script := fmt.Sprintf("test %s", expr) diff --git a/interp/api.go b/interp/api.go index d58f554d..b257a219 100644 --- a/interp/api.go +++ b/interp/api.go @@ -295,7 +295,7 @@ func (r *Runner) Reset() { return r.sandbox.Open(path, HandlerCtx(ctx).Dir, flag, perm) } r.readDirHandler = func(ctx context.Context, path string) ([]os.DirEntry, error) { - return r.sandbox.ReadDir(path, HandlerCtx(ctx).Dir) + return r.sandbox.ReadDirForGlob(path, HandlerCtx(ctx).Dir) } r.execHandler = noExecHandler() } diff --git a/interp/runner_expand.go b/interp/runner_expand.go index 7912c4df..d0befce2 100644 --- a/interp/runner_expand.go +++ b/interp/runner_expand.go @@ -29,7 +29,12 @@ func (r *Runner) fillExpandConfig(ctx context.Context) { func (r *Runner) updateExpandOpts() { r.ecfg.ReadDir2 = func(s string) ([]fs.DirEntry, error) { - return r.readDirHandler(r.handlerCtx(r.ectx, todoPos), s) + ctx := r.handlerCtx(r.ectx, todoPos) + if r.readDirHandler != nil { + return r.readDirHandler(ctx, s) + } + // Fallback when a custom openHandler was set without a readDirHandler. + return r.sandbox.ReadDirForGlob(s, HandlerCtx(ctx).Dir) } }