patch 9.2.0679: [security]: Out-of-bounds read with text property virtual text

Problem:  [security]: Out-of-bounds read with text property virtual text.
          A crafted undo file can declare a virtual-text property whose
          offset points outside the line's property data, so reading the
          virtual text reads out of bounds.  This completes the count-only
          check added in 9.2.0670.
Solution: Validate the virtual-text offset and length of each property
          against the available property data before turning the offset
          into a pointer.

Github Security Advisory:
https://github.com/vim/vim/security/advisories/GHSA-ww8h-47xp-hp4w

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Hirohito Higashi <h.east.727@gmail.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>

diff --git a/src/proto/textprop.pro b/src/proto/textprop.pro
index 82f47275f28e6ef91907691646765ace28142c93..511d3b506540cfaea4f4491a441e50e5a0e8f623 100644 (file)
--- a/src/proto/textprop.pro
+++ b/src/proto/textprop.pro
@@ -37,4 +37,5 @@ int adjust_prop_columns(linenr_T lnum, colnr_T col, int bytes_added, int flags);
 void adjust_props_for_split(linenr_T lnum_props, linenr_T lnum_top, int kept, int deleted, int at_eol);
 void prepend_joined_props(unpacked_memline_T *um, linenr_T lnum, int last_line, long col, int removed);
 bool text_prop_count_valid(int prop_count, size_t propdata_len);
+bool text_prop_vtext_valid(char_u *props, int prop_count, size_t propdata_len);
 /* vim: set ft=c : */

diff --git a/src/testdir/test_textprop2.vim b/src/testdir/test_textprop2.vim
index fdebf794af742e0fd8a2f73308130cc815a8b70c..523a59b1b2ed5e6571bb00fe27199b67f5029922 100644 (file)
--- a/src/testdir/test_textprop2.vim
+++ b/src/testdir/test_textprop2.vim
@@ -428,14 +428,12 @@ func Test_multiline_prop_delete_penultimate_line()
   call s:CleanupPropTypes(['1', '2', '3'])
 endfunc
 
-func s:ManipulateUndoBlob(name)
-  " Patch the saved old line in the undo file:
-  "   00 00 00 08 'QQQQQQQQ'  ->  00 00 00 27 'AAAA' NUL count=0xFFFF <32x00>
-  " i.e. textlen 8 text-only  ->  39-byte blob: text "AAAA", NUL, prop_count
-  "   0xFFFF, one zeroed textprop_T(32).  propdata_len becomes 34, count 65535.
+func s:ManipulateUndoBlob(name, repl)
+  " Replace the saved old line (00 00 00 08 'QQQQQQQQ') in the undo file with
+  " the crafted "repl" blob, then read it back in.
   let blob   = readfile(a:name, 'B')
   let marker = 0z000000085151515151515151
-  let repl   = 0z000000274141414100FFFF + repeat(0z00, 32)
+  let repl   = a:repl
   let mlen   = len(marker)
   let idx    = -1
   let i      = 0
@@ -466,7 +464,10 @@ func Test_textprop_undo_bad_prop_count()
   let &ul = &ul
   call setline(1, 'BBBB')           " undo step saves old line 1 = "QQQQQQQQ"
   wundo Xtpundo
-  call s:ManipulateUndoBlob('Xtpundo')
+  " 39-byte blob: "AAAA" NUL count=0xFFFF, one zeroed textprop_T(32).
+  " propdata_len becomes 34 while the count claims 65535 properties.
+  call s:ManipulateUndoBlob('Xtpundo', 0z000000274141414100FFFF
+       \ + repeat(0z00, 32))
 
   undo
 
@@ -488,4 +489,49 @@ func Test_textprop_undo_bad_prop_count()
   call delete('Xtpundo')
 endfunc
 
+" A crafted undo file can restore a line whose virtual-text property declares an
+" out-of-range tp_text_offset.  Turning that offset into a pointer and reading
+" the virtual text would read past the line buffer.  Restore such a line and
+" force a consumer; reaching the asserts (no ASan abort / crash) means the
+" offset is bounded.
+func Test_textprop_undo_bad_vtext_offset()
+  CheckFeature persistent_undo
+
+  new
+  call setline(1, ['QQQQQQQQ', 'DECOYLINE'])
+  let &ul = &ul
+  call setline(1, 'BBBB')           " undo step saves old line 1 = "QQQQQQQQ"
+  wundo Xtpundo
+
+  " One textprop_T for a virtual text prop (tp_id < 0) whose tp_text_offset
+  " (0x00100000) points far past the 34-byte property data.  The count (1) is
+  " valid, so only the offset/length check can reject this.
+  let prop  = 0z01000000          " tp_col = 1
+  let prop += 0z04000000          " tp_len = 4
+  let prop += 0zFFFFFFFF          " tp_id = -1 (virtual text)
+  let prop += 0z00000000          " tp_type = 0
+  let prop += 0z00000000          " tp_flags = 0
+  let prop += 0z00000000          " tp_padleft = 0
+  let prop += 0z00001000          " u.tp_text_offset = 0x00100000
+  let prop += 0z00000000          " union upper bytes
+  call s:ManipulateUndoBlob('Xtpundo', 0z000000274141414100 + 0z0100 + prop)
+
+  undo
+
+  " Safety: prove the malicious line was actually restored before the consumer
+  " runs, so the test can't pass vacuously if the patch missed.
+  call assert_equal('AAAA', getline(1))
+
+  call prop_type_add('Xtp', {})
+  call prop_add(2, 1, {'type': 'Xtp', 'length': 1})
+
+  " this caused OOB read, now it is rejected as a corrupted (untrusted) undo
+  " file with a catchable error
+  call assert_fails('call prop_list(1)', 'E967:')
+
+  call prop_type_delete('Xtp')
+  bwipe!
+  call delete('Xtpundo')
+endfunc
+
 " vim: shiftwidth=2 sts=2 expandtab

diff --git a/src/textprop.c b/src/textprop.c
index e13e44b9461ec711e9e9c656dcedb44dd53129cb..6d5a108631b3718fef92c796258feb39d503cfa7 100644 (file)
--- a/src/textprop.c
+++ b/src/textprop.c
@@ -118,6 +118,13 @@ um_goto_line(unpacked_memline_T *um, linenr_T lnum, int extra_props)
     proplen = (int)prop_count;
     props_start = count_ptr + PROP_COUNT_SIZE;
 
+    if (!text_prop_vtext_valid(props_start, proplen, propdata_len))
+    {
+       emsg(e_text_property_info_corrupted);
+       um->buf = NULL;
+       return false;
+    }
+
     um->props = ALLOC_MULT(textprop_T, proplen + extra_props);
     if (um->props == NULL)
     {
@@ -1246,6 +1253,12 @@ get_text_props(buf_T *buf, linenr_T lnum, char_u **props, int will_change)
        emsg(e_text_property_info_corrupted);
        return 0;
     }
+    if (!text_prop_vtext_valid(text + textlen + PROP_COUNT_SIZE,
+                                          (int)prop_count, propdata_len))
+    {
+       emsg(e_text_property_info_corrupted);
+       return 0;
+    }
     *props = text + textlen + PROP_COUNT_SIZE;
     return (int)prop_count;
 }
@@ -3268,4 +3281,27 @@ text_prop_count_valid(int prop_count, size_t propdata_len)
                    <= propdata_len - PROP_COUNT_SIZE;
 }
 
+/*
+ * Return true when every virtual text property's offset and length stay within
+ * "propdata_len", so tp_text_offset can be safely turned into a pointer.
+ * "props" may be unaligned.
+ */
+    bool
+text_prop_vtext_valid(char_u *props, int prop_count, size_t propdata_len)
+{
+    for (int i = 0; i < prop_count; ++i)
+    {
+       textprop_T  prop;
+
+       mch_memmove(&prop, props + (size_t)i * sizeof(textprop_T),
+                                                         sizeof(textprop_T));
+       if (prop.tp_id >= 0 || prop.u.tp_text_offset <= 0)
+           continue;
+       if (prop.tp_len < 0 || (size_t)prop.u.tp_text_offset
+                               + (size_t)prop.tp_len + 1 > propdata_len)
+           return false;
+    }
+    return true;
+}
+
 #endif // FEAT_PROP_POPUP

diff --git a/src/version.c b/src/version.c
index 9de098d718b0673ea3483c83799b28dbff1424ed..933e847377bd125822195c5545d74d3056e24e53 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -759,6 +759,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    679,
 /**/
     678,
 /**/