v1.11.1: повышение устойчивости офлайн анализа подсистем из выгрузки к некорректным и специально сформированным каталогам

Author: Vyacheslav

dump/subsystem_dirposition_test.go

@@ -0,0 +1,111 @@
+//go:build unix
+
+package dump
+
+import (
+	"path/filepath"
+	"testing"
+	"time"
+)
+
+// These tests extend the writer-less-FIFO DoS coverage from the subsystem FILE
+// positions (subsystem_nonregular_test.go) to the subsystem DIRECTORY positions. A
+// directory open that is not type-checked first blocks forever on a writer-less FIFO
+// planted at that position, and (unlike a bounded read) the blocked open() is not
+// interruptible by ctx, so parseBounded's timeout IS the reproduction. plantFIFO,
+// parseBounded and the assertion helpers are shared from subsystem_nonregular_test.go
+// / subsystem_testhelper_test.go.
+
+// enumBounded runs EnumerateAppliedObjects under a deadline; a non-regular applied
+// folder that reached an unguarded open would block it forever (a DoS), so a timeout
+// here reproduces that. EnumerateAppliedObjects has no ctx, so bounding is external.
+func enumBounded(t *testing.T, dir string) []string {
+	t.Helper()
+	ch := make(chan []string, 1)
+	go func() { ch <- EnumerateAppliedObjects(dir) }()
+	select {
+	case r := <-ch:
+		return r
+	case <-time.After(10 * time.Second):
+		t.Fatal("EnumerateAppliedObjects did not return: a non-regular folder blocked enumeration (DoS)")
+		return nil
+	}
+}
+
+// DP-1 (top directory position): a writer-less FIFO planted exactly where the
+// top-level Subsystems/ directory is expected must not block the parse. Layout
+// detection opens Subsystems/ first, so this is the earliest and most severe hang.
+// After the fix it is refused, the whole-tree drop is NAMED (never silent), the walk
+// stays BOUNDED, and no error leaks the server-side path.
+func TestParse_WriterlessFIFOAtSubsystemsDir_BoundedAndNamed(t *testing.T) {
+	dir := t.TempDir()
+	plantFIFO(t, filepath.Join(dir, "Subsystems"))
+
+	subs, warnings := parseBounded(t, dir)
+	if len(subs) != 0 {
+		t.Errorf("a FIFO at the Subsystems position yields no subsystems; got %v", flattenNames(subs))
+	}
+	if !warningsContain(warnings, "каталог подсистем") {
+		t.Errorf("the refused Subsystems catalog must be NAMED in a warning; warnings=%v", warnings)
+	}
+}
+
+// DP-2 (nested recursion directory position): a valid Hierarchical parent whose
+// child-recursion directory Subsystems/<Name>/Subsystems is a writer-less FIFO must
+// not block the walk. walkHierarchical recurses into that directory unconditionally
+// after parsing the parent, so an unguarded open there hangs the walk exactly like
+// the top position. After the fix the parent still parses, the refused recursion
+// directory is NAMED (by its parent), and the walk stays BOUNDED.
+func TestWalkHierarchical_WriterlessFIFOAtRecursionDir_BoundedAndNamed(t *testing.T) {
+	dir := t.TempDir()
+	secWrite(t, filepath.Join(dir, "Subsystems", "Parent.xml"), secSubBody("Parent"))
+	plantFIFO(t, filepath.Join(dir, "Subsystems", "Parent", "Subsystems"))
+
+	subs, warnings := parseBounded(t, dir)
+	names := flattenNames(subs)
+	if !containsStr(names, "Parent") {
+		t.Errorf("the valid parent must still parse; names=%v", names)
+	}
+	if !warningsContain(warnings, "Parent") {
+		t.Errorf("the refused recursion directory must be NAMED (by its parent); warnings=%v", warnings)
+	}
+}
+
+// DP-3 (Ext intermediate directory position): a writer-less FIFO named Ext at
+// Subsystems/<N>/Ext. This position is reached only by an lstat THROUGH the FIFO
+// (Subsystems/<N>/Ext/Subsystem.xml), which returns ENOTDIR without ever opening the
+// FIFO, so it never blocked; this test documents that audit finding and guards it
+// from regressing. A valid Ext sibling must still parse and the FIFO child is NAMED.
+func TestWalkExt_WriterlessFIFOAtExtDir_BoundedAndNamed(t *testing.T) {
+	dir := t.TempDir()
+	secWrite(t, filepath.Join(dir, "Subsystems", "Good", "Ext", "Subsystem.xml"), secSubBody("Good"))
+	plantFIFO(t, filepath.Join(dir, "Subsystems", "Blk", "Ext"))
+
+	subs, warnings := parseBounded(t, dir)
+	names := flattenNames(subs)
+	if !containsStr(names, "Good") {
+		t.Errorf("the valid Ext sibling must still parse; names=%v", names)
+	}
+	if containsStr(names, "Blk") {
+		t.Errorf("the FIFO-Ext child must not parse; names=%v", names)
+	}
+	if !warningsContain(warnings, "Blk") {
+		t.Errorf("the FIFO-Ext child must be NAMED; warnings=%v", warnings)
+	}
+}
+
+// DP-4 (applied-kind folder position, universe enumerator): a writer-less FIFO named
+// like an applied-kind folder (Documents). EnumerateAppliedObjects only descends
+// directory entries (DirEntry.IsDir gate), so a FIFO entry is skipped before any
+// open; this documents that audit finding, and the hardened directory open is the
+// defense-in-depth backstop. A valid applied object must still enumerate, bounded.
+func TestEnumerateAppliedObjects_WriterlessFIFOFolder_Bounded(t *testing.T) {
+	dir := t.TempDir()
+	secWrite(t, filepath.Join(dir, "Catalogs", "Валюты.xml"), objBody("Валюты"))
+	plantFIFO(t, filepath.Join(dir, "Documents"))
+
+	got := enumBounded(t, dir)
+	if !containsStr(got, "Справочник.Валюты") {
+		t.Errorf("the valid applied object must still enumerate; got %v", got)
+	}
+}

dump/subsystem_reader.go

@@ -48,6 +48,37 @@ var (
 // dropped rather than wrapped. Customer-facing RU: no тире.
 var errReadSubsystemsRoot = errors.New("не удалось прочитать каталог подсистем дампа")
 
+// errNotDirectory marks a dump path that occupies a directory position but is not a
+// directory: a FIFO, socket, device, plain file, or a symlink standing in for the
+// directory. Opening such a position unconditionally is the DoS this guard closes: a
+// writer-less FIFO at a directory position blocks the open forever and, unlike a
+// bounded read, a blocked open() cannot be interrupted by ctx. Every directory read
+// refuses a non-directory BEFORE that blocking open, mirroring the subsystem-file
+// guard, and NAMES the refusal so the drop is never silent.
+var errNotDirectory = errors.New("dump path is not a directory")
+
+// errDumpDirNotDirectory is the path-free RU refusal returned by ParseAllSubsystemsCtx
+// when dumpDir ITSELF is a non-directory node (a FIFO, socket, device, plain file, or a
+// symlink resolving to one). It is errNotDirectory's guard one level up, at the dump
+// root: os.OpenRoot(dumpDir) on a writer-less FIFO blocks on an open() that ctx cannot
+// interrupt, so the node type is checked with os.Stat (which does not open it) BEFORE
+// the open. Customer-facing RU: no тире, never an absolute path.
+var errDumpDirNotDirectory = errors.New("каталог дампа имеет неверный тип")
+
+// dumpDirIsNonDir reports whether dumpDir exists but is NOT a directory: a FIFO,
+// socket, device, plain file, or a symlink resolving to one of those. os.Stat follows a
+// symlink and, crucially, does NOT open the node, so it returns immediately on a
+// writer-less FIFO where os.OpenRoot would block on the ctx-uninterruptible open (the
+// DoS this guards). A missing path or a permission error (err != nil) and a genuine
+// directory (including a symlink to a directory) all report false, so every
+// os.OpenRoot(dumpDir) entry point falls through to its existing missing / unreadable /
+// symlink-to-directory contract unchanged; only a non-directory dumpDir is refused
+// before the blocking open.
+func dumpDirIsNonDir(dumpDir string) bool {
+	fi, err := os.Stat(dumpDir)
+	return err == nil && !fi.IsDir()
+}
+
 // Subsystem is one node of the dump's subsystem forest: its canonical full path,
 // display synonym, direct member composition (Content, canonical RU full names)
 // and any nested child subsystems.
@@ -266,6 +297,9 @@ const (
 // so the walk returns an empty tree; a present but unreadable Subsystems/ yields
 // the path-free errReadSubsystemsRoot.
 func detectSubsystemLayout(dumpDir string) (subsystemLayout, error) {
+	if dumpDirIsNonDir(dumpDir) {
+		return layoutExt, nil // non-directory dumpDir: default layout, empty tree (never open it)
+	}
 	root, err := os.OpenRoot(dumpDir)
 	if err != nil {
 		return layoutExt, nil
@@ -285,9 +319,11 @@ func detectLayoutInRoot(root *os.Root) (subsystemLayout, error) {
 		if errors.Is(err, os.ErrPermission) {
 			return layoutExt, errReadSubsystemsRoot // present but unreadable: path-free
 		}
-		// Containment refusal (an escaping Subsystems/ symlink, os.Root "path
-		// escapes") or any other read error: never probe outside; default layout so
-		// the walk returns an empty tree.
+		// Containment refusal (an escaping Subsystems/ symlink, os.Root "path escapes"),
+		// a non-directory Subsystems position (errNotDirectory: a FIFO/socket/device/
+		// plain file, refused BEFORE the blocking open by openDirInRoot so detection
+		// never hangs), or any other read error: never probe outside; default layout so
+		// the walk returns an empty tree (the walk re-encounters and NAMES it).
 		return layoutExt, nil
 	}
 	for _, e := range entries {
@@ -327,13 +363,18 @@ func ParseAllSubsystems(dumpDir string) ([]Subsystem, error) {
 // (warnings) alongside the parsed tree. Hierarchical uses a disk-walk recursion;
 // Ext preserves the nested-children-from-XML behaviour. Every filesystem access is
 // confined to the dump via an os.Root (a crafted symlink that escapes the dump at
-// ANY path component is refused by the OS primitive, never followed), recursion is
-// depth-capped, and each dropped subsystem is NAMED in warnings (never silently
-// dropped).
+// ANY path component is refused by the OS primitive, never followed), every directory
+// AND file position is type-checked before it is opened (a writer-less FIFO planted at
+// any position is refused before the blocking open rather than hanging the walk),
+// recursion is depth-capped, and each dropped subsystem or directory is NAMED in
+// warnings (never silently dropped).
 func ParseAllSubsystemsCtx(ctx context.Context, dumpDir string) ([]Subsystem, []string, error) {
 	if err := ctx.Err(); err != nil {
 		return nil, nil, err
 	}
+	if dumpDirIsNonDir(dumpDir) {
+		return nil, nil, errDumpDirNotDirectory // dumpDir itself is a FIFO/socket/device/file: refuse before the blocking open
+	}
 	root, err := os.OpenRoot(dumpDir)
 	if err != nil {
 		if errors.Is(err, os.ErrNotExist) {
@@ -381,20 +422,30 @@ func (w *subsystemWalker) warn(name, reason string) {
 	w.warnings = append(w.warnings, fmt.Sprintf("подсистема %s: %s", n, reason))
 }
 
+// warnSubsystemsRoot NAMES a refused top-level Subsystems catalog: a non-directory at
+// the Subsystems position (a FIFO, socket, device, plain file, or a symlink standing in
+// for it) drops the entire tree, which must never be silent. Path-free, no тире.
+func (w *subsystemWalker) warnSubsystemsRoot() {
+	w.warnings = append(w.warnings, "каталог подсистем дампа имеет неверный тип и пропущен")
+}
+
 // readDir reads a dump-relative directory confined to the walker's os.Root.
 func (w *subsystemWalker) readDir(rel string) ([]os.DirEntry, error) {
 	return readDirInRoot(w.root, rel)
 }
 
 // readDirInRoot reads a directory confined to root and returns its entries sorted
 // by name (matching os.ReadDir), so the hierarchical walk's on-disk child ordering
-// is deterministic. os.Root confines EVERY path component beneath the root using
-// the OS primitive (openat2 RESOLVE_BENEATH on Linux, equivalents elsewhere), so an
-// escaping symlink at ANY depth is refused with a "path escapes" error rather than
+// is deterministic. The directory is opened through openDirInRoot, which refuses any
+// non-directory at that position (a FIFO/socket/device/plain file, or a symlink
+// standing in for the directory) BEFORE the blocking open, so a planted writer-less
+// FIFO at a directory position can never hang the walk. os.Root confines EVERY path
+// component beneath the root using the OS primitive (openat2 RESOLVE_BENEATH on Linux,
+// equivalents elsewhere), so an escaping symlink at ANY depth is refused rather than
 // followed out of the dump. os.File.ReadDir (unlike os.ReadDir) returns entries in
 // directory order, so they are sorted here.
 func readDirInRoot(root *os.Root, rel string) ([]os.DirEntry, error) {
-	f, err := root.Open(rel)
+	f, err := openDirInRoot(root, rel)
 	if err != nil {
 		return nil, err
 	}
@@ -407,6 +458,38 @@ func readDirInRoot(root *os.Root, rel string) ([]os.DirEntry, error) {
 	return entries, nil
 }
 
+// openDirInRoot opens rel as a directory confined to root, refusing any non-directory
+// at that position. It mirrors openSubsystemFile's guard, for directories: it lstats
+// and requires Mode().IsDir() BEFORE the open, so a writer-less FIFO, socket, device,
+// or a symlink standing in for the directory can never reach the blocking open() that
+// ctx cannot interrupt. It then opens with O_NONBLOCK on unix, so a directory swapped
+// for a FIFO in the check->use window still returns immediately instead of blocking,
+// and fstats the descriptor to require it be the very directory the lstat saw (IsDir
+// plus os.SameFile), which closes that TOCTOU window. Containment across every path
+// component is enforced by os.Root, so an escaping symlink at ANY depth is refused
+// rather than followed. A genuinely absent path returns os.ErrNotExist (callers treat
+// it as a normal empty position); a non-directory returns errNotDirectory (callers
+// NAME it); permission and containment ("path escapes") errors propagate unchanged.
+func openDirInRoot(root *os.Root, rel string) (*os.File, error) {
+	li, err := root.Lstat(rel)
+	if err != nil {
+		return nil, err
+	}
+	if !li.Mode().IsDir() {
+		return nil, errNotDirectory
+	}
+	f, err := root.OpenFile(rel, os.O_RDONLY|nonblockOpenFlag, 0)
+	if err != nil {
+		return nil, err
+	}
+	fi, err := f.Stat()
+	if err != nil || !fi.IsDir() || !os.SameFile(li, fi) {
+		_ = f.Close()
+		return nil, errNotDirectory
+	}
+	return f, nil
+}
+
 // openSubsystemFile opens a dump-relative subsystem file for reading, confined to
 // the walker's os.Root, and returns it only when it is a plain regular file.
 // Containment across every path component is enforced by os.Root: an escaping
@@ -475,6 +558,10 @@ func (w *subsystemWalker) walkExt(relRoot string) ([]Subsystem, error) {
 		if errors.Is(err, os.ErrPermission) {
 			return nil, errReadSubsystemsRoot // path-free
 		}
+		if errors.Is(err, errNotDirectory) {
+			w.warnSubsystemsRoot() // a non-directory Subsystems position: NAME the drop
+			return nil, nil
+		}
 		return nil, nil // containment refusal or other: empty tree, never leak
 	}
 	out := make([]Subsystem, 0, len(entries))
@@ -532,8 +619,19 @@ func (w *subsystemWalker) walkHierarchical(relRoot, parentPath string, depth int
 			if errors.Is(err, os.ErrPermission) {
 				return nil, errReadSubsystemsRoot // path-free, top level only
 			}
+			if errors.Is(err, errNotDirectory) {
+				w.warnSubsystemsRoot() // a non-directory Subsystems position: NAME the drop
+				return nil, nil
+			}
 			return nil, nil // containment refusal or other at top: empty tree
 		}
+		if errors.Is(err, errNotDirectory) {
+			// A non-directory at the recursion position (a FIFO/socket/device, or a
+			// symlink standing in for the child Subsystems/ directory): refused BEFORE
+			// the blocking open by openDirInRoot; skip its subtree, NAMED by its parent.
+			w.warn(parentPath, "вложенный каталог подсистем имеет неверный тип и пропущен")
+			return nil, nil
+		}
 		// A nested recursion directory that is unreadable or escapes the dump (an
 		// intermediate directory symlink refused by os.Root): skip its subtree, NAMED.
 		w.warn(parentPath, "каталог подсистемы недоступен и пропущен")