Commit 0a0cd67dcbc9406f57de17be5d1dde25897677dd
2022-02-08T20:18:15
diff_file: fix crash if size of diffed file changes in workdir "diff_file_content_load_workdir_file()" maps a file from the workdir into memory. It uses git_diff_file.size to determine the size of the memory mapping. If this value goes stale, the mmaped area would be sized incorrectly. This could occur if an external program changes the contents of the file after libgit2 had cached its size. This used to segfault if the file becomes smaller (mmaped area too large). This patch causes diff_file_content_load_workdir_file to fail without crashing if it detects that the file size has changed.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116
diff --git a/src/diff_file.c b/src/diff_file.c
index 78e332f..0a5ded7 100644
--- a/src/diff_file.c
+++ b/src/diff_file.c
@@ -328,16 +328,25 @@ static int diff_file_content_load_workdir_file(
git_filter_list *fl = NULL;
git_file fd = git_futils_open_ro(git_str_cstr(path));
git_str raw = GIT_STR_INIT;
+ git_object_size_t new_file_size = 0;
if (fd < 0)
return fd;
- if (!fc->file->size)
- error = git_futils_filesize(&fc->file->size, fd);
+ error = git_futils_filesize(&new_file_size, fd);
- if (error < 0 || !fc->file->size)
+ if (error < 0 || !new_file_size)
goto cleanup;
+ /* if file size doesn't match cached value, abort */
+ if (fc->file->size && fc->file->size != new_file_size)
+ {
+ error = -1;
+ goto cleanup;
+ }
+
+ fc->file->size = new_file_size;
+
if ((diff_opts->flags & GIT_DIFF_SHOW_BINARY) == 0 &&
diff_file_content_binary_by_size(fc))
goto cleanup;
diff --git a/tests/diff/externalmodifications.c b/tests/diff/externalmodifications.c
new file mode 100644
index 0000000..d18e1fe
--- /dev/null
+++ b/tests/diff/externalmodifications.c
@@ -0,0 +1,77 @@
+#include "clar_libgit2.h"
+#include "../checkout/checkout_helpers.h"
+
+#include "index.h"
+#include "repository.h"
+
+static git_repository *g_repo;
+
+void test_diff_externalmodifications__initialize(void)
+{
+ g_repo = cl_git_sandbox_init("testrepo2");
+}
+
+void test_diff_externalmodifications__cleanup(void)
+{
+ cl_git_sandbox_cleanup();
+ g_repo = NULL;
+}
+
+void test_diff_externalmodifications__file_becomes_smaller(void)
+{
+ git_index *index;
+ git_diff *diff;
+ git_patch* patch;
+ git_str path = GIT_STR_INIT;
+ char big_string[500001];
+
+ cl_git_pass(git_str_joinpath(&path, git_repository_workdir(g_repo), "README"));
+
+ /* Modify the file with a large string */
+ memset(big_string, '\n', sizeof(big_string) - 1);
+ big_string[sizeof(big_string) - 1] = '\0';
+ cl_git_mkfile(path.ptr, big_string);
+
+ /* Get a diff */
+ cl_git_pass(git_repository_index(&index, g_repo));
+ cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, index, NULL));
+ cl_assert_equal_i(1, git_diff_num_deltas(diff));
+ cl_assert_equal_i(500000, git_diff_get_delta(diff, 0)->new_file.size);
+
+ /* Simulate file modification after we've gotten the diff.
+ * Write a shorter string to ensure that we don't mmap 500KB from
+ * the previous revision, which would most likely crash. */
+ cl_git_mkfile(path.ptr, "hello");
+
+ /* Attempt to get a patch */
+ cl_git_fail(git_patch_from_diff(&patch, diff, 0));
+
+ git_index_free(index);
+ git_diff_free(diff);
+ git_str_dispose(&path);
+}
+
+void test_diff_externalmodifications__file_deleted(void)
+{
+ git_index *index;
+ git_diff *diff;
+ git_patch* patch;
+ git_str path = GIT_STR_INIT;
+
+ cl_git_pass(git_str_joinpath(&path, git_repository_workdir(g_repo), "README"));
+
+ /* Get a diff */
+ cl_git_pass(git_repository_index(&index, g_repo));
+ cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, index, NULL));
+ cl_assert_equal_i(0, git_diff_num_deltas(diff));
+
+ /* Delete the file */
+ cl_git_rmfile(path.ptr);
+
+ /* Attempt to get a patch */
+ cl_git_fail(git_patch_from_diff(&patch, diff, 0));
+
+ git_index_free(index);
+ git_diff_free(diff);
+ git_str_dispose(&path);
+}