[vim/vim] Potential null pointer dereference in vim (Issue #18437)
youngmith
lionheartys created an issue (vim/vim#18437)Steps to reproduce
Using the vim build compiled with Clang-LLVM 16 (version 16)
Configuration options: ./configure --with-features=huge --enable-gui=none
After the compilation is complete, use the binary executable file /vim/src/vim to execute: vim -u NONE -i NONE -X -Z -m -n -e -s -S POC -c :qa!
Expected behaviour
Vim should not crash. The code should safely handle any null pointer situations and prevent segmentation faults.
Version of Vim
9.1.1792
Environment
OS:Ubuntu 22.04.5 LTS
$TERM: xterm
Shell: GNU bash, version 5.1.16(1)-release (x86_64-pc-linux-gnu)
Logs and stack traces
The crash is triggered by a null pointer dereference during normal command execution. The issue seems to originate from the /src/diff.c:calculate_topfill_and_topline() function when accessing the thistopdiff pointer, which can be NULL. When I use GDB to debug this crash, the call stack and the crash part are shown in the attached figure(The information in the Logs and stack traces section cannot be attached. I can only put the gdb debugging process in the Steps to reproduce section. I'm sorry.)
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
youngmith
lionheartys left a comment (vim/vim#18437)The POC used to reproduce the crash is the following attachment
POC.zip
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
phanium
phanen left a comment (vim/vim#18437)minimal repro, which just recursively create a lots of diff windows
diffs a edit POC file b exe "norm! \<C-w>\<C-w>" exe "norm! \<C-w>\<C-w>" exe "norm! \<C-w>\<C-w>" exe "norm! \<C-w>\<C-w>" exe "norm! \<C-w>\<C-w>" exe "norm! \<C-w>\L" exe "norm! \<C-j>oy\<C-j>" edit POC sil!so
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#18437)Please next time, paste the stacktrace here as text and not as a picture.
It seems this patch fixes it:
diff --git a/src/diff.c b/src/diff.c index 25b9ae977..ec0640c56 100644 --- a/src/diff.c +++ b/src/diff.c @@ -2226,6 +2226,7 @@ calculate_topfill_and_topline( diff_T *thistopdiff = NULL; diff_T *next_adjacent_blocks = NULL; int virtual_lines_passed = 0; + int curlinenum_to = 1; find_top_diff_block(&thistopdiff, &next_adjacent_blocks, fromidx, from_topline); @@ -2253,7 +2254,8 @@ calculate_topfill_and_topline( // move the same amount of virtual lines in the target buffer to find the // cursor's line number - int curlinenum_to = thistopdiff->df_lnum[toidx]; + if (thistopdiff != NULL) + curlinenum_to = thistopdiff->df_lnum[toidx]; int virt_lines_left = virtual_lines_passed; curdif = thistopdiff;
ping @ychin
BTW @phanen I cannot reproduce it using your recipe.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
youngmith
lionheartys left a comment (vim/vim#18437)Please next time, paste the stacktrace here as text and not as a picture.
It seems this patch fixes it:
diff --git a/src/diff.c b/src/diff.c
index 25b9ae977..ec0640c56 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -2226,6 +2226,7 @@ calculate_topfill_and_topline(
diff_T *thistopdiff = NULL;
diff_T *next_adjacent_blocks = NULL;
int virtual_lines_passed = 0;
int curlinenum_to = 1;
find_top_diff_block(&thistopdiff, &next_adjacent_blocks, fromidx, from_topline);
@@ -2253,7 +2254,8 @@ calculate_topfill_and_topline(
// move the same amount of virtual lines in the target buffer to find the // cursor's line number
- int curlinenum_to = thistopdiff->df_lnum[toidx];
if (thistopdiff != NULL)
curlinenum_to = thistopdiff->df_lnum[toidx];int virt_lines_left = virtual_lines_passed;
curdif = thistopdiff;
ping @ychin
BTW @phanen I cannot reproduce it using your recipe.
OK, I'll pay attention to it next time I submit an issue. Also, thank you for your reply, so this is indeed an existing bug?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
youngmith
lionheartys left a comment (vim/vim#18437)@chrisbra Thank you for your prompt response and the patch you provided. I will make sure to follow your advice and paste the stacktrace as text next time, rather than as a picture.
Additionally, I was wondering if it would be possible to assign a CVE to this bug, given that it may potentially affect the stability of Vim in certain scenarios. I would greatly appreciate your input on whether this could be considered for CVE allocation.
Thanks again for your time and support.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#18437)No, I don't consider this security relevant. It's a simple null pointer dereference and a crash. I am not going to get a CVE assigned.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Yee Cheng Chin
ychin left a comment (vim/vim#18437)Taking a look. The null check seems fine but just trying to make sure it is not masking some logic bug here.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.