lix-project/lix
20
57
Fork 31
Code Issues 349 Code Review (Gerrit) Projects 3 Releases Wiki Activity

diff-closures: fix a use after free

Browse source 
Found by looking for interesting asan reports from the test suite.

What happened here is that name got overwritten, but it was what
actually held the backing memory for the thing it got overwritten by,
which was a by-reference value coming out of std::regex.

Due to absurd reasons I cannot seem to use a string_view iterator here,
so I just copy the string with a longer lifetime instead. idk lol

==3796364==ERROR: AddressSanitizer: heap-use-after-free on address 0x503000014c61 at pc 0x74843523bf1d bp 0x7ffc68351330 sp 0x7ffc68350af0
READ of size 3 at 0x503000014c61 thread T0
    0 0x74843523bf1c in __asan_memcpy (/nix/store/mzhqknx2mc94jdz4n320hn1lml86398y-clang-wrapper-17.0.6/resource-root/lib/linux/libclang_rt.asan-x86_64.so+0x159f1c)
    1 0x6403cf6cbff4 in std::char_traits<char>::copy(char*, char const*, unsigned long) /nix/store/14c6s4xzhy14i2b05s00rjns2j93gzz4-gcc-13.2.0/include/c++/13.2.0/bits/char_traits.h:445:33
    <...>
    7 0x6403cf6cbff4 in std::__cxx11::sub_match<__gnu_cxx::__normal_iterator<char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>>::str() const /nix/store/14c6s4xzhy14i2b05s00rjns2j93gzz4-gcc-13.2.0/include/c++/13.2.0/bits/regex.h:966:6
    8 0x6403cf6cbff4 in std::__cxx11::sub_match<__gnu_cxx::__normal_iterator<char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>>::operator std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>() const /nix/store/14c6s4xzhy14i2b05s00rjns2j93gzz4-gcc-13.2.0/include/c++/13.2.0/bits/regex.h:955:16
    9 0x6403cf6cbff4 in nix::getClosureInfo[abi:cxx11](nix::ref<nix::Store>, nix::StorePath const&) /home/jade/lix/lix2/build/src/nix/diff-closures.cc:37:26
    10 0x6403cf6cd70c in nix::printClosureDiff(nix::ref<nix::Store>, nix::StorePath const&, nix::StorePath const&, std::basic_string_view<char, std::char_traits<char>>) /home/jade/lix/lix2/build/src/nix/diff-closures.cc:54:25
    11 0x6403cf873331 in CmdProfileDiffClosures::run(nix::ref<nix::Store>) /home/jade/lix/lix2/build/src/nix/profile.cc:479:17
    <...>

0x503000014c61 is located 17 bytes inside of 21-byte region [0x503000014c50,0x503000014c65)
freed by thread T0 here:
    0 0x748435250470 in operator delete(void*) (/nix/store/mzhqknx2mc94jdz4n320hn1lml86398y-clang-wrapper-17.0.6/resource-root/lib/linux/libclang_rt.asan-x86_64.so+0x16e470)
    <...>
    6 0x6403cf6cbda2 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>::~basic_string() /nix/store/14c6s4xzhy14i2b05s00rjns2j93gzz4-gcc-13.2.0/include/c++/13.2.0/bits/basic_string.h:792:9
    7 0x6403cf6cbda2 in nix::getClosureInfo[abi:cxx11](nix::ref<nix::Store>, nix::StorePath const&) /home/jade/lix/lix2/build/src/nix/diff-closures.cc:36:13
    8 0x6403cf6cd70c in nix::printClosureDiff(nix::ref<nix::Store>, nix::StorePath const&, nix::StorePath const&, std::basic_string_view<char, std::char_traits<char>>) /home/jade/lix/lix2/build/src/nix/diff-closures.cc:54:25
    <...>

previously allocated by thread T0 here:
    0 0x74843524fa38 in operator new(unsigned long) (/nix/store/mzhqknx2mc94jdz4n320hn1lml86398y-clang-wrapper-17.0.6/resource-root/lib/linux/libclang_rt.asan-x86_64.so+0x16da38)
    <...>
    9 0x6403cf6cb68c in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>::basic_string<std::basic_string_view<char, std::char_traits<char>>, void>(std::basic_string_view<char, std::char_traits<char>> const&, std::allocator<char> const&) /nix/store/14c6s4xzhy14i2b05s00rjns2j93gzz4-gcc-13.2.0/include/c++/13.2.0/bits/basic_string.h:784:4
    10 0x6403cf6cb68c in nix::getClosureInfo[abi:cxx11](nix::ref<nix::Store>, nix::StorePath const&) /home/jade/lix/lix2/build/src/nix/diff-closures.cc:33:21
    11 0x6403cf6cd70c in nix::printClosureDiff(nix::ref<nix::Store>, nix::StorePath const&, nix::StorePath const&, std::basic_string_view<char, std::char_traits<char>>) /home/jade/lix/lix2/build/src/nix/diff-closures.cc:54:25
    12 0x6403cf873331 in CmdProfileDiffClosures::run(nix::ref<nix::Store>) /home/jade/lix/lix2/build/src/nix/profile.cc:479:17
    <...>

Change-Id: I9c408cf2a3d3155f9f7b2ad4848ee6c741331db0
This commit is contained in:
jade 2024-06-18 00:54:05 -07:00
parent 8e6661cce7
commit b9b1bbd22f
1 changed files with 11 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

13
src/nix/diff-closures.cc
View file

@ -30,9 +30,18 @@ GroupedPaths getClosureInfo(ref<Store> store, const StorePath & toplevel)
version suffixes like "unstable"). */
static std::regex regex("(.*)-([a-z]+|lib32|lib64)");
std::smatch match;
std::string name(path.name());
std::string name{path.name()};
// Used to keep name alive through being potentially overwritten below
// (to not invalidate the references from the regex result)
//
// n.b. cannot be just path.name().{begin,end}() since that returns const
// char *, which does not, for some reason, convert as required on
// libstdc++. Seems like a libstdc++ bug or standard bug to me... we
// can afford the allocation in any case.
const std::string origName{path.name()};
std::string outputName;
if (std::regex_match(name, match, regex)) {
if (std::regex_match(origName, match, regex)) {
name = match[1];
outputName = match[2];
}