vim-patch:8.2.2207: illegal memory access if popup menu items are changed (#21028)

Problem:    Illegal memory access if popup menu items are changed while the
            menu is visible. (Tomáš Janoušek)
Solution:   Make a copy of the text. (closes vim/vim#7537)

38455a9213

Co-authored-by: Bram Moolenaar <Bram@vim.org>
This commit is contained in:
zeertzjq
2022-11-12 09:57:29 +08:00
committed by GitHub
parent eee9560516
commit 2425fe2dc5
6 changed files with 212 additions and 4 deletions

View File

@@ -1043,10 +1043,16 @@ void pum_show_popupmenu(vimmenu_T *menu)
pumitem_T *array = (pumitem_T *)xcalloc((size_t)pum_size, sizeof(pumitem_T));
for (vimmenu_T *mp = menu->children; mp != NULL; mp = mp->next) {
char *s = NULL;
// Make a copy of the text, the menu may be redefined in a callback.
if (menu_is_separator(mp->dname)) {
array[idx++].pum_text = (char_u *)"";
s = "";
} else if (mp->modes & mp->enabled & mode) {
array[idx++].pum_text = (char_u *)mp->dname;
s = mp->dname;
}
if (s != NULL) {
s = xstrdup(s);
array[idx++].pum_text = (char_u *)s;
}
}
@@ -1117,6 +1123,9 @@ void pum_show_popupmenu(vimmenu_T *menu)
}
}
for (idx = 0; idx < pum_size; idx++) {
xfree(array[idx].pum_text);
}
xfree(array);
pum_undisplay(true);
if (!p_mousemev) {

View File

@@ -871,18 +871,30 @@ func Test_popup_command()
call assert_fails('popup Foo', 'E337:')
unmenu Test.Foo
let script =<< trim END
func StartTimer()
call timer_start(100, {-> ChangeMenu()})
endfunc
func ChangeMenu()
nunmenu PopUp.&Paste
nnoremenu 1.40 PopUp.&Paste :echomsg "pasted"<CR>
echomsg 'changed'
endfunc
END
call writefile(script, 'XtimerScript')
let lines =<< trim END
one two three four five
and one two Xthree four five
one more two three four five
END
call writefile(lines, 'Xtest')
let buf = RunVimInTerminal('Xtest', {})
let buf = RunVimInTerminal('-S XtimerScript Xtest', {})
call term_sendkeys(buf, ":source $VIMRUNTIME/menu.vim\<CR>")
call term_sendkeys(buf, "/X\<CR>:popup PopUp\<CR>")
call VerifyScreenDump(buf, 'Test_popup_command_01', {})
" Select a word
" go to the Paste entry in the menu
call term_sendkeys(buf, "jj")
call VerifyScreenDump(buf, 'Test_popup_command_02', {})
@@ -891,8 +903,20 @@ func Test_popup_command()
call VerifyScreenDump(buf, 'Test_popup_command_03', {})
call term_sendkeys(buf, "\<Esc>")
" Set a timer to change a menu entry while it's displayed. The text should
" not change but the command does. Making the screendump also verifies that
" "changed" shows up, which means the timer triggered
call term_sendkeys(buf, "/X\<CR>:call StartTimer() | popup PopUp\<CR>")
call VerifyScreenDump(buf, 'Test_popup_command_04', {})
" Select the Paste entry, executes the changed menu item.
call term_sendkeys(buf, "jj\<CR>")
call VerifyScreenDump(buf, 'Test_popup_command_05', {})
call StopVimInTerminal(buf)
call delete('Xtest')
call delete('XtimerScript')
endfunc
func Test_popup_complete_backwards()

View File

@@ -161,6 +161,7 @@ describe('Screen', function()
]])
end)
-- oldtest: Test_expr_map_restore_cursor()
it('cursor is restored after :map <expr> which redraws statusline vim-patch:8.1.2336', function()
exec([[
call setline(1, ['one', 'two', 'three'])
@@ -246,6 +247,7 @@ describe('Screen', function()
]])
end)
-- oldtest: Test_map_listing()
it('listing mappings clears command line vim-patch:8.2.4401', function()
screen:try_resize(40, 5)
command('nmap a b')

View File

@@ -17,6 +17,7 @@ describe('search stat', function()
screen:attach()
end)
-- oldtest: Test_search_stat_screendump()
it('right spacing with silent mapping vim-patch:8.1.1970', function()
exec([[
set shortmess-=S
@@ -57,6 +58,7 @@ describe('search stat', function()
]])
end)
-- oldtest: Test_search_stat_foldopen()
it('when only match is in fold vim-patch:8.2.0840', function()
exec([[
set shortmess-=S
@@ -86,6 +88,7 @@ describe('search stat', function()
screen:expect_unchanged()
end)
-- oldtest: Test_search_stat_then_gd()
it('is cleared by gd and gD vim-patch:8.2.3583', function()
exec([[
call setline(1, ['int cat;', 'int dog;', 'cat = dog;'])
@@ -120,6 +123,7 @@ describe('search stat', function()
]])
end)
-- oldtest: Test_search_stat_and_incsearch()
it('is not broken by calling searchcount() in tabline vim-patch:8.2.4378', function()
exec([[
call setline(1, ['abc--c', '--------abc', '--abc'])

View File

@@ -1048,6 +1048,7 @@ describe('CursorLine and CursorLineNr highlights', function()
]])
end)
-- oldtest: Test_cursorline_after_yank()
it('always updated. vim-patch:8.1.0849', function()
local screen = Screen.new(50,5)
screen:set_default_attr_ids({
@@ -1081,6 +1082,7 @@ describe('CursorLine and CursorLineNr highlights', function()
]])
end)
-- oldtest: Test_cursorline_with_visualmode()
it('with visual area. vim-patch:8.1.1001', function()
local screen = Screen.new(50,5)
screen:set_default_attr_ids({
@@ -1108,6 +1110,7 @@ describe('CursorLine and CursorLineNr highlights', function()
]])
end)
-- oldtest: Test_cursorline_callback()
it('is updated if cursor is moved up from timer vim-patch:8.2.4591', function()
local screen = Screen.new(50, 8)
screen:set_default_attr_ids({
@@ -1237,6 +1240,7 @@ describe('CursorLine and CursorLineNr highlights', function()
})
end)
-- oldtest: Test_diff_with_cursorline_number()
it('CursorLineNr shows correctly just below filler lines', function()
local screen = Screen.new(50,12)
screen:set_default_attr_ids({
@@ -1357,6 +1361,7 @@ describe('CursorColumn highlight', function()
]])
end)
-- oldtest: Test_cursorcolumn_callback()
it('is updated if cursor is moved from timer', function()
exec([[
call setline(1, ['aaaaa', 'bbbbb', 'ccccc', 'ddddd'])
@@ -1412,6 +1417,7 @@ describe('ColorColumn highlight', function()
screen:attach()
end)
-- oldtest: Test_colorcolumn()
it('when entering a buffer vim-patch:8.1.2073', function()
exec([[
set nohidden
@@ -1443,6 +1449,7 @@ describe('ColorColumn highlight', function()
]])
end)
-- oldtest: Test_colorcolumn_bri()
it("in 'breakindent' vim-patch:8.2.1689", function()
exec([[
call setline(1, 'The quick brown fox jumped over the lazy dogs')
@@ -1467,6 +1474,7 @@ describe('ColorColumn highlight', function()
]])
end)
-- oldtest: Test_colorcolumn_sbr()
it("in 'showbreak' vim-patch:8.2.1689", function()
exec([[
call setline(1, 'The quick brown fox jumped over the lazy dogs')
@@ -1686,6 +1694,7 @@ describe("'number' and 'relativenumber' highlight", function()
]])
end)
-- oldtest: Test_relativenumber_callback()
it('relative number highlight is updated if cursor is moved from timer', function()
local screen = Screen.new(50, 8)
screen:set_default_attr_ids({

View File

@@ -967,6 +967,7 @@ describe('builtin popupmenu', function()
[4] = {bold = true, reverse = true},
[5] = {bold = true, foreground = Screen.colors.SeaGreen},
[6] = {foreground = Screen.colors.Grey100, background = Screen.colors.Red},
[7] = {background = Screen.colors.Yellow}, -- Search
})
end)
@@ -3037,6 +3038,165 @@ describe('builtin popupmenu', function()
eq(false, screen.options.mousemoveevent)
eq('bar', meths.get_var('menustr'))
end)
-- oldtest: Test_popup_command()
it(':popup command', function()
exec([[
menu Test.Foo Foo
call assert_fails('popup Test.Foo', 'E336:')
call assert_fails('popup Test.Foo.X', 'E327:')
call assert_fails('popup Foo', 'E337:')
unmenu Test.Foo
]])
eq({}, meths.get_vvar('errors'))
exec([[
func ChangeMenu()
aunmenu PopUp.&Paste
nnoremenu 1.40 PopUp.&Paste :echomsg "pasted"<CR>
echomsg 'changed'
return "\<Ignore>"
endfunc
let lines =<< trim END
one two three four five
and one two Xthree four five
one more two three four five
END
call setline(1, lines)
aunmenu *
source $VIMRUNTIME/menu.vim
]])
feed('/X<CR>:popup PopUp<CR>')
screen:expect([[
one two three four five |
and one two {7:^X}three four five |
one more tw{n: Undo } |
{1:~ }{n: }{1: }|
{1:~ }{n: Paste }{1: }|
{1:~ }{n: }{1: }|
{1:~ }{n: Select Word }{1: }|
{1:~ }{n: Select Sentence }{1: }|
{1:~ }{n: Select Paragraph }{1: }|
{1:~ }{n: Select Line }{1: }|
{1:~ }{n: Select Block }{1: }|
{1:~ }{n: Select All }{1: }|
{1:~ }|
{1:~ }|
{1:~ }|
{1:~ }|
{1:~ }|
{1:~ }|
{1:~ }|
:popup PopUp |
]])
-- go to the Paste entry in the menu
feed('jj')
screen:expect([[
one two three four five |
and one two {7:^X}three four five |
one more tw{n: Undo } |
{1:~ }{n: }{1: }|
{1:~ }{s: Paste }{1: }|
{1:~ }{n: }{1: }|
{1:~ }{n: Select Word }{1: }|
{1:~ }{n: Select Sentence }{1: }|
{1:~ }{n: Select Paragraph }{1: }|
{1:~ }{n: Select Line }{1: }|
{1:~ }{n: Select Block }{1: }|
{1:~ }{n: Select All }{1: }|
{1:~ }|
{1:~ }|
{1:~ }|
{1:~ }|
{1:~ }|
{1:~ }|
{1:~ }|
:popup PopUp |
]])
-- Select a word
feed('j')
screen:expect([[
one two three four five |
and one two {7:^X}three four five |
one more tw{n: Undo } |
{1:~ }{n: }{1: }|
{1:~ }{n: Paste }{1: }|
{1:~ }{n: }{1: }|
{1:~ }{s: Select Word }{1: }|
{1:~ }{n: Select Sentence }{1: }|
{1:~ }{n: Select Paragraph }{1: }|
{1:~ }{n: Select Line }{1: }|
{1:~ }{n: Select Block }{1: }|
{1:~ }{n: Select All }{1: }|
{1:~ }|
{1:~ }|
{1:~ }|
{1:~ }|
{1:~ }|
{1:~ }|
{1:~ }|
:popup PopUp |
]])
feed('<Esc>')
-- Set an <expr> mapping to change a menu entry while it's displayed.
-- The text should not change but the command does.
-- Also verify that "changed" shows up, which means the mapping triggered.
command('nnoremap <expr> <F2> ChangeMenu()')
feed('/X<CR>:popup PopUp<CR><F2>')
screen:expect([[
one two three four five |
and one two {7:^X}three four five |
one more tw{n: Undo } |
{1:~ }{n: }{1: }|
{1:~ }{n: Paste }{1: }|
{1:~ }{n: }{1: }|
{1:~ }{n: Select Word }{1: }|
{1:~ }{n: Select Sentence }{1: }|
{1:~ }{n: Select Paragraph }{1: }|
{1:~ }{n: Select Line }{1: }|
{1:~ }{n: Select Block }{1: }|
{1:~ }{n: Select All }{1: }|
{1:~ }|
{1:~ }|
{1:~ }|
{1:~ }|
{1:~ }|
{1:~ }|
{1:~ }|
changed |
]])
-- Select the Paste entry, executes the changed menu item.
feed('jj<CR>')
screen:expect([[
one two three four five |
and one two {7:^X}three four five |
one more two three four five |
{1:~ }|
{1:~ }|
{1:~ }|
{1:~ }|
{1:~ }|
{1:~ }|
{1:~ }|
{1:~ }|
{1:~ }|
{1:~ }|
{1:~ }|
{1:~ }|
{1:~ }|
{1:~ }|
{1:~ }|
{1:~ }|
pasted |
]])
end)
end)
describe('builtin popupmenu with ui/ext_multigrid', function()