GHSA-P6PH-3JX2-3337: Root-Cause Fix for OpenList Bleve Search Scope Bypass

Vulnerability

GHSA-P6PH-3JX2-3337 describes two authorization failures in OpenList search and sharing flows. The first is a path-scope validation bug: directory access checks used strings.HasPrefix, which is not a safe authorization primitive for filesystem-like hierarchies. A base path such as /base incorrectly matches sibling paths like /base2, enabling horizontal privilege escalation and metadata disclosure across adjacent namespaces. This vulnerable pattern appears in search and sharing handlers and is directly shown in the commit at 59bd3431408578f420895457554700cc9a52375a.

The second issue is a search-side information leak in Bleve-backed queries. The original implementation executed a broad search, returned searchResults.Total, and only then filtered unauthorized nodes in the HTTP handler. That means the response total reflected all index hits, not just accessible hits, allowing blind verification of restricted metadata and keyword presence within unauthorized scopes. The vulnerable flow is visible in 84ecda35aae2bd0020474086e6ddfd3aa2340679, where handler-side filtering occurred after search.Search returned both page data and total count.

if !strings.HasPrefix(node.Parent, user.BasePath) {
    // vulnerable: /base matches /base2
}

nodes, total, err := search.Search(c, req.SearchReq)
// ...filter unauthorized nodes afterwards...
// total still exposes global hit count

In effect, the bug class is not just insufficient output filtering; it is incorrect trust in string prefix matching for path authorization, combined with a search API contract that computed totals before access control.

Patch

The first patch set replaces prefix checks with utils.IsSubPath in the search and sharing handlers, and adds re-validation of shared file paths against the creator's current BasePath. This is a stronger semantic check for hierarchical containment and also protects against stale sharing state after an administrator changes a creator's scope. These changes are in commit 59bd3431408578f420895457554700cc9a52375a.

if !utils.IsSubPath(user.BasePath, node.Parent) {
    // patched: validates real subpath relationship
}

if sharing.Creator != nil && !sharing.Creator.IsAdmin() {
    for _, f := range sharing.Files {
        if !utils.IsSubPath(sharing.Creator.BasePath, f) {
            return "", errors.Errorf("sharing path [%s] is outside the creator's base path", f)
        }
    }
}

The second patch set introduces a filtered-search abstraction so authorization filtering is applied before pagination and total calculation. A new FilteredSearcher interface is added, with a Bleve implementation that iterates through search results in batches, applies path and access filters, and increments totals only for authorized matches. The HTTP handler now calls search.SearchFiltered instead of search.Search. This directly removes the metadata leak caused by exposing unfiltered totals. The implementation and tests are in commit 84ecda35aae2bd0020474086e6ddfd3aa2340679.

nodes, total, err := search.SearchFiltered(c, req.SearchReq, func(node model.SearchNode) bool {
    if !utils.IsSubPath(user.BasePath, node.Parent) {
        return false
    }
    return common.CanAccess(user, meta, path.Join(node.Parent, node.Name), req.Password)
})

The Bleve implementation also stabilizes pagination by sorting on name and _id, then using searchAfter across batches. The added tests specifically cover duplicate sort values and filter-before-pagination semantics, which are both relevant to correctness of the new authorization-aware total calculation.

Review

Pros

The path authorization fix is well targeted. Replacing strings.HasPrefix with utils.IsSubPath addresses the actual root cause rather than trying to sanitize edge cases around string comparison. Applying the same correction in both search and sharing paths reduces inconsistency across authorization boundaries. The additional re-validation of shared paths against the creator's current base path is also a meaningful hardening step, because it closes a time-of-check/time-of-use style drift in persisted sharing state.

The search fix is also structurally sound. Moving filtering into SearchFiltered changes the API contract so totals and pagination are derived from authorized results, not raw index hits. That is the correct place to solve the Bleve metadata leak. The fallback implementation in internal/search/search.go preserves behavior for non-filter-aware backends while still enforcing filter-before-pagination semantics. The tests added in both Bleve-specific and generic search layers are relevant and source-grounded.

Cons

The reviewability of the path fix depends on the correctness of utils.IsSubPath, but its implementation is not included in the provided sources. If it does not normalize separators, resolve traversal segments, or handle edge cases consistently, residual bypass risk could remain. The patch strongly suggests the right direction, but the security guarantee ultimately rests on that helper.

The new Bleve filtering model may have performance costs for broad queries over large indexes, because it scans batched results until enough authorized matches are collected and the authorized total is computed. That tradeoff is acceptable for confidentiality, but engineers should expect different latency characteristics compared with returning raw engine totals. Also, the handler filter still depends on meta and common.CanAccess; if those inputs are stale or incomplete, authorization correctness remains coupled to higher-level access logic.

Finally, the patch fixes the observed leakage in search totals, but it does not demonstrate index-level scoping. Unauthorized documents are still searchable internally and then discarded in application logic. That is a valid mitigation for this advisory, but it is not the strongest possible isolation model.

Verdict

Root-cause.

This patch addresses both underlying causes identified in the advisory: unsafe path-prefix authorization and post-search filtering that leaked unfiltered totals. The switch to utils.IsSubPath is the correct semantic repair for hierarchical path checks, and the introduction of SearchFiltered fixes the API-layer design flaw that exposed Bleve global hit statistics. Based on the provided commits and advisory references, this is a substantive and technically coherent fix rather than a superficial output scrub.

References: commit 59bd3431408578f420895457554700cc9a52375a, commit 84ecda35aae2bd0020474086e6ddfd3aa2340679, GitHub Security Advisory, CVE Reports summary.