[PATCH] pickaxe: fix segfault with '-S<...> --pickaxe-regex'
- From
SZEDER Gábor <szeder.dev@gmail.com>
- Date
- Mar 18, 2017, 15:12 UTC
- Message-ID
- <20170318151239.17196-1-szeder.dev@gmail.com>
'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 b7d36ffca (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.
Make sure that the buffer size is reduced on each iteration as the buffer pointer is advanced, thus maintaining the correct end of buffer location.
The new test is flaky, I've never seen it fail on my Linux box, but this is expected according to db5dfa331 (regex: -G<pattern> feeds a non NUL-terminated string to regexec() and fails, 2016-09-21). And based on that commit message I would expect the new test without the fix to fail reliably on Windows.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> ---
diffcore-pickaxe.c | 5 ++++- t/t4062-diff-pickaxe.sh | 5 +++++ 2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c index 9795ca1c1..03f84b714 100644 --- a/diffcore-pickaxe.c +++ b/diffcore-pickaxe.c @@ -85,8 +85,11 @@ static unsigned int contains(mmfile_t *mf, regex_t *regexp, kwset_t kws) !regexec_buf(regexp, data, sz, 1, ®match, flags)) { flags |= REG_NOTBOL; data += regmatch.rm_eo; - if (*data && regmatch.rm_so == regmatch.rm_eo) + sz -= regmatch.rm_eo; + if (*data && regmatch.rm_so == regmatch.rm_eo) { data++; + sz--; + } cnt++; } diff --git a/t/t4062-diff-pickaxe.sh b/t/t4062-diff-pickaxe.sh index f0bf50bda..7c4903f49 100755 --- a/t/t4062-diff-pickaxe.sh +++ b/t/t4062-diff-pickaxe.sh @@ -19,4 +19,9 @@ test_expect_success '-G matches' ' test 4096-zeroes.txt = "$(cat out)" ' +test_expect_success '-S --pickaxe-regex' ' + git diff --name-only -S0 --pickaxe-regex HEAD^ >out && + verbose test 4096-zeroes.txt = "$(cat out)" +' + test_done
-- 2.12.0.377.g15f6ffe90