src/libexpr/search-path.cc: avoid out-of-bounds read on string_view

[trofi]

Without the change build with -D_GLIBCXX_ASSERTIONS exposes testsuite assertion:

$ gdb src/libexpr/tests/libnixexpr-tests
Reading symbols from src/libexpr/tests/libnixexpr-tests...
(gdb) break __glibcxx_assert_fail
(gdb) run
(gdb) bt
in std::__glibcxx_assert_fail(char const*, int, char const*, char const*)@plt () from /mnt/archive/big/git/nix/src/libexpr/libnixexpr.so
in std::basic_string_view<char, std::char_traits<char> >::operator[] (this=0x7fffffff56c0, __pos=4)
    at /nix/store/r74fw2j8rx5idb0w8s1s6ynwwgs0qmh9-gcc-14.0.0/include/c++/14.0.0/string_view:258
in nix::SearchPath::Prefix::suffixIfPotentialMatch (this=0x7fffffff5780, path=...) at src/libexpr/search-path.cc:15
in nix::SearchPathElem_suffixIfPotentialMatch_partialPrefix_Test::TestBody (this=0x555555a17540) at src/libexpr/tests/search-path.cc:62

As string sizes are usigned types (a - b) > 0 effectively means a != b. While the intention should be a > b.

The change fixes test suite pass.

Motivation

Context

Checklist for maintainers

Maintainers: tick if completed or explain if not relevant

• agreed on idea
• agreed on implementation strategy
• tests, as appropriate
  • functional tests - tests/**.sh
  • unit tests - src/*/tests
  • integration tests - tests/nixos/*
• documentation in the manual
• documentation in the internal API docs
• code and comments are self-explanatory
• commit message explains why the change was made
• new feature or incompatible change: updated release notes

Priorities

Add 👍 to pull requests you find important.

[iFreilicht]

Is this the only assertion that fails when -D_GLIBCXX_ASSERTIONS is set? If so, might it make sense to enable this while running the test suites to prevent regressions?

[trofi]

Certainly that failure was the only one on the nix test suite. And it was enough for the original reported of the failure at https://discourse.nixos.org/t/error-while-installing-home-manager/31720/13 to get working nix in Gentoo environment.

There is a tiny chance there are more bugs lurking, but so far it looks functioning.

As for -D_GLIBCXX_ASSERTIONS enabled by default it sounds reasonable to have at least as one of the flavours of the test. I'm not very familiar with nix's flake.nix to extend it though.

I hoped nix also uses -fsanitize=address as part of it's testsuite which should have caught the failures as well. But maybe boehm-gc does not make it as simple.

[github-actions]

Backport failed for 2.15-maintenance, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git fetch origin 2.15-maintenance
git worktree add -d .worktree/backport-8825-to-2.15-maintenance origin/2.15-maintenance
cd .worktree/backport-8825-to-2.15-maintenance
git checkout -b backport-8825-to-2.15-maintenance
ancref=$(git merge-base 5542c1f87ee3325bce8140f4087b12647b4107ef b74962c92b2d8d9b957934e0aefcf4983169ae1e)
git cherry-pick -x $ancref..b74962c92b2d8d9b957934e0aefcf4983169ae1e

[Ericson2314]

Thanks for fixing this! (I probably wrote the original)

[trofi]

I think be518e7 changed (path.size() > s && path[s] != '/') to n > 0 && (path.size() - n) > 0. It's only a part of 2.17.0. Should be no need to backport anywhere.

[github-actions]

Backport failed for 2.15-maintenance, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git fetch origin 2.15-maintenance
git worktree add -d .worktree/backport-8825-to-2.15-maintenance origin/2.15-maintenance
cd .worktree/backport-8825-to-2.15-maintenance
git checkout -b backport-8825-to-2.15-maintenance
ancref=$(git merge-base 5542c1f87ee3325bce8140f4087b12647b4107ef b74962c92b2d8d9b957934e0aefcf4983169ae1e)
git cherry-pick -x $ancref..b74962c92b2d8d9b957934e0aefcf4983169ae1e

[github-actions]

Backport failed for 2.16-maintenance, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git fetch origin 2.16-maintenance
git worktree add -d .worktree/backport-8825-to-2.16-maintenance origin/2.16-maintenance
cd .worktree/backport-8825-to-2.16-maintenance
git checkout -b backport-8825-to-2.16-maintenance
ancref=$(git merge-base 5542c1f87ee3325bce8140f4087b12647b4107ef b74962c92b2d8d9b957934e0aefcf4983169ae1e)
git cherry-pick -x $ancref..b74962c92b2d8d9b957934e0aefcf4983169ae1e

[github-actions]

Backport failed for 2.15-maintenance, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git fetch origin 2.15-maintenance
git worktree add -d .worktree/backport-8825-to-2.15-maintenance origin/2.15-maintenance
cd .worktree/backport-8825-to-2.15-maintenance
git checkout -b backport-8825-to-2.15-maintenance
ancref=$(git merge-base 5542c1f87ee3325bce8140f4087b12647b4107ef b74962c92b2d8d9b957934e0aefcf4983169ae1e)
git cherry-pick -x $ancref..b74962c92b2d8d9b957934e0aefcf4983169ae1e

[github-actions]

Backport failed for 2.16-maintenance, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git fetch origin 2.16-maintenance
git worktree add -d .worktree/backport-8825-to-2.16-maintenance origin/2.16-maintenance
cd .worktree/backport-8825-to-2.16-maintenance
git checkout -b backport-8825-to-2.16-maintenance
ancref=$(git merge-base 5542c1f87ee3325bce8140f4087b12647b4107ef b74962c92b2d8d9b957934e0aefcf4983169ae1e)
git cherry-pick -x $ancref..b74962c92b2d8d9b957934e0aefcf4983169ae1e

[github-actions]

Successfully created backport PR for 2.17-maintenance:

• [Backport 2.17-maintenance] src/libexpr/search-path.cc: avoid out-of-bounds read on string_view #8936

[github-actions]

Git push to origin failed for 2.17-maintenance with exitcode 1