Re: [PATCH v2 1/2] dir: do not apply prefix to negative pathspecs

Elijah Newren [off-list ref] writes:

Hi Yannik,

On Thu, Sep 3, 2026 at 3:23 AM Yannik Tausch [off-list ref] wrote:

quoted

common_prefix_len() derives the common prefix solely from positive
pathspecs, skipping those marked with PATHSPEC_EXCLUDE. However,
match_pathspec_with_flags() also passes that prefix when matching the
negative pathspecs.

A negative pathspec may be shorter than the prefix. In that case,
match_pathspec_item() advances item->match beyond its allocation and
subtracts the prefix from item->len, producing a negative matchlen. It
then dereferences the out-of-bounds pointer. If the resulting byte is
not NUL, matchlen is converted to size_t when passed to ps_strncmp(),
which may cause a much larger out-of-bounds read.

The problem can be reproduced with AddressSanitizer:

...
Would it make sense to add a regression case whose failure before this
patch is deterministic without ASan?

Very good point.

Even if a negative pathspec were long enough, it would produce an
incorrect result if you strip the leading part of a negative entry.

With positive elements "a/b" and "a/c", and a negative element
"x/b", both paths "a/b/m" and "a/c/n" should match the pathspec with
these three elements, but if you incorrectly use prefix=2 to strip
the common prefix computed across positives, i.e., "a/", while
trying to see if the path "a/b/m" matches negative "x/b", we'd end
up trying to see if subpath "b/m" (in "a/b/m", after 2 leading
prefix bytes are stripped away) matches subpattern "b" (in "x/b",
after incorrectly stripping 2 leading bytes).  Yay, "b/m" begins
with "b" so it matches!  Not quite.

    $ git init
    $ mkdir -p a/b a/c
    $ >a/b/m >a/c/n
    $ git add a
    $ rungit jch ls-files a/b ':!x/b' a/c
    a/b/m
    a/c/n
    $ rungit master ls-files a/b ':!x/b' a/c
    a/c/n

So "if prefix computed across positives is longer than a negative
element" is a special case that may manifest as one extra breakage
(i.e., logically it is wrong in that it uses incorrectly shortened
pattern and path for negated matching and produce incorrect result,
but in addition to that, the negated pattern string points outside
the original string, accessing wrong piece of memory), but I tend to
agree that it is equally if not more important to demonstrate what
is broken even without that extra breakage.

Thanks.