From: Hirohito Higashi Date: Wed, 22 Jul 2026 05:51:00 +0000 (+0900) Subject: patch 9.2.0843: [security]: popup: opacity mask indexed out of bounds X-Git-Tag: v9.2.0843^0 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=975e191dc817d8d00abca7197c4529a417c2f805;p=thirdparty%2Fvim.git patch 9.2.0843: [security]: popup: opacity mask indexed out of bounds Problem: [security]: A "clipwindow" popup with "opacity" anchored to a text property can keep a negative window row when the anchor scrolls above the top of the host window. The opacity mask loop then indexes the screen array before its start, causing an out-of-bounds read and a conditional out-of-bounds write (tdjackey). Solution: Keep the popup window row at the first visible row and record the clipped-off top rows only in the top offset, so no consumer has to clamp the row and the opacity mask loop stays in bounds (Hirohito Higashi). Github Security Advisory: https://github.com/vim/vim/security/advisories/GHSA-pmvp-6rcj-98p4 Signed-off-by: Hirohito Higashi Signed-off-by: Christian Brabandt --- diff --git a/src/popupwin.c b/src/popupwin.c index 0acb7453df..293305ed92 100644 --- a/src/popupwin.c +++ b/src/popupwin.c @@ -2313,10 +2313,8 @@ popup_apply_winupdate_clip(win_T *wp, popup_clip_T *cl) if (wp->w_height < 0) wp->w_height = 0; if (cl->clip_top_content > 0) - { + // w_winrow already points at the first visible row. wp->w_topline += cl->clip_top_content; - wp->w_winrow += cl->clip_top_content; - } } if (wp->w_popup_leftclip > 0 || wp->w_popup_rightclip > 0) { @@ -2982,6 +2980,10 @@ popup_adjust_position(win_T *wp) return; } + // Make w_winrow the first visible screen row (>= 0); the clipped-off top + // rows are recorded in w_popup_topoff. + wp->w_winrow += wp->w_popup_topoff; + #ifdef FEAT_IMAGE_SIXEL // Final winrow is now known: encode (or re-encode) the sixel image so it // fits within the screen and does not trigger sixel-scroll on terminals. @@ -6120,8 +6122,11 @@ popup_mark_opacity_zindex(win_T *wp) width = popup_width(wp); height = popup_height(wp); + // w_winrow/w_wincol are visible cells (>= 0); w_popup_topoff rows are + // clipped off the top. for (r = wp->w_winrow; - r < wp->w_winrow + height && r < screen_Rows; ++r) + r < wp->w_winrow + height - wp->w_popup_topoff + && r < screen_Rows; ++r) for (c = wp->w_wincol; c < wp->w_wincol + width - wp->w_popup_leftoff && c < screen_Columns; ++c) @@ -6417,7 +6422,7 @@ may_update_popup_mask(int type) continue; { - int mask_start = wp->w_winrow + wp->w_popup_topoff; + int mask_start = wp->w_winrow; int mask_end = mask_start + height; int mask_col_start = wp->w_wincol + wp->w_popup_leftclip; int mask_col_end = wp->w_wincol + width - wp->w_popup_leftoff @@ -7453,7 +7458,8 @@ update_popups(void (*win_update)(win_T *wp)) - wp->w_popup_leftoff; if (wp->w_wincol + left_extra < 0) left_extra = -wp->w_wincol; - wp->w_winrow += top_off; + // Move to the content top, skipping any clipped top border/padding. + wp->w_winrow += MAX(top_off - wp->w_popup_topoff, 0); wp->w_wincol += left_extra; // Draw the popup text, unless it's off screen. @@ -7486,15 +7492,9 @@ update_popups(void (*win_update)(win_T *wp)) wp->w_cursor.lnum = wp->w_botline - 1; } - wp->w_winrow -= top_off; + wp->w_winrow -= MAX(top_off - wp->w_popup_topoff, 0); wp->w_wincol -= left_extra; - // "clipwindow" with top-clip shifts all popup decorations down so the - // first visible row of the popup lands at the host window's top edge. - // Apply the shift before drawing borders/padding/etc. and restore at - // the end of this popup's iteration. - wp->w_winrow += wp->w_popup_topoff; - // Add offset for border and padding if not done already. if ((wp->w_flags & WFLAG_WCOL_OFF_ADDED) == 0) { @@ -7896,10 +7896,6 @@ update_popups(void (*win_update)(win_T *wp)) if (override_success) pop_highlight_overrides(); - // Undo the topoff shift applied before drawing the borders so the - // next iteration sees the popup's logical winrow. - wp->w_winrow -= wp->w_popup_topoff; - #ifdef FEAT_IMAGE // Emit the popup image right after this popup's decorations land in // ScreenLines. Popups are walked in zindex order, so a higher diff --git a/src/testdir/test_popupwin.vim b/src/testdir/test_popupwin.vim index 364a50fe02..6aed7be3ec 100644 --- a/src/testdir/test_popupwin.vim +++ b/src/testdir/test_popupwin.vim @@ -4685,6 +4685,37 @@ func Test_popup_clipwindow_hide_when_prop_off_screen() call prop_type_delete('clipprop') endfunc +func Test_popup_clipwindow_opacity_negative_winrow() + " A "clipwindow" popup with "opacity" whose textprop anchor scrolls above + " the window top must not index the opacity mask out of bounds. + call prop_type_add('clipprop', {}) + new + call setline(1, range(1, 200)->mapnew({_, v -> 'line ' .. v})) + call prop_add(5, 1, #{type: 'clipprop', length: 5}) + let host = win_getid() + + let id = popup_create(['aaa', 'bbb', 'ccc', 'ddd', 'eee'], #{ + \ textprop: 'clipprop', + \ textpropwin: host, + \ wrap: v:false, + \ fixed: v:true, + \ clipwindow: v:true, + \ opacity: 50, + \ }) + call assert_true(id > 0) + redraw + + " Scroll so the prop (line 5) sits a couple of lines above the top, so the + " popup is clipped at the host window's top edge. + call win_execute(host, 'normal! 8Gzt') + redraw + redraw + + call popup_close(id) + bwipe! + call prop_type_delete('clipprop') +endfunc + func Test_popup_clipwindow_top_clip() CheckScreendump diff --git a/src/version.c b/src/version.c index a5cca3061b..a9bcdc88ec 100644 --- a/src/version.c +++ b/src/version.c @@ -758,6 +758,8 @@ static char *(features[]) = static int included_patches[] = { /* Add new patch number below this line */ +/**/ + 843, /**/ 842, /**/