Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Undefined Behavior/Segfault when using git2 < 0.14.0 with libgit2 >= 1.4.0 #813

Closed
zRedShift opened this issue Feb 25, 2022 · 10 comments
Closed

Comments

@zRedShift
Copy link

A new field was added to git_fetch_options in v1.4.0 of libgit2, which makes this call unsound on the condition that any version of git2 below v0.14.0 is used and libgit2.so.1.4 is linked, which has happened already on bleeding edge distributions.

This has already happened to me and apparently a few others on killercup/cargo-edit#641, and in my investigation I found the issue to be due to a segfault in validate_custom_headers, as follow_redirects is incorrectly "initialized" with custom_headers's git_strarray while the actual custom_headers points to junk data past the end of the struct, causing git_strarray's count to be uninitialized memory/random junk, which causes the code to dereference strings which also points to random junk.

I'm not entirely sure how to fix this gracefully. Maybe issue an advisory to all crates using git2 to upgrade to 0.14.0? Wouldn't it cause the same issue in reverse to those who still use libgit2.so.1.3?

@saethlin
Copy link
Member

saethlin commented Mar 1, 2022

This is a bug in libgit2-sys, I tried to explain what's going on and what could be done in the issue that reported this problem when there was a ABI break with 1.3.0: #746 (comment)

Now that there has been another ABI break I think it's pretty clear that the version of libgit2 doesn't indicate ABI stability, and the build script should be changed to an exact version. If possible this should be backported to past releases so there's a chance people stop getting segfaults from other crates that depend on older versions of libgit2-rs.

Frederick888 added a commit to kbknapp/cargo-outdated that referenced this issue Mar 2, 2022
saethlin added a commit to saethlin/git2-rs that referenced this issue Mar 4, 2022
libgit2 does not have a stable ABI across minor versions, as has been
demonstrated in the last few minor releases, see rust-lang#813 and rust-lang#746. This
pain is primarily suffered by users of rolling release distros, because
they tend to get a new libgit2 version before the authors of libraries
release versions that pull in the new libgit2-sys version which works
with the new binary.

This patch means that going forward, users don't need to rush to
upgrade their version of libgit2-sys or suffer errors/UB in the
meantime. If the system version changes, they will just start using
the vendored version which has been tested against the bindings.
@demurgos
Copy link

demurgos commented Mar 9, 2022

Is there a workaround available currently for this issue? I tried enabling the vendored-libgit2 feature but still get the crash in my project.

@saethlin
Copy link
Member

saethlin commented Mar 9, 2022

If you run ldd on your executable, does it mention libgit2? If not, I would very much like to be able to compile this project for myself and check this out.

The vendored-libgit2 feature ought to fix this.

@demurgos
Copy link

demurgos commented Mar 9, 2022

I'll try to reduce the binary to a minimal reproduction.
Here is what I have:

  • Project: https://gitlab.com/eternaltwin/katal/
  • Branch: git2-issue (it has the vendored feature)
  • Reproduction:
    1. cargo build --bin katal
    2. ldd ./target/debug/katal
      Output:
      $ ldd ./target/debug/katal
        linux-vdso.so.1 (0x00007ffde5954000)
        libz.so.1 => /usr/lib/libz.so.1 (0x00007f0ff0015000)
        libssl.so.1.1 => /usr/lib/libssl.so.1.1 (0x00007f0feff83000)
        libcrypto.so.1.1 => /usr/lib/libcrypto.so.1.1 (0x00007f0fefca2000)
        libgcc_s.so.1 => /usr/lib/libgcc_s.so.1 (0x00007f0fefc87000)
        libm.so.6 => /usr/lib/libm.so.6 (0x00007f0fefb9f000)
        libc.so.6 => /usr/lib/libc.so.6 (0x00007f0fef995000)
        /lib64/ld-linux-x86-64.so.2 => /usr/lib64/ld-linux-x86-64.so.2 (0x00007f0ff1c36000)
        libpthread.so.0 => /usr/lib/libpthread.so.0 (0x00007f0fef98e000)
        libdl.so.2 => /usr/lib/libdl.so.2 (0x00007f0fef989000)
      
    3. At runtime, I get a segfault when calling remote::fetch

This project is used to manage the services on my server and spawns multiple sub-processes so it may not be easiest one to debug the issue. I am compiling on Arch Linux 64 bit. I have libgit2 1:1.4.2-1 on my system (but it should not matter when using the vendored feature).

@zRedShift
Copy link
Author

Seems like system libgit2 is being dynamically loaded with dlopen.
In case it's not that, can you post your gdb backtrace? If stuff is missing you can compile your program and libgit2 in debug mode (you can do that using cmake -DCMAKE_BUILD_TYPE=Debug).

@demurgos
Copy link

I will see what I can find.
In the meantime, here is a smaller repo reproducing the issue on my computer: https://github.com/demurgos/git2-segfault

@demurgos
Copy link

I ran this smaller program with strace: I don´t think libgit2 is loaded dynamically (I included the strace log in the repo).

Running it with gdb, I get the following output:

$ gdb ./target/debug/git2-segfault
GNU gdb (GDB) 11.2
Copyright (C) 2022 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-pc-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from ./target/debug/git2-segfault...
warning: Missing auto-load script at offset 0 in section .debug_gdb_scripts
of file /data/projects/various/git2-segfault/target/debug/git2-segfault.
Use `info auto-load python-scripts [REGEXP]' to list them.
(gdb) start
Temporary breakpoint 1 at 0x18bb7: file src/main.rs, line 5.
Starting program: /data/projects/various/git2-segfault/target/debug/git2-segfault 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib/libthread_db.so.1".

Temporary breakpoint 1, git2_segfault::main () at src/main.rs:5
5           let input_path = PathBuf::from("./git2");
(gdb) continue
Continuing.

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7b634bd in __strlen_avx2 () from /usr/lib/libc.so.6

Sorry for the big wall of text, I am not very familiar with gdb. All I can see is that the segfault seems to come from __strlen_avx2 () from /usr/lib/libc.so.6.
I am not sure how to provide more information.

@saethlin
Copy link
Member

This is a null pointer dereference. ASan reports this:

AddressSanitizer:DEADLYSIGNAL
=================================================================
==124554==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x55c97b93d9b0 bp 0x7ffdc9a03650 sp 0x7ffdc9a02e08 T0)
==124554==The signal is caused by a READ memory access.
==124554==Hint: address points to the zero page.
    #0 0x55c97b93d9b0 in __sanitizer::internal_strlen(char const*) /rustc/llvm/src/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_libc.cpp:167:10
    #1 0x55c97b917951 in __interceptor_strdup /rustc/llvm/src/llvm-project/compiler-rt/lib/asan/asan_interceptors.cpp:436:17
    #2 0x55c97bae1e35 in stdalloc__strdup /home/ben/.cargo/registry/src/github.com-1ecc6299db9ec823/libgit2-sys-0.13.1+1.4.2/libgit2/src/allocators/stdalloc.c:57:8
    #3 0x55c97bacd0bc in maybe_want_oid /home/ben/.cargo/registry/src/github.com-1ecc6299db9ec823/libgit2-sys-0.13.1+1.4.2/libgit2/src/fetch.c:79:19
    #4 0x55c97bacd392 in filter_wants /home/ben/.cargo/registry/src/github.com-1ecc6299db9ec823/libgit2-sys-0.13.1+1.4.2/libgit2/src/fetch.c:149:16
    #5 0x55c97bacd483 in git_fetch_negotiate /home/ben/.cargo/registry/src/github.com-1ecc6299db9ec823/libgit2-sys-0.13.1+1.4.2/libgit2/src/fetch.c:172:6
    #6 0x55c97ba666d9 in git_remote__download /home/ben/.cargo/registry/src/github.com-1ecc6299db9ec823/libgit2-sys-0.13.1+1.4.2/libgit2/src/remote.c:1306:15
    #7 0x55c97ba6691a in git_remote_fetch /home/ben/.cargo/registry/src/github.com-1ecc6299db9ec823/libgit2-sys-0.13.1+1.4.2/libgit2/src/remote.c:1373:10
    #8 0x55c97b9594a3 in git2::remote::Remote::fetch::h75afb82f5eea0146 /home/ben/.cargo/registry/src/github.com-1ecc6299db9ec823/git2-0.14.1/src/remote.rs:303:13
    #9 0x55c97b9641ae in git2_segfault::main::h2ab6d5e209923114 /tmp/git2-segfault/src/main.rs:22:9

I suspect this is unrelated to the ABI problem.

@demurgos
Copy link

@saethlin I opened a dedicated issue in #819 then. I was confused because the error appeared a few days ago after a system update.

joshtriplett pushed a commit that referenced this issue Mar 18, 2022
libgit2 does not have a stable ABI across minor versions, as has been
demonstrated in the last few minor releases, see #813 and #746. This
pain is primarily suffered by users of rolling release distros, because
they tend to get a new libgit2 version before the authors of libraries
release versions that pull in the new libgit2-sys version which works
with the new binary.

This patch means that going forward, users don't need to rush to
upgrade their version of libgit2-sys or suffer errors/UB in the
meantime. If the system version changes, they will just start using
the vendored version which has been tested against the bindings.
@joshtriplett
Copy link
Member

I believe this has now been addressed in current git2, which ensures that it depends on a sufficiently recent libgit2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants