aboutsummaryrefslogtreecommitdiffstats
path: root/t (follow)
AgeCommit message (Collapse)AuthorFilesLines
2022-12-09pretty: fix out-of-bounds read when parsing invalid padding formatPatrick Steinhardt1-0/+6
An out-of-bounds read can be triggered when parsing an incomplete padding format string passed via `--pretty=format` or in Git archives when files are marked with the `export-subst` gitattribute. This bug exists since we have introduced support for truncating output via the `trunc` keyword a7f01c6b4d (pretty: support truncating in %>, %< and %><, 2013-04-19). Before this commit, we used to find the end of the formatting string by using strchr(3P). This function returns a `NULL` pointer in case the character in question wasn't found. The subsequent check whether any character was found thus simply checked the returned pointer. After the commit we switched to strcspn(3P) though, which only returns the offset to the first found character or to the trailing NUL byte. As the end pointer is now computed by adding the offset to the start pointer it won't be `NULL` anymore, and as a consequence the check doesn't do anything anymore. The out-of-bounds data that is being read can in fact end up in the formatted string. As a consequence, it is possible to leak memory contents either by calling git-log(1) or via git-archive(1) when any of the archived files is marked with the `export-subst` gitattribute. ==10888==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000000398 at pc 0x7f0356047cb2 bp 0x7fff3ffb95d0 sp 0x7fff3ffb8d78 READ of size 1 at 0x602000000398 thread T0 #0 0x7f0356047cb1 in __interceptor_strchrnul /usr/src/debug/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:725 #1 0x563b7cec9a43 in strbuf_expand strbuf.c:417 #2 0x563b7cda7060 in repo_format_commit_message pretty.c:1869 #3 0x563b7cda8d0f in pretty_print_commit pretty.c:2161 #4 0x563b7cca04c8 in show_log log-tree.c:781 #5 0x563b7cca36ba in log_tree_commit log-tree.c:1117 #6 0x563b7c927ed5 in cmd_log_walk_no_free builtin/log.c:508 #7 0x563b7c92835b in cmd_log_walk builtin/log.c:549 #8 0x563b7c92b1a2 in cmd_log builtin/log.c:883 #9 0x563b7c802993 in run_builtin git.c:466 #10 0x563b7c803397 in handle_builtin git.c:721 #11 0x563b7c803b07 in run_argv git.c:788 #12 0x563b7c8048a7 in cmd_main git.c:923 #13 0x563b7ca99682 in main common-main.c:57 #14 0x7f0355e3c28f (/usr/lib/libc.so.6+0x2328f) #15 0x7f0355e3c349 in __libc_start_main (/usr/lib/libc.so.6+0x23349) #16 0x563b7c7fe0e4 in _start ../sysdeps/x86_64/start.S:115 0x602000000398 is located 0 bytes to the right of 8-byte region [0x602000000390,0x602000000398) allocated by thread T0 here: #0 0x7f0356072faa in __interceptor_strdup /usr/src/debug/gcc/libsanitizer/asan/asan_interceptors.cpp:439 #1 0x563b7cf7317c in xstrdup wrapper.c:39 #2 0x563b7cd9a06a in save_user_format pretty.c:40 #3 0x563b7cd9b3e5 in get_commit_format pretty.c:173 #4 0x563b7ce54ea0 in handle_revision_opt revision.c:2456 #5 0x563b7ce597c9 in setup_revisions revision.c:2850 #6 0x563b7c9269e0 in cmd_log_init_finish builtin/log.c:269 #7 0x563b7c927362 in cmd_log_init builtin/log.c:348 #8 0x563b7c92b193 in cmd_log builtin/log.c:882 #9 0x563b7c802993 in run_builtin git.c:466 #10 0x563b7c803397 in handle_builtin git.c:721 #11 0x563b7c803b07 in run_argv git.c:788 #12 0x563b7c8048a7 in cmd_main git.c:923 #13 0x563b7ca99682 in main common-main.c:57 #14 0x7f0355e3c28f (/usr/lib/libc.so.6+0x2328f) #15 0x7f0355e3c349 in __libc_start_main (/usr/lib/libc.so.6+0x23349) #16 0x563b7c7fe0e4 in _start ../sysdeps/x86_64/start.S:115 SUMMARY: AddressSanitizer: heap-buffer-overflow /usr/src/debug/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:725 in __interceptor_strchrnul Shadow bytes around the buggy address: 0x0c047fff8020: fa fa fd fd fa fa 00 06 fa fa 05 fa fa fa fd fd 0x0c047fff8030: fa fa 00 02 fa fa 06 fa fa fa 05 fa fa fa fd fd 0x0c047fff8040: fa fa 00 07 fa fa 03 fa fa fa fd fd fa fa 00 00 0x0c047fff8050: fa fa 00 01 fa fa fd fd fa fa 00 00 fa fa 00 01 0x0c047fff8060: fa fa 00 06 fa fa 00 06 fa fa 05 fa fa fa 05 fa =>0x0c047fff8070: fa fa 00[fa]fa fa fd fa fa fa fd fd fa fa fd fd 0x0c047fff8080: fa fa fd fd fa fa 00 00 fa fa 00 fa fa fa fd fa 0x0c047fff8090: fa fa fd fd fa fa 00 00 fa fa fa fa fa fa fa fa 0x0c047fff80a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c047fff80b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c047fff80c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==10888==ABORTING Fix this bug by checking whether `end` points at the trailing NUL byte. Add a test which catches this out-of-bounds read and which demonstrates that we used to write out-of-bounds data into the formatted message. Reported-by: Markus Vervier <markus.vervier@x41-dsec.de> Original-patch-by: Markus Vervier <markus.vervier@x41-dsec.de> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-12-09pretty: fix out-of-bounds read when left-flushing with stealingPatrick Steinhardt1-0/+6
With the `%>>(<N>)` pretty formatter, you can ask git-log(1) et al to steal spaces. To do so we need to look ahead of the next token to see whether there are spaces there. This loop takes into account ANSI sequences that end with an `m`, and if it finds any it will skip them until it finds the first space. While doing so it does not take into account the buffer's limits though and easily does an out-of-bounds read. Add a test that hits this behaviour. While we don't have an easy way to verify this, the test causes the following failure when run with `SANITIZE=address`: ==37941==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x603000000baf at pc 0x55ba6f88e0d0 bp 0x7ffc84c50d20 sp 0x7ffc84c50d10 READ of size 1 at 0x603000000baf thread T0 #0 0x55ba6f88e0cf in format_and_pad_commit pretty.c:1712 #1 0x55ba6f88e7b4 in format_commit_item pretty.c:1801 #2 0x55ba6f9b1ae4 in strbuf_expand strbuf.c:429 #3 0x55ba6f88f020 in repo_format_commit_message pretty.c:1869 #4 0x55ba6f890ccf in pretty_print_commit pretty.c:2161 #5 0x55ba6f7884c8 in show_log log-tree.c:781 #6 0x55ba6f78b6ba in log_tree_commit log-tree.c:1117 #7 0x55ba6f40fed5 in cmd_log_walk_no_free builtin/log.c:508 #8 0x55ba6f41035b in cmd_log_walk builtin/log.c:549 #9 0x55ba6f4131a2 in cmd_log builtin/log.c:883 #10 0x55ba6f2ea993 in run_builtin git.c:466 #11 0x55ba6f2eb397 in handle_builtin git.c:721 #12 0x55ba6f2ebb07 in run_argv git.c:788 #13 0x55ba6f2ec8a7 in cmd_main git.c:923 #14 0x55ba6f581682 in main common-main.c:57 #15 0x7f2d08c3c28f (/usr/lib/libc.so.6+0x2328f) #16 0x7f2d08c3c349 in __libc_start_main (/usr/lib/libc.so.6+0x23349) #17 0x55ba6f2e60e4 in _start ../sysdeps/x86_64/start.S:115 0x603000000baf is located 1 bytes to the left of 24-byte region [0x603000000bb0,0x603000000bc8) allocated by thread T0 here: #0 0x7f2d08ebe7ea in __interceptor_realloc /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:85 #1 0x55ba6fa5b494 in xrealloc wrapper.c:136 #2 0x55ba6f9aefdc in strbuf_grow strbuf.c:99 #3 0x55ba6f9b0a06 in strbuf_add strbuf.c:298 #4 0x55ba6f9b1a25 in strbuf_expand strbuf.c:418 #5 0x55ba6f88f020 in repo_format_commit_message pretty.c:1869 #6 0x55ba6f890ccf in pretty_print_commit pretty.c:2161 #7 0x55ba6f7884c8 in show_log log-tree.c:781 #8 0x55ba6f78b6ba in log_tree_commit log-tree.c:1117 #9 0x55ba6f40fed5 in cmd_log_walk_no_free builtin/log.c:508 #10 0x55ba6f41035b in cmd_log_walk builtin/log.c:549 #11 0x55ba6f4131a2 in cmd_log builtin/log.c:883 #12 0x55ba6f2ea993 in run_builtin git.c:466 #13 0x55ba6f2eb397 in handle_builtin git.c:721 #14 0x55ba6f2ebb07 in run_argv git.c:788 #15 0x55ba6f2ec8a7 in cmd_main git.c:923 #16 0x55ba6f581682 in main common-main.c:57 #17 0x7f2d08c3c28f (/usr/lib/libc.so.6+0x2328f) #18 0x7f2d08c3c349 in __libc_start_main (/usr/lib/libc.so.6+0x23349) #19 0x55ba6f2e60e4 in _start ../sysdeps/x86_64/start.S:115 SUMMARY: AddressSanitizer: heap-buffer-overflow pretty.c:1712 in format_and_pad_commit Shadow bytes around the buggy address: 0x0c067fff8120: fa fa fd fd fd fa fa fa fd fd fd fa fa fa fd fd 0x0c067fff8130: fd fd fa fa fd fd fd fd fa fa fd fd fd fa fa fa 0x0c067fff8140: fd fd fd fa fa fa fd fd fd fa fa fa fd fd fd fa 0x0c067fff8150: fa fa fd fd fd fd fa fa 00 00 00 fa fa fa fd fd 0x0c067fff8160: fd fa fa fa fd fd fd fa fa fa fd fd fd fa fa fa =>0x0c067fff8170: fd fd fd fa fa[fa]00 00 00 fa fa fa 00 00 00 fa 0x0c067fff8180: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c067fff8190: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c067fff81a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c067fff81b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c067fff81c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb Luckily enough, this would only cause us to copy the out-of-bounds data into the formatted commit in case we really had an ANSI sequence preceding our buffer. So this bug likely has no security consequences. Fix it regardless by not traversing past the buffer's start. Reported-by: Patrick Steinhardt <ps@pks.im> Reported-by: Eric Sesterhenn <eric.sesterhenn@x41-dsec.de> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-12-09pretty: fix out-of-bounds write caused by integer overflowPatrick Steinhardt1-0/+17
When using a padding specifier in the pretty format passed to git-log(1) we need to calculate the string length in several places. These string lengths are stored in `int`s though, which means that these can easily overflow when the input lengths exceeds 2GB. This can ultimately lead to an out-of-bounds write when these are used in a call to memcpy(3P): ==8340==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7f1ec62f97fe at pc 0x7f2127e5f427 bp 0x7ffd3bd63de0 sp 0x7ffd3bd63588 WRITE of size 1 at 0x7f1ec62f97fe thread T0 #0 0x7f2127e5f426 in __interceptor_memcpy /usr/src/debug/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:827 #1 0x5628e96aa605 in format_and_pad_commit pretty.c:1762 #2 0x5628e96aa7f4 in format_commit_item pretty.c:1801 #3 0x5628e97cdb24 in strbuf_expand strbuf.c:429 #4 0x5628e96ab060 in repo_format_commit_message pretty.c:1869 #5 0x5628e96acd0f in pretty_print_commit pretty.c:2161 #6 0x5628e95a44c8 in show_log log-tree.c:781 #7 0x5628e95a76ba in log_tree_commit log-tree.c:1117 #8 0x5628e922bed5 in cmd_log_walk_no_free builtin/log.c:508 #9 0x5628e922c35b in cmd_log_walk builtin/log.c:549 #10 0x5628e922f1a2 in cmd_log builtin/log.c:883 #11 0x5628e9106993 in run_builtin git.c:466 #12 0x5628e9107397 in handle_builtin git.c:721 #13 0x5628e9107b07 in run_argv git.c:788 #14 0x5628e91088a7 in cmd_main git.c:923 #15 0x5628e939d682 in main common-main.c:57 #16 0x7f2127c3c28f (/usr/lib/libc.so.6+0x2328f) #17 0x7f2127c3c349 in __libc_start_main (/usr/lib/libc.so.6+0x23349) #18 0x5628e91020e4 in _start ../sysdeps/x86_64/start.S:115 0x7f1ec62f97fe is located 2 bytes to the left of 4831838265-byte region [0x7f1ec62f9800,0x7f1fe62f9839) allocated by thread T0 here: #0 0x7f2127ebe7ea in __interceptor_realloc /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:85 #1 0x5628e98774d4 in xrealloc wrapper.c:136 #2 0x5628e97cb01c in strbuf_grow strbuf.c:99 #3 0x5628e97ccd42 in strbuf_addchars strbuf.c:327 #4 0x5628e96aa55c in format_and_pad_commit pretty.c:1761 #5 0x5628e96aa7f4 in format_commit_item pretty.c:1801 #6 0x5628e97cdb24 in strbuf_expand strbuf.c:429 #7 0x5628e96ab060 in repo_format_commit_message pretty.c:1869 #8 0x5628e96acd0f in pretty_print_commit pretty.c:2161 #9 0x5628e95a44c8 in show_log log-tree.c:781 #10 0x5628e95a76ba in log_tree_commit log-tree.c:1117 #11 0x5628e922bed5 in cmd_log_walk_no_free builtin/log.c:508 #12 0x5628e922c35b in cmd_log_walk builtin/log.c:549 #13 0x5628e922f1a2 in cmd_log builtin/log.c:883 #14 0x5628e9106993 in run_builtin git.c:466 #15 0x5628e9107397 in handle_builtin git.c:721 #16 0x5628e9107b07 in run_argv git.c:788 #17 0x5628e91088a7 in cmd_main git.c:923 #18 0x5628e939d682 in main common-main.c:57 #19 0x7f2127c3c28f (/usr/lib/libc.so.6+0x2328f) #20 0x7f2127c3c349 in __libc_start_main (/usr/lib/libc.so.6+0x23349) #21 0x5628e91020e4 in _start ../sysdeps/x86_64/start.S:115 SUMMARY: AddressSanitizer: heap-buffer-overflow /usr/src/debug/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:827 in __interceptor_memcpy Shadow bytes around the buggy address: 0x0fe458c572a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0fe458c572b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0fe458c572c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0fe458c572d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0fe458c572e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa =>0x0fe458c572f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa[fa] 0x0fe458c57300: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0fe458c57310: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0fe458c57320: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0fe458c57330: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0fe458c57340: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==8340==ABORTING The pretty format can also be used in `git archive` operations via the `export-subst` attribute. So this is what in our opinion makes this a critical issue in the context of Git forges which allow to download an archive of user supplied Git repositories. Fix this vulnerability by using `size_t` instead of `int` to track the string lengths. Add tests which detect this vulnerability when Git is compiled with the address sanitizer. Reported-by: Joern Schneeweisz <jschneeweisz@gitlab.com> Original-patch-by: Joern Schneeweisz <jschneeweisz@gitlab.com> Modified-by: Taylor Blau <me@ttalorr.com> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-12-09test-lib: add prerequisite for 64-bit platformsCarlo Marcelo Arenas Belón1-0/+4
Allow tests that assume a 64-bit `size_t` to be skipped in 32-bit platforms and regardless of the size of `long`. This imitates the `LONG_IS_64BIT` prerequisite. Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-12-09t1509: facilitate repeated script invocationsEric Sunshine1-0/+5
t1509-root-work-tree.sh, which tests behavior of a Git repository located at the root `/` directory, refuses to run if it detects the presence of an existing repository at `/`. This safeguard ensures that it won't clobber a legitimate repository at that location. However, because t1509 does a poor job of cleaning up after itself, it runs afoul of its own safety check on subsequent runs, which makes it painful to run the script repeatedly since each run requires manual cleanup of detritus from the previous run. Address this shortcoming by making t1509 clean up after itself as its last action. This is safe since the script can only make it to this cleanup action if it did not find a legitimate repository at `/` in the first place, so the resources cleaned up here can only have been created by the script itself. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
2022-12-09t1509: make "setup" test more robustEric Sunshine1-1/+1
One of the t1509 setup tests is very particular about the output it expects from `git init`, and fails if the output differs even slightly which can happen easily if the script is run multiple times since it doesn't do a good job of cleaning up after itself (i.e. it leaves detritus in the root directory `/`). One bit of cruft in particular (`/HEAD`) makes the test fail since its presence causes `git init` to alter its output; rather than reporting "Initialized empty Git repository", it instead reports "Reinitialized existing Git repository" when `/HEAD` is present. Address this problem by making the test do a more careful job of crafting its intended initial state. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
2022-12-09t1509: fix failing "root work tree" test due to owner-checkEric Sunshine1-1/+2
When 8959555cee (setup_git_directory(): add an owner check for the top-level directory, 2022-03-02) tightened security surrounding directory ownership, it neglected to adjust t1509-root-work-tree.sh to take the new restriction into account. As a result, since the root directory `/` is typically not owned by the user running the test (indeed, t1509 refuses to run as `root`), the ownership check added by 8959555cee kicks in and causes the test to fail: fatal: detected dubious ownership in repository at '/' To add an exception for this directory, call: git config --global --add safe.directory / This problem went unnoticed for so long because t1509 is rarely run since it requires setting up a `chroot` environment or a sacrificial virtual machine in which `/` can be made writable and polluted by any user. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
2022-12-07t3920: support CR-eating grepRené Scharfe1-1/+1
grep(1) converts CRLF line endings to LF on current MinGW: $ uname -sr MINGW64_NT-10.0-22621 3.3.6-341.x86_64 $ printf 'a\r\n' | hexdump.exe -C 00000000 61 0d 0a |a..| 00000003 $ printf 'a\r\n' | grep . | hexdump.exe -C 00000000 61 0a |a.| 00000002 Create the intended test file by grepping the original file with LF line endings and adding CRs explicitly. The missing CRs went unnoticed because test_cmp on MinGW ignores line endings since 4d715ac05c (Windows: a test_cmp that is agnostic to random LF <> CRLF conversions, 2013-10-26). Fix this test anyway to avoid depending on that special test_cmp behavior, especially since this is the only test that needs it. Piping the output of grep(1) through append_cr has the side-effect of ignoring its return value. That means we no longer need the explicit "|| true" to support commit messages without a body. Signed-off-by: René Scharfe <l.s.r@web.de> Acked-by: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-12-07t0021: use Windows-friendly `pwd`Johannes Schindelin1-2/+2
In Git for Windows, when passing paths from shell scripts to regular Win32 executables, thanks to the MSYS2 runtime a somewhat magic path conversion happens that lets the shell script think that there is a file at `/git/Makefile` and the Win32 process it spawned thinks that the shell script said `C:/git-sdk-64/git/Makefile` instead. This conversion is documented in detail over here: https://www.msys2.org/docs/filesystem-paths/#automatic-unix-windows-path-conversion As all automatic conversions, there are gaps. For example, to avoid mistaking command-line options like `/LOG=log.txt` (which are quite common in the Windows world) from being mistaken for a Unix-style absolute path, the MSYS2 runtime specifically exempts arguments containing a `=` character from that conversion. We are about to change `test_cmp` to use `git diff --no-index`, which involves spawning precisely such a Win32 process. In combination, this would cause a failure in `t0021-conversion.sh` where we pass an absolute path containing an equal character to the `test_cmp` function. Seeing as the Unix tools like `cp` and `diff` that are used by Git's test suite in the Git for Windows SDK (thanks to the MSYS2 project) understand both Unix-style as well as Windows-style paths, we can stave off this problem by simply switching to Windows-style paths and side-stepping the need for any automatic path conversion. Note: The `PATH` variable is obviously special, as it is colon-separated in the MSYS2 Bash used by Git for Windows, and therefore _cannot_ contain absolute Windows-style paths, lest the colon after the drive letter is mistaken for a path separator. Therefore, we need to be careful to keep the Unix-style when modifying the `PATH` variable. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-12-05attr: ignore overly large gitattributes filesPatrick Steinhardt1-0/+17
Similar as with the preceding commit, start ignoring gitattributes files that are overly large to protect us against out-of-bounds reads and writes caused by integer overflows. Unfortunately, we cannot just define "overly large" in terms of any preexisting limits in the codebase. Instead, we choose a very conservative limit of 100MB. This is plenty of room for specifying gitattributes, and incidentally it is also the limit for blob sizes for GitHub. While we don't want GitHub to dictate limits here, it is still sensible to use this fact for an informed decision given that it is hosting a huge set of repositories. Furthermore, over at GitLab we scanned a subset of repositories for their root-level attribute files. We found that 80% of them have a gitattributes file smaller than 100kB, 99.99% have one smaller than 1MB, and only a single repository had one that was almost 3MB in size. So enforcing a limit of 100MB seems to give us ample of headroom. With this limit in place we can be reasonably sure that there is no easy way to exploit the gitattributes file via integer overflows anymore. Furthermore, it protects us against resource exhaustion caused by allocating the in-memory data structures required to represent the parsed attributes. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-12-05attr: ignore attribute lines exceeding 2048 bytesPatrick Steinhardt1-2/+23
There are two different code paths to read gitattributes: once via a file, and once via the index. These two paths used to behave differently because when reading attributes from a file, we used fgets(3P) with a buffer size of 2kB. Consequentially, we silently truncate line lengths when lines are longer than that and will then parse the remainder of the line as a new pattern. It goes without saying that this is entirely unexpected, but it's even worse that the behaviour depends on how the gitattributes are parsed. While this is simply wrong, the silent truncation saves us with the recently discovered vulnerabilities that can cause out-of-bound writes or reads with unreasonably long lines due to integer overflows. As the common path is to read gitattributes via the worktree file instead of via the index, we can assume that any gitattributes file that had lines longer than that is already broken anyway. So instead of lifting the limit here, we can double down on it to fix the vulnerabilities. Introduce an explicit line length limit of 2kB that is shared across all paths that read attributes and ignore any line that hits this limit while printing a warning. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-12-05attr: fix silently splitting up lines longer than 2048 bytesPatrick Steinhardt1-0/+21
When reading attributes from a file we use fgets(3P) with a buffer size of 2048 bytes. This means that as soon as a line exceeds the buffer size we split it up into multiple parts and parse each of them as a separate pattern line. This is of course not what the user intended, and even worse the behaviour is inconsistent with how we read attributes from the index. Fix this bug by converting the code to use `strbuf_getline()` instead. This will indeed read in the whole line, which may theoretically lead to an out-of-memory situation when the gitattributes file is huge. We're about to reject any gitattributes files larger than 100MB in the next commit though, which makes this less of a concern. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-12-05t3920: don't ignore errors of more than one command with `|| true`Johannes Sixt1-1/+1
It is customary to write `A || true` to ignore a potential error exit of command A. But when we have a sequence `A && B && C || true && D`, then a failure of any of A, B, or C skips to D right away. This is not intended here. Turn the command whose failure is to be ignored into a compound command to ensure it is the only one that is allowed to fail. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-12-05t4023: fix ignored exit codes of gitÆvar Arnfjörð Bjarmason1-6/+6
Change a "git diff-tree" command to be &&-chained so that we won't ignore its exit code, see the ea05fd5fbf7 (Merge branch 'ab/keep-git-exit-codes-in-tests', 2022-03-16) topic for prior art. This fixes code added in b45563a229f (rename: Break filepairs with different types., 2007-11-30). Due to hiding the exit code we hid a memory leak under SANITIZE=leak. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-12-05t7600: don't ignore "rev-parse" exit code in helperÆvar Arnfjörð Bjarmason1-1/+1
Change the verify_mergeheads() helper the check the exit code of "git rev-parse". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-12-02t5314: check exit code of "git"Ævar Arnfjörð Bjarmason1-7/+10
Amend the test added in [1] to check the exit code of the "git" invocations. An in-flight change[2] introduced a memory leak in these invocations, which went undetected unless we were running under "GIT_TEST_SANITIZE_LEAK_LOG=true". Note that the in-flight change made 8 test files fail, but as far as I can tell only this one would have had its exit code hidden unless under "GIT_TEST_SANITIZE_LEAK_LOG=true". The rest would be caught without it. We could pick other variable names here than "ln%d", e.g. "commit", "dummy_blob" and "file_blob", but having the "rev-parse" invocations aligned makes the difference between them more readable, so let's pick "ln%d". 1. 4cf2143e029 (pack-objects: break delta cycles before delta-search phase, 2016-08-11) 2. https://lore.kernel.org/git/221128.868rjvmi3l.gmgdl@evledraar.gmail.com/ 3. faececa53f9 (test-lib: have the "check" mode for SANITIZE=leak consider leak logs, 2022-07-28) Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-12-02t4205: don't exit test script on failureRené Scharfe1-1/+1
Only abort the individual check instead of exiting the whole test script if git show fails. Noticed with GIT_TEST_PASSING_SANITIZE_LEAK=check. Helped-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-12-01status: modernize git-status "slow untracked files" adviceRudy Rigot1-0/+70
`git status` can be slow when there are a large number of untracked files and directories since Git must search the entire worktree to enumerate them. When it is too slow, Git prints advice with the elapsed search time and a suggestion to disable the search using the `-uno` option. This suggestion also carries a warning that might scare off some users. However, these days, `-uno` isn't the only option. Git can reduce the time taken to enumerate untracked files by caching results from previous `git status` invocations, when the `core.untrackedCache` and `core.fsmonitor` features are enabled. Update the `git status` man page to explain these configuration options, and update the advice to provide more detail about the current configuration and to refer to the updated documentation. Signed-off-by: Rudy Rigot <rudy.rigot@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-11-30t1301: do not change $CWD in "shared=all" test caseJiang Xin1-2/+1
In test case "shared=all", the working directory is permanently changed to the "sub" directory. This leads to a strange behavior that the temporary repositories created by subsequent test cases are all in this "sub" directory, such as "sub/new", "sub/child.git". If we bypass this test case, all subsequent test cases will have different working directory. Besides, all subsequent test cases assuming they are in the "sub" directory do not run any destructive operations in their parent directory (".."), and will not make damage out side of $TRASH_DIRECTORY. So it is a safe change for us to run the test case "shared=all" in current repository instead of creating and changing to "sub". For the next test case, the path ".git/info" is assumed to be missing, but we no longer run the test case in the "sub" repository which is initialized from an empty template. In order for the test case to run properly, we can set "TEST_CREATE_REPO_NO_TEMPLATE=1" to initialize the default repository without a template. Helped-by: Junio C Hamano <gitster@pobox.com> Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-11-30t1301: use test_when_finished for cleanupJiang Xin1-3/+5
Refactor several test cases to use "test_when_finished" for cleanup. 1. For first of these, we used to clean-up outside the test, but instead let's use test_when_finished for that. 2. For the second, we used to leave "new" after we are done, but not use it at all later. Now we do clean up. 3. For the rest, these child.git test repositories used to follow "initialize what we are going to use to a known state before we use" pattern, which is not wrong per-se, but now we use "clean up the cruft we made after we are done" pattern, which may arguably be better simply because the test that makes cruft should know what cruft it created better than whatever comes later that may not know. Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-11-30t1301: fix wrong template dir for git-initJiang Xin1-1/+2
The template dir prepared in test case "forced modes" is not used as expected because a wrong template dir is provided to "git init". This is because the $CWD for "git-init" command is a sibling directory alongside the template directory. Change it to the right template directory and add a protection test using "test_path_is_file". The wrong template directory was introduced by mistake in commit e1df7fe43f (init: make --template path relative to $CWD, 2019-05-10). Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-11-30pack-objects: fix handling of multiple --filter optionsRené Scharfe1-1/+1
Since 5cb28270a1 (pack-objects: lazily set up "struct rev_info", don't leak, 2022-03-28) --filter options given to git pack-objects overrule earlier ones, letting only the leftmost win and leaking the memory allocated for earlier ones. Fix that by only initializing the rev_info struct once. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-11-30t5317: demonstrate failure to handle multiple --filter optionsRené Scharfe1-0/+38
git pack-objects should accept multiple --filter options as documented in Documentation/rev-list-options.txt, but currently the last one wins. Show that using tests with multiple blob size limits Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-11-30t5317: stop losing return codes of git ls-filesRené Scharfe1-24/+28
fb2d0db502 (test-lib-functions: add parsing helpers for ls-files and ls-tree, 2022-04-04) not only started to use helper functions, it also started to pipe the output of git ls-files into them directly, without using a temporary file. No explanation was given. This causes the return code of that git command to be ignored. Revert that part of the change, use temporary files and check the return code of git ls-files again. Suggested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-11-30completion: add case-insensitive match of pseudorefsAlison Winters1-0/+15
When GIT_COMPLETION_IGNORE_CASE is set, also allow lowercase completion text like "head" to match uppercase HEAD and other pseudorefs. Signed-off-by: Alison Winters <alisonatwork@outlook.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-11-30completion: add optional ignore-case when matching refsAlison Winters1-0/+15
If GIT_COMPLETION_IGNORE_CASE is set, --ignore-case will be added to git for-each-ref calls so that refs can be matched case insensitively, even when running on case sensitive filesystems. Signed-off-by: Alison Winters <alisonatwork@outlook.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-11-29Merge branch 'jh/trace2-timers-and-counters'Junio C Hamano1-2/+2
Test fix. * jh/trace2-timers-and-counters: trace2 tests: guard pthread test with "PTHREAD"
2022-11-29Merge branch 'ah/chainlint-cpuinfo-parse-fix'Junio C Hamano1-1/+1
The format of a line in /proc/cpuinfo that describes a CPU on s390x looked different from everybody else, and the code in chainlint.pl failed to parse it. * ah/chainlint-cpuinfo-parse-fix: chainlint.pl: fix /proc/cpuinfo regexp
2022-11-29Merge branch 'gc/resolve-alternate-symlinks'Junio C Hamano1-0/+16
Resolve symbolic links when processing the locations of alternate object stores, since failing to do so can lead to confusing and buggy behavior. * gc/resolve-alternate-symlinks: object-file: use real paths when adding alternates
2022-11-28Merge branch 'ab/fewer-the-index-macros'Junio C Hamano10-26/+33
Progress on removing 'the_index' convenience wrappers. * ab/fewer-the-index-macros: cocci: apply "pending" index-compatibility to some "builtin/*.c" cache.h & test-tool.h: add & use "USE_THE_INDEX_VARIABLE" {builtin/*,repository}.c: add & use "USE_THE_INDEX_VARIABLE" cocci: apply "pending" index-compatibility to "t/helper/*.c" cocci & cache.h: apply variable section of "pending" index-compatibility cocci & cache.h: apply a selection of "pending" index-compatibility cocci: add a index-compatibility.pending.cocci read-cache API & users: make discard_index() return void cocci & cache.h: remove rarely used "the_index" compat macros builtin/{grep,log}.: don't define "USE_THE_INDEX_COMPATIBILITY_MACROS" cache.h: remove unused "the_index" compat macros
2022-11-28Merge branch 'es/locate-httpd-module-location-in-test'Junio C Hamano1-1/+2
Add one more candidate directory that may house httpd modules while running tests. * es/locate-httpd-module-location-in-test: lib-httpd: extend module location auto-detection
2022-11-28Merge branch 'zk/push-use-bitmaps'Junio C Hamano1-3/+3
Test fix. * zk/push-use-bitmaps: t5516: fail to run in verbose mode
2022-11-28Merge branch 'ew/prune-with-missing-objects-pack'Junio C Hamano1-0/+8
"git prune" may try to iterate over .git/objects/pack for trash files to remove in it, and loudly fail when the directory is missing, which is not necessary. The command has been taught to ignore such a failure. * ew/prune-with-missing-objects-pack: prune: quiet ENOENT on missing directories
2022-11-28Merge branch 'pw/config-int-parse-fixes'Junio C Hamano3-0/+24
Assorted fixes of parsing end-user input as integers. * pw/config-int-parse-fixes: git_parse_signed(): avoid integer overflow config: require at least one digit when parsing numbers git_parse_unsigned: reject negative values
2022-11-28Merge branch 'jk/parse-object-type-mismatch'Junio C Hamano1-2/+2
`parse_object()` hardening when checking for the existence of a suspected blob object. * jk/parse-object-type-mismatch: parse_object(): simplify blob conditional parse_object(): check on-disk type of suspected blob parse_object(): drop extra "has" check before checking object type
2022-11-27var: allow GIT_EDITOR to return nullSean Allred1-0/+62
The handling to die early when there is no EDITOR is valuable when used in normal code (i.e., editor.c). In git-var, where null/empty-string is a perfectly valid value to return, it doesn't make as much sense. Remove this handling from `git var GIT_EDITOR` so that it does not fail so noisily when there is no defined editor. Signed-off-by: Sean Allred <allred.sean@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-11-25object-file: use real paths when adding alternatesGlen Choo1-0/+16
When adding an alternate ODB, we check if the alternate has the same path as the object dir, and if so, we do nothing. However, that comparison does not resolve symlinks. This makes it possible to add the object dir as an alternate, which may result in bad behavior. For example, it can trick "git repack -a -l -d" (possibly run by "git gc") into thinking that all packs come from an alternate and delete all objects. rm -rf test && git clone https://github.com/git/git test && ( cd test && ln -s objects .git/alt-objects && # -c repack.updateserverinfo=false silences a warning about not # being able to update "info/refs", it isn't needed to show the # bad behavior GIT_ALTERNATE_OBJECT_DIRECTORIES=".git/alt-objects" git \ -c repack.updateserverinfo=false repack -a -l -d && # It's broken! git status # Because there are no more objects! ls .git/objects/pack ) Fix this by resolving symlinks and relative paths before comparing the alternate and object dir. This lets us clean up a number of issues noted in 37a95862c6 (alternates: re-allow relative paths from environment, 2016-11-07): - Now that we compare the real paths, duplicate detection is no longer foiled by relative paths. - Using strbuf_realpath() allows us to "normalize" paths that strbuf_normalize_path() can't, so we can stop silently ignoring errors when "normalizing" paths from the environment. - We now store an absolute path based on getcwd() (the "future direction" named in 37a95862c6), so chdir()-ing in the process no longer changes the directory pointed to by the alternate. This is a change in behavior, but a desirable one. Signed-off-by: Glen Choo <chooglen@google.com> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-11-25trace2 tests: guard pthread test with "PTHREAD"Ævar Arnfjörð Bjarmason1-2/+2
Since 81071626ba1 (trace2: add global counter mechanism, 2022-10-24) these tests have been failing when git is compiled with NO_PTHREADS=Y, which is always the case e.g. if 'uname -s' is "NONSTOP_KERNEL". Reported-by: Randall S. Becker <randall.becker@nexbridge.ca> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-11-23Merge branch 'ps/receive-use-only-advertised'Junio C Hamano2-0/+203
"git receive-pack" used to use all the local refs as the boundary for checking connectivity of the data "git push" sent, but now it uses only the refs that it advertised to the pusher. In a repository with the .hideRefs configuration, this reduces the resources needed to perform the check. cf. <221028.86bkpw805n.gmgdl@evledraar.gmail.com> cf. <xmqqr0yrizqm.fsf@gitster.g> * ps/receive-use-only-advertised: receive-pack: only use visible refs for connectivity check rev-parse: add `--exclude-hidden=` option revision: add new parameter to exclude hidden refs revision: introduce struct to handle exclusions revision: move together exclusion-related functions refs: get rid of global list of hidden refs refs: fix memory leak when parsing hideRefs config
2022-11-23Merge branch 'jt/submodule-on-demand'Junio C Hamano1-0/+50
Push all submodules recursively with '--recurse-submodules=on-demand'. * jt/submodule-on-demand: Doc: document push.recurseSubmodules=only
2022-11-23Merge branch 'es/chainlint-lineno'Junio C Hamano2-23/+49
Teach chainlint.pl to show corresponding line numbers when printing the source of a test. * es/chainlint-lineno: chainlint: prefix annotated test definition with line numbers chainlint: latch line numbers at which each token starts and ends chainlint: sidestep impoverished macOS "terminfo"
2022-11-23Merge branch 'ab/t7610-timeout'Junio C Hamano1-2/+2
Fix a source of flakiness in CI when compiling with SANITIZE=leak. * ab/t7610-timeout: t7610: use "file:///dev/null", not "/dev/null", fixes MinGW t7610: fix flaky timeout issue, don't clone from example.com
2022-11-23Merge branch 'rp/maintenance-qol'Junio C Hamano2-1/+26
'git maintenance register' is taught to write configuration to an arbitrary path, and 'git for-each-repo' is taught to expand tilde characters in paths. * rp/maintenance-qol: builtin/gc.c: fix use-after-free in maintenance_unregister() maintenance --unregister: fix uninit'd data use & -Wdeclaration-after-statement maintenance: add option to register in a specific config for-each-repo: interpolate repo path arguments
2022-11-23Merge branch 'pw/strict-label-lookups'Junio C Hamano1-0/+17
Correct an error where `git rebase` would mistakenly use a branch or tag named "refs/rewritten/xyz" when missing a rebase label. * pw/strict-label-lookups: sequencer: tighten label lookups sequencer: unify label lookup
2022-11-23Merge branch 'gc/redact-h2h3-headers'Junio C Hamano4-5/+36
Redact headers from cURL's h2h3 module in GIT_CURL_VERBOSE and others. * gc/redact-h2h3-headers: http: redact curl h2h3 headers in info t: run t5551 tests with both HTTP and HTTP/2
2022-11-23Merge branch 'es/chainlint-output'Junio C Hamano22-66/+206
Teach chainlint.pl to annotate the original test definition instead of the token stream. * es/chainlint-output: chainlint: annotate original test definition rather than token stream chainlint: latch start/end position of each token chainlint: tighten accuracy when consuming input stream chainlint: add explanatory comments
2022-11-23Merge branch 'js/remove-stale-scalar-repos'Junio C Hamano1-0/+14
'scalar reconfigure -a' is taught to automatically remove scalar.repo entires which no longer exist. * js/remove-stale-scalar-repos: tests(scalar): tighten the stale `scalar.repo` test some scalar reconfigure -a: remove stale `scalar.repo` entries
2022-11-23Merge branch 'dd/bisect-helper-subcommand'Junio C Hamano1-0/+10
Fix a regression in the bisect-helper which mistakenly treats arguments to the command given to 'git bisect run' as arguments to the helper. * dd/bisect-helper-subcommand: bisect--helper: parse subcommand with OPT_SUBCOMMAND bisect--helper: move all subcommands into their own functions bisect--helper: remove unused options
2022-11-23Merge branch 'ab/submodule-helper-prep-only'Junio C Hamano6-20/+289
Preparation to remove git-submodule.sh and replace it with a builtin. * ab/submodule-helper-prep-only: submodule--helper: use OPT_SUBCOMMAND() API submodule--helper: drop "update --prefix <pfx>" for "-C <pfx> update" submodule--helper: remove --prefix from "absorbgitdirs" submodule API & "absorbgitdirs": remove "----recursive" option submodule.c: refactor recursive block out of absorb function submodule tests: test for a "foreach" blind-spot submodule--helper: fix a memory leak in "status" submodule tests: add tests for top-level flag output submodule--helper: move "config" to a test-tool
2022-11-23chainlint.pl: fix /proc/cpuinfo regexpAndreas Hasenack1-1/+1
29fb2ec3 (chainlint.pl: validate test scripts in parallel, 2022-09-01) introduced a function that gets the number of cores from /proc/cpuinfo on some systems, notably linux. The regexp it uses (^processor\s*:) fails to match the desired lines in the s390x architecture, where they look like this: processor 0: version = FF, identification = 148F67, machine = 2964 As a result, on s390x that function returns 0 as the number of cores, and the chainlint.pl script exits without doing anything. Signed-off-by: Andreas Hasenack <andreas.hasenack@canonical.com> Acked-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
generated by cgit v1.2.3 (git 2.39.1) at 2025-12-25 18:13:46 +0000