patch 9.2.0670: [security]: Out-of-bounds read with text properties

Problem:  [security]: Out-of-bounds read with text properties
          (cipher-creator)
Solution: Add out-of-bound checks (Yasuhiro Matsumoto)

Github Security Advisory:
https://github.com/vim/vim/security/advisories/GHSA-f36c-2qcp-7gpw

Supported by AI

Signed-off-by: Yasuhiro Matsumoto <mattn.jp@gmail.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>

diff --git a/src/memline.c b/src/memline.c
index 51b09cc1caac2ad864dfe3fddaf5270f27618b7d..4ad09f43a58fd2f18558f7304c187923f0af30aa 100644 (file)
--- a/src/memline.c
+++ b/src/memline.c
@@ -3808,6 +3808,11 @@ adjust_text_props_for_delete(
                uint16_t pc;
 
                mch_memmove(&pc, text + textlen, PROP_COUNT_SIZE);
+               if (!text_prop_count_valid(pc, (size_t)(line_size - (long)textlen)))
+               {
+                   internal_error("text property count too large");
+                   return;
+               }
                this_props_len = pc * (int)sizeof(textprop_T);
            }
 
@@ -4046,6 +4051,8 @@ theend:
        mch_memmove(&pc, textprop_save, PROP_COUNT_SIZE);
        props_data = textprop_save + PROP_COUNT_SIZE;
        props_bytes = pc * (int)sizeof(textprop_T);
+       if (!text_prop_count_valid(pc, (size_t)textprop_len))
+           props_bytes = 0;
 
        // Adjust text properties in the line above and below.
        if (lnum > 1)

diff --git a/src/proto/textprop.pro b/src/proto/textprop.pro
index be9d80c6e3f09a5003d13f7625419de77731da84..82f47275f28e6ef91907691646765ace28142c93 100644 (file)
--- a/src/proto/textprop.pro
+++ b/src/proto/textprop.pro
@@ -36,4 +36,5 @@ void clear_buf_prop_types(buf_T *buf);
 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);
 /* vim: set ft=c : */

diff --git a/src/testdir/test_textprop2.vim b/src/testdir/test_textprop2.vim
index 193a808415af6639450dbe07684fc45f308b87c7..48387d1c0440d8da41c1fbfb96f5859ab1f91453 100644 (file)
--- a/src/testdir/test_textprop2.vim
+++ b/src/testdir/test_textprop2.vim
@@ -428,4 +428,63 @@ 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.
+  let blob   = readfile(a:name, 'B')
+  let marker = 0z000000085151515151515151
+  let repl   = 0z000000274141414100FFFF + repeat(0z00, 32)
+  let mlen   = len(marker)
+  let idx    = -1
+  let i      = 0
+  while i <= len(blob) - mlen
+    if blob[i : i + mlen - 1] ==# marker
+      let idx = i
+      break
+    endif
+    let i += 1
+  endwhile
+  call assert_true(idx >= 0, 'saved-line marker not found in undo file')
+
+  let head = idx > 0 ? blob[0 : idx - 1] : 0z
+  call writefile(head + repl + blob[idx + mlen :], a:name)
+
+  exe "rundo" a:name
+endfunc
+
+" A crafted undo file can restore a line whose declared text-property count is
+" far larger than the data, making get_text_props() / consumers read past the
+" line buffer.  Restore such a line and force a consumer; reaching the asserts
+" (no ASan abort / crash) means the count is bounded.
+func Test_textprop_undo_bad_prop_count()
+  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
+  call s:ManipulateUndoBlob('Xtpundo')
+
+  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))
+
+  " Adding a property anywhere sets b_has_textprop, so get_text_props() will
+  " actually inspect line 1 instead of returning early.
+  call prop_type_add('Xtp', {})
+  call prop_add(2, 1, {'type': 'Xtp', 'length': 1})
+
+  " this caused OOB read, now it triggers internal error
+  call assert_fails('call prop_list(1)', ['E340:', 'corrupted'])
+
+  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 a049edec9b9fc4136dab0eacd742a4347e879010..858d8dbceca907d6d0aa405571043b3875c64414 100644 (file)
--- a/src/textprop.c
+++ b/src/textprop.c
@@ -109,6 +109,12 @@ um_goto_line(unpacked_memline_T *um, linenr_T lnum, int extra_props)
     char_u         *props_start;
 
     mch_memmove(&prop_count, count_ptr, PROP_COUNT_SIZE);
+    if (!text_prop_count_valid(prop_count, propdata_len))
+    {
+       iemsg(e_text_property_info_corrupted);
+       um->buf = NULL;
+       return false;
+    }
     proplen = (int)prop_count;
     props_start = count_ptr + PROP_COUNT_SIZE;
 
@@ -1235,6 +1241,11 @@ get_text_props(buf_T *buf, linenr_T lnum, char_u **props, int will_change)
        return 0;
     }
     mch_memmove(&prop_count, text + textlen, PROP_COUNT_SIZE);
+    if (!text_prop_count_valid(prop_count, propdata_len))
+    {
+       iemsg(e_text_property_info_corrupted);
+       return 0;
+    }
     *props = text + textlen + PROP_COUNT_SIZE;
     return (int)prop_count;
 }
@@ -3248,4 +3259,13 @@ prepend_joined_props(
     um_abort(&r_um);
 }
 
+    bool
+text_prop_count_valid(int prop_count, size_t propdata_len)
+{
+    if (propdata_len < PROP_COUNT_SIZE)
+       return false;
+    return (size_t)prop_count * sizeof(textprop_T)
+                   <= propdata_len - PROP_COUNT_SIZE;
+}
+
 #endif // FEAT_PROP_POPUP

diff --git a/src/version.c b/src/version.c
index e4f09bcab5cc29874d8a222a85d2b19b87fe72fe..7d1c8885b68a02f141f6a24b166dae9e29390364 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 */
+/**/
+    670,
 /**/
     669,
 /**/