Discarded afero WalkFunc error yields nil FileInfo dereference (nil-pointer panic) in search handler

Status

Repository / Component

Plain-English Description

The search feature ignores errors while walking the file tree. If a file disappears at the wrong moment during a search, the code uses an empty file record and crashes that one search request. Only the single request fails; the server keeps running.

Description of the Underlying Issue

search/search.go:29 declares the afero.Walk WalkFunc with signature func(fPath string, f os.FileInfo, _ error) error, discarding the per-entry error. When a child entry's lstat fails mid-walk, afero (path.go:63-67, basepath.go:186-196) invokes walkFn with fileInfo == nil. The fPath == scope early return (search.go:38) guards only the root nil, so child nils flow to found(relativePath, f) in both the terms and default branches (search.go:69, :75) with no nil check. http/search.go:64-65 then evaluates a response map literal containing 'dir': f.IsDir(); per the Go spec a select statement evaluates its send values on entry, so f.IsDir() runs on the nil FileInfo and panics. The http package has zero recover() calls and searchHandler runs search.Search synchronously in the handler goroutine, so the panic reaches net/http's per-connection recover. Distinct from the unbounded-walk DOK-100029.

Potential Attack

An authenticated user issues GET /api/search?query= (empty query -> every entry reaches found()) over a scope while concurrently deleting files in that scope. A child removed after its parent's readDirNames but before its lstat yields f == nil -> found(_, nil) -> f.IsDir() nil-pointer panic. The attacker can self-induce this by deleting their own files in a loop while running an empty/all-matching search over the same scope, composing with the full-tree walk noted in DOK-100029.

Outcomes of Potential Attack

The single /api/search request is aborted by net/http's per-connection recover, plus a panic stack trace in the log. There is no process crash, goroutine leak, or memory leak. Reliability is probabilistic — the panic requires winning a TOCTOU lstat race.

Affected Scope

GET /api/search over any scope (withUser/JWT only, no extra capability gate); nil os.FileInfo dereferenced at http/search.go:65 f.IsDir() via the found() callback from search/search.go:69 and :75.

Suggested Fix (plain english)

Stop ignoring walk errors: skip an entry whose information could not be read instead of passing an empty record onward, and guard against an empty record before using it.

Suggested Fix (detailed)

Stop discarding the WalkFunc error at search/search.go:29: on a non-nil err (or a nil f) for a child entry, return nil to skip it before calling found(). As a defense-in-depth also guard the dereference as f != nil && f.IsDir() at http/search.go:65. Root cause is the '_ error' parameter combined with the unchecked found(relativePath, f) calls. Verify that a search over a scope with entries deleted mid-walk returns a normal (possibly partial) result with no panic stack in the log. Stop discarding the WalkFunc error at search/search.go:29: on a non-nil err (or a nil f) for a child entry return nil to skip it before calling found(); alternatively guard the dereference as f != nil && f.IsDir() at http/search.go:65. Root cause is the '_ error' parameter combined with the unchecked found(relativePath, f) calls.

Validation

The nil-deref defect and its mechanism are fully proven from the workspace plus on-disk afero source, and it is a distinct weakness from the unbounded-walk DOK-100029. It is possibly_valid rather than fully_valid because an actual panic is gated on winning a TOCTOU lstat race whose reliability is a runtime timing property not settleable from source; impact is honestly bounded to a single aborted request.

first_opinion

Full Evidence

search/search.go:29 — afero.Walk WalkFunc signature func(fPath string, f os.FileInfo, _ error) error discards the per-entry error

search/search.go:38 — the fPath == scope early return guards the ROOT nil delivered by afero path.go:103, so only CHILD entries can reach found()

search/search.go:69 and :75 — found(relativePath, f) passes f with no nil check in both the terms and default branches

http/search.go:64-65 — the response map literal evaluates 'dir': f.IsDir(); per the Go spec a select statement evaluates its send values on entry, so f.IsDir() runs even if ctx is already Done -> nil deref panics

/home/ubuntu/go/pkg/mod/github.com/spf13/afero@v1.15.0/path.go:63-67 — on a child lstat error afero calls walkFn(filename, fileInfo, err) with fileInfo from lstatIfPossible, which returns nil on error (path.go:81-84)

/home/ubuntu/go/pkg/mod/github.com/spf13/afero@v1.15.0/basepath.go:186-196 — BasePathFs.LstatIfPossible returns nil os.FileInfo on every error branch (RealPath error, or propagated source error)

http/ has zero recover() (grep) and searchHandler runs search.Search synchronously in the handler goroutine, so the panic reaches net/http conn.serve which recovers per connection

Proven fact: The discarded WalkFunc error plus the missing nil check is a real defect: if a child entry's lstat fails mid-walk, afero delivers f == nil and search.go forwards it to found(), and http/search.go:65 dereferences it via f.IsDir() -> nil-pointer panic

Proven fact: Only child entries are affected (root nil is guarded by search.go:38), and the panic is recovered per connection, so impact is a single aborted /api/search request plus panic-stack log noise — no process crash, goroutine leak, or memory leak

Proven fact: The nil-delivery behavior is confirmed against the on-disk afero v1.15.0 source, not assumed

Unvalidated fact: Triggering an actual panic requires a child entry to become unstattable between afero's readDirNames and the subsequent lstat (a TOCTOU race). Whether, and how reliably, an attacker can win this race is a runtime timing property (filesystem, scheduler, tree size) that cannot be settled by reading source

Unvalidated fact: An attacker can plausibly self-induce it (delete their own files in a loop while running an empty/all-matching /api/search over the same scope, which composes with the full-tree walk noted in DOK-100029), but reliable exploitation is probabilistic rather than guaranteed per request

search/search.go:38 — the fPath == scope early return guards only the ROOT nil (afero path.go:103), so only CHILD entries reach found()

search/search.go:69,75 — found(relativePath, f) passes f with no nil check in both terms and default branches

http/search.go:64-65 — the response map literal evaluates 'dir': f.IsDir(); a select send-value is evaluated on entry, so f.IsDir() runs even if ctx is Done -> nil deref

afero@v1.15.0/path.go:63-67,81-84 — on a child lstat error afero calls walkFn(filename, fileInfo, err) with fileInfo nil

afero@v1.15.0/basepath.go:186-196 — BasePathFs.LstatIfPossible returns nil os.FileInfo on every error branch

http/ has zero recover() (grep) and searchHandler runs search.Search synchronously, so the panic reaches net/http conn.serve per-connection recover

Proven fact: The discarded WalkFunc error plus the missing nil check is a real defect: if a child entry's lstat fails mid-walk, afero delivers f == nil and search.go forwards it to found(), and http/search.go:65 dereferences it via f.IsDir() -> nil-pointer panic.

Proven fact: Only child entries are affected (root nil guarded by search.go:38); the panic is recovered per connection, so impact is a single aborted /api/search request plus panic-stack log noise — no process crash, goroutine leak, or memory leak.

Proven fact: The nil-delivery behavior is confirmed against the on-disk afero v1.15.0 source, not assumed.

Unvalidated fact: Triggering an actual panic requires a child entry to become unstattable between afero's readDirNames and the subsequent lstat (a TOCTOU race). Whether, and how reliably, an attacker can win this race is a runtime timing property (filesystem, scheduler, tree size) not settleable from source.

Unvalidated fact: An attacker can plausibly self-induce it (delete own files in a loop while running an empty/all-matching search over the same scope, composing with DOK-100029's full-tree walk), but reliable exploitation is probabilistic rather than guaranteed per request.

search/search.go:29 — afero.Walk WalkFunc signature `func(fPath string, f os.FileInfo, _ error) error` discards the per-entry error

search/search.go:69 and search/search.go:75 — `found(relativePath, f)` passes f without a nil check; root-lstat failure is guarded by the `fPath == scope` early-return at search/search.go:38, so only CHILD entries reach found()

http/search.go:65 — `"dir": f.IsDir()` dereferences the FileInfo inside the response map literal

http/search.go:17 (withUser) + no recover() in http/ (grep) — handler runs synchronously; panic propagates to net/http conn.serve which recovers per-request

/home/ubuntu/go/pkg/mod/github.com/spf13/afero@v1.15.0/path.go:63-65 — child-lstat-error branch calls walkFn(filename, fileInfo, err) with fileInfo=nil

/home/ubuntu/go/pkg/mod/github.com/spf13/afero@v1.15.0/basepath.go:186-196 — BasePathFs.LstatIfPossible returns/propagates nil FileInfo on every error branch

/home/ubuntu/go/pkg/mod/github.com/spf13/afero@v1.15.0/os.go:102-105 — OsFs.LstatIfPossible returns os.Lstat result, which is nil on ENOENT