Skip to content

Commit 0a0cd67

Browse files
jorioEdward Thomson
authored andcommitted
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 parent 1d811f0 commit 0a0cd67

File tree

2 files changed

+89
-3
lines changed

2 files changed

+89
-3
lines changed

src/diff_file.c

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -328,16 +328,25 @@ static int diff_file_content_load_workdir_file(
328328
git_filter_list *fl = NULL;
329329
git_file fd = git_futils_open_ro(git_str_cstr(path));
330330
git_str raw = GIT_STR_INIT;
331+
git_object_size_t new_file_size = 0;
331332

332333
if (fd < 0)
333334
return fd;
334335

335-
if (!fc->file->size)
336-
error = git_futils_filesize(&fc->file->size, fd);
336+
error = git_futils_filesize(&new_file_size, fd);
337337

338-
if (error < 0 || !fc->file->size)
338+
if (error < 0 || !new_file_size)
339339
goto cleanup;
340340

341+
/* if file size doesn't match cached value, abort */
342+
if (fc->file->size && fc->file->size != new_file_size)
343+
{
344+
error = -1;
345+
goto cleanup;
346+
}
347+
348+
fc->file->size = new_file_size;
349+
341350
if ((diff_opts->flags & GIT_DIFF_SHOW_BINARY) == 0 &&
342351
diff_file_content_binary_by_size(fc))
343352
goto cleanup;

tests/diff/externalmodifications.c

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
#include "clar_libgit2.h"
2+
#include "../checkout/checkout_helpers.h"
3+
4+
#include "index.h"
5+
#include "repository.h"
6+
7+
static git_repository *g_repo;
8+
9+
void test_diff_externalmodifications__initialize(void)
10+
{
11+
g_repo = cl_git_sandbox_init("testrepo2");
12+
}
13+
14+
void test_diff_externalmodifications__cleanup(void)
15+
{
16+
cl_git_sandbox_cleanup();
17+
g_repo = NULL;
18+
}
19+
20+
void test_diff_externalmodifications__file_becomes_smaller(void)
21+
{
22+
git_index *index;
23+
git_diff *diff;
24+
git_patch* patch;
25+
git_str path = GIT_STR_INIT;
26+
char big_string[500001];
27+
28+
cl_git_pass(git_str_joinpath(&path, git_repository_workdir(g_repo), "README"));
29+
30+
/* Modify the file with a large string */
31+
memset(big_string, '\n', sizeof(big_string) - 1);
32+
big_string[sizeof(big_string) - 1] = '\0';
33+
cl_git_mkfile(path.ptr, big_string);
34+
35+
/* Get a diff */
36+
cl_git_pass(git_repository_index(&index, g_repo));
37+
cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, index, NULL));
38+
cl_assert_equal_i(1, git_diff_num_deltas(diff));
39+
cl_assert_equal_i(500000, git_diff_get_delta(diff, 0)->new_file.size);
40+
41+
/* Simulate file modification after we've gotten the diff.
42+
* Write a shorter string to ensure that we don't mmap 500KB from
43+
* the previous revision, which would most likely crash. */
44+
cl_git_mkfile(path.ptr, "hello");
45+
46+
/* Attempt to get a patch */
47+
cl_git_fail(git_patch_from_diff(&patch, diff, 0));
48+
49+
git_index_free(index);
50+
git_diff_free(diff);
51+
git_str_dispose(&path);
52+
}
53+
54+
void test_diff_externalmodifications__file_deleted(void)
55+
{
56+
git_index *index;
57+
git_diff *diff;
58+
git_patch* patch;
59+
git_str path = GIT_STR_INIT;
60+
61+
cl_git_pass(git_str_joinpath(&path, git_repository_workdir(g_repo), "README"));
62+
63+
/* Get a diff */
64+
cl_git_pass(git_repository_index(&index, g_repo));
65+
cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, index, NULL));
66+
cl_assert_equal_i(0, git_diff_num_deltas(diff));
67+
68+
/* Delete the file */
69+
cl_git_rmfile(path.ptr);
70+
71+
/* Attempt to get a patch */
72+
cl_git_fail(git_patch_from_diff(&patch, diff, 0));
73+
74+
git_index_free(index);
75+
git_diff_free(diff);
76+
git_str_dispose(&path);
77+
}

0 commit comments

Comments
 (0)