From 113a9392ff833e71cf12416da46c0f16ae53b9c7 Mon Sep 17 00:00:00 2001 From: Johnothan King Date: Tue, 30 Mar 2021 03:25:20 -0700 Subject: [PATCH] Fix vi mode crashes when going back one word (#246) This bug was originally reported at . A crash can occur when using the 'b' or 'B' vi mode commands to go back one word. I was able to reproduce these crashes with 100% consistency on an OpenBSD virtual machine when ksh is compiled with -D_std_malloc. Reproducer: $ set -o vi $ asdf The fix is based on Matthew DeVore's analysis: > I suspect this is caused by this line: >> while (vi_isalph(tcur_virt) && tcur_virt >= first_virt) --tcur_virt; > which is in the b codepath. It checks vi_isalph(tcur_virt) before checking > if tcur_virt is in range. These two clauses should be reversed. Note that > line 316 is a similar check for pressing B, and there the tcur_virt value > is checked first. src/cmd/ksh93/edit/vi.c: - Check tcur_virt before using isalph() or isblank() to fix both crashes. At the start of the backword() while loop this check was performed twice, so the redundant check has been removed. src/cmd/ksh93/tests/pty.sh: - Add a regression test for the b, B, w and W editor commands. --- NEWS | 5 +++++ src/cmd/ksh93/edit/vi.c | 34 +++++++++++++++------------------ src/cmd/ksh93/include/version.h | 2 +- src/cmd/ksh93/tests/pty.sh | 13 +++++++++++++ 4 files changed, 34 insertions(+), 20 deletions(-) diff --git a/NEWS b/NEWS index 79afb8211d4c..7daba1bc9d47 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,11 @@ For full details, see the git log at: https://github.com/ksh93/ksh Any uppercase BUG_* names are modernish shell bug IDs. +2021-03-29: + +- Fixed an intermittent crash that could occur in vi mode when using the 'b' + or 'B' commands to go back one word. + 2021-03-27: - The 'test' builtin will now show an error message when given the invalid ']]' diff --git a/src/cmd/ksh93/edit/vi.c b/src/cmd/ksh93/edit/vi.c index 8196ffca047c..59f0ab16a2cc 100644 --- a/src/cmd/ksh93/edit/vi.c +++ b/src/cmd/ksh93/edit/vi.c @@ -664,8 +664,7 @@ static void backword(Vi_t *vp,int nwords, register int cmd) register int tcur_virt = cur_virt; while( nwords-- && tcur_virt > first_virt ) { - if( !isblank(tcur_virt) && isblank(tcur_virt-1) - && tcur_virt>first_virt ) + if( !isblank(tcur_virt) && isblank(tcur_virt-1) ) --tcur_virt; else if(cmd != 'B') { @@ -674,21 +673,20 @@ static void backword(Vi_t *vp,int nwords, register int cmd) if((!cur && last) || (cur && !last)) --tcur_virt; } - while( isblank(tcur_virt) && tcur_virt>=first_virt ) + while( tcur_virt >= first_virt && isblank(tcur_virt) ) --tcur_virt; if( cmd == 'B' ) { - while( !isblank(tcur_virt) && tcur_virt>=first_virt ) + while( tcur_virt >= first_virt && !isblank(tcur_virt) ) --tcur_virt; } else { if(isalph(tcur_virt)) - while( isalph(tcur_virt) && tcur_virt>=first_virt ) + while( tcur_virt >= first_virt && isalph(tcur_virt) ) --tcur_virt; else - while( !isalph(tcur_virt) && !isblank(tcur_virt) - && tcur_virt>=first_virt ) + while( tcur_virt >= first_virt && !isalph(tcur_virt) && !isblank(tcur_virt) ) --tcur_virt; } cur_virt = ++tcur_virt; @@ -1235,23 +1233,22 @@ static void endword(Vi_t *vp, int nwords, register int cmd) register int tcur_virt = cur_virt; while( nwords-- ) { - if( !isblank(tcur_virt) && tcur_virt<=last_virt ) + if( tcur_virt <= last_virt && !isblank(tcur_virt) ) ++tcur_virt; - while( isblank(tcur_virt) && tcur_virt<=last_virt ) + while( tcur_virt <= last_virt && isblank(tcur_virt) ) ++tcur_virt; if( cmd == 'E' ) { - while( !isblank(tcur_virt) && tcur_virt<=last_virt ) + while( tcur_virt <= last_virt && !isblank(tcur_virt) ) ++tcur_virt; } else { if( isalph(tcur_virt) ) - while( isalph(tcur_virt) && tcur_virt<=last_virt ) + while( tcur_virt <= last_virt && isalph(tcur_virt) ) ++tcur_virt; else - while( !isalph(tcur_virt) && !isblank(tcur_virt) - && tcur_virt<=last_virt ) + while( tcur_virt <= last_virt && !isalph(tcur_virt) && !isblank(tcur_virt) ) ++tcur_virt; } if( tcur_virt > first_virt ) @@ -1274,24 +1271,23 @@ static void forward(Vi_t *vp,register int nwords, int cmd) { if( cmd == 'W' ) { - while( !isblank(tcur_virt) && tcur_virt < last_virt ) + while( tcur_virt < last_virt && !isblank(tcur_virt) ) ++tcur_virt; } else { if( isalph(tcur_virt) ) { - while( isalph(tcur_virt) && tcur_virt