Skip to content

Commit 2c6cf4e

Browse files
szederdscho
authored andcommitted
pickaxe: fix segfault with '-S<...> --pickaxe-regex'
'git {log,diff,...} -S<...> --pickaxe-regex' can segfault as a result of out-of-bounds memory reads. diffcore-pickaxe.c:contains() looks for all matches of the given regex in a buffer in a loop, advancing the buffer pointer to the end of the last match in each iteration. When we switched to REG_STARTEND in b7d36ff (regex: use regexec_buf(), 2016-09-21), we started passing the size of that buffer to the regexp engine, too. Unfortunately, this buffer size is never updated on subsequent iterations, and as the buffer pointer advances on each iteration, this "bufptr+bufsize" points past the end of the buffer. This results in segmentation fault, if that memory can't be accessed. In case of 'git log' it can also result in erroneously listed commits, if the memory past the end of buffer is accessible and happens to contain data matching the regex. Reduce the buffer size on each iteration as the buffer pointer is advanced, thus maintaining the correct end of buffer location. Furthermore, make sure that the buffer pointer is not dereferenced in the control flow statements when we already reached the end of the buffer. The new test is flaky, I've never seen it fail on my Linux box even without the fix, but this is expected according to db5dfa3 (regex: -G<pattern> feeds a non NUL-terminated string to regexec() and fails, 2016-09-21). However, it did fail on Travis CI with the first (and incomplete) version of the fix, and based on that commit message I would expect the new test without the fix to fail most of the time on Windows. Signed-off-by: SZEDER Gábor <[email protected]> Signed-off-by: Johannes Schindelin <[email protected]>
1 parent 24e8a39 commit 2c6cf4e

File tree

2 files changed

+10
-2
lines changed

2 files changed

+10
-2
lines changed

diffcore-pickaxe.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,12 +81,15 @@ static unsigned int contains(mmfile_t *mf, regex_t *regexp, kwset_t kws)
8181
regmatch_t regmatch;
8282
int flags = 0;
8383

84-
while (*data &&
84+
while (sz && *data &&
8585
!regexec_buf(regexp, data, sz, 1, &regmatch, flags)) {
8686
flags |= REG_NOTBOL;
8787
data += regmatch.rm_eo;
88-
if (*data && regmatch.rm_so == regmatch.rm_eo)
88+
sz -= regmatch.rm_eo;
89+
if (sz && *data && regmatch.rm_so == regmatch.rm_eo) {
8990
data++;
91+
sz--;
92+
}
9093
cnt++;
9194
}
9295

t/t4062-diff-pickaxe.sh

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,4 +19,9 @@ test_expect_success '-G matches' '
1919
test 4096-zeroes.txt = "$(cat out)"
2020
'
2121

22+
test_expect_success '-S --pickaxe-regex' '
23+
git diff --name-only -S0 --pickaxe-regex HEAD^ >out &&
24+
verbose test 4096-zeroes.txt = "$(cat out)"
25+
'
26+
2227
test_done

0 commit comments

Comments
 (0)