patch 9.2.0315: missing bound-checks

Problem:  missing bound-checks
Solution: Add defensive guards against potential buffer overflow
          (Yasuhiro Matsumoto)

Add bounds checking and integer overflow guards across multiple files
as a defensive measure. While these code paths are unlikely to be
exploitable in practice, the guards prevent undefined behavior in
edge cases.

- libvterm/vterm.c: use heap tmpbuffer instead of stack buffer in
  vsprintf() fallback path
- channel.c: validate len in channel_consume() before mch_memmove()
- spell.c: use long instead of int for addlen to avoid signed overflow
  in size_t subtraction
- alloc.c: add SIZE_MAX overflow check in ga_grow_inner() before
  itemsize multiplication
- list.c: add overflow check before count * sizeof(listitem_T)
- popupwin.c: add overflow check before width * height allocation
- insexpand.c: add overflow check before compl_num_bests multiplication
- regexp_bt.c: replace sprintf() with vim_snprintf() in regprop()
- spellfile.c: use SIZE_MAX instead of LONG_MAX for allocation overflow
  check

closes: #19904

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

diff --git a/src/alloc.c b/src/alloc.c
index 0fa59ad6d6b61fdd0df91988609aa0c8958b329c..463a7104e3330e4891cb4dba3b3427770ca5386f 100644 (file)
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -746,6 +746,8 @@ ga_grow_inner(garray_T *gap, int n)
     if (n < gap->ga_len / 2)
        n = gap->ga_len / 2;
 
+    if (n > 0 && (size_t)(gap->ga_len + n) > SIZE_MAX / gap->ga_itemsize)
+       return FAIL;
     new_len = (size_t)gap->ga_itemsize * (gap->ga_len + n);
     pp = vim_realloc(gap->ga_data, new_len);
     if (pp == NULL)

diff --git a/src/channel.c b/src/channel.c
index 37f0bf1be52393732a05e8df0c2f74d04a127b38..6b4bd3cf2b7c1e7ffe662ac0c191f5e709b9e90b 100644 (file)
--- a/src/channel.c
+++ b/src/channel.c
@@ -2290,6 +2290,8 @@ channel_consume(channel_T *channel, ch_part_T part, int len)
     readq_T *node = head->rq_next;
     char_u *buf = node->rq_buffer;
 
+    if (len < 0 || (long_u)len > node->rq_buflen)
+       return;
     mch_memmove(buf, buf + len, node->rq_buflen - len);
     node->rq_buflen -= len;
     node->rq_buffer[node->rq_buflen] = NUL;

diff --git a/src/insexpand.c b/src/insexpand.c
index bfae80f61bb60ae7fd9e2a2a5f27f538aa1db5ae..3fade6cb160290a0c952e53cf871bb4367890d64 100644 (file)
--- a/src/insexpand.c
+++ b/src/insexpand.c
@@ -4538,6 +4538,8 @@ fuzzy_longest_match(void)
        return;
     }
 
+    if ((size_t)compl_num_bests > SIZE_MAX / sizeof(compl_T *))
+       return;
     compl_best_matches = (compl_T **)alloc(compl_num_bests * sizeof(compl_T *));
     if (compl_best_matches == NULL)
        return;

diff --git a/src/libvterm/src/vterm.c b/src/libvterm/src/vterm.c
index dba63771865fe81be57715a138f74504d8039532..a5f5ce3f8aab5d2c9d854dcad6b9645441ffe214 100644 (file)
--- a/src/libvterm/src/vterm.c
+++ b/src/libvterm/src/vterm.c
@@ -181,18 +181,17 @@ INTERNAL void vterm_push_output_bytes(VTerm *vt, const char *bytes, size_t len)
 INTERNAL void vterm_push_output_vsprintf(VTerm *vt, const char *format, va_list args)
 {
   size_t len;
-#ifndef VSNPRINTF
-  // When vsnprintf() is not available (C90) fall back to vsprintf().
-  char buffer[1024]; // 1Kbyte is enough for everybody, right?
-#endif
-
 #ifdef VSNPRINTF
   len = VSNPRINTF(vt->tmpbuffer, vt->tmpbuffer_len, format, args);
-  vterm_push_output_bytes(vt, vt->tmpbuffer, len);
 #else
-  len = vsprintf(buffer, format, args);
-  vterm_push_output_bytes(vt, buffer, len);
+  // When vsnprintf() is not available (C90) fall back to vsprintf().
+  // Use the heap-allocated tmpbuffer (default 4096 bytes) instead of a small
+  // stack buffer to reduce the risk of overflow.
+  len = vsprintf(vt->tmpbuffer, format, args);
+  if (len >= vt->tmpbuffer_len)
+    len = vt->tmpbuffer_len - 1;
 #endif
+  vterm_push_output_bytes(vt, vt->tmpbuffer, len);
 }
 
 INTERNAL void vterm_push_output_sprintf(VTerm *vt, const char *format, ...)

diff --git a/src/list.c b/src/list.c
index cd12bcb0a010c474a6464fa48675959f0635ba29..c9e9d686bdb91f47b2aa055cd24a8afc4529865b 100644 (file)
--- a/src/list.c
+++ b/src/list.c
@@ -118,6 +118,9 @@ list_alloc_with_items(int count)
 {
     list_T     *l;
 
+    if (count > 0
+           && (size_t)count > (SIZE_MAX - sizeof(list_T)) / sizeof(listitem_T))
+       return NULL;
     l = (list_T *)alloc_clear(sizeof(list_T) + count * sizeof(listitem_T));
     if (l == NULL)
        return NULL;

diff --git a/src/popupwin.c b/src/popupwin.c
index cbbb7c6e5d0723dc66af00542370d3e7dba16e04..3ab96cb3e58da48b6c9f0a16aaec8bb7ee41a322 100644 (file)
--- a/src/popupwin.c
+++ b/src/popupwin.c
@@ -4103,6 +4103,11 @@ popup_update_mask(win_T *wp, int width, int height)
        return;  // cache is still valid
 
     vim_free(wp->w_popup_mask_cells);
+    if (width > 0 && (size_t)height > SIZE_MAX / (size_t)width)
+    {
+       wp->w_popup_mask_cells = NULL;
+       return;
+    }
     wp->w_popup_mask_cells = alloc_clear((size_t)width * height);
     if (wp->w_popup_mask_cells == NULL)
        return;

diff --git a/src/regexp_bt.c b/src/regexp_bt.c
index a2877fc8de5d0fc90e27ff6c833a33ae56a66345..1fa80f5c7206956cc44c51ce83a04bce3e4c25da 100644 (file)
--- a/src/regexp_bt.c
+++ b/src/regexp_bt.c
@@ -5525,7 +5525,7 @@ regprop(char_u *op)
       case MOPEN + 7:
       case MOPEN + 8:
       case MOPEN + 9:
-       buflen += sprintf(buf + buflen, "MOPEN%d", OP(op) - MOPEN);
+       buflen += vim_snprintf(buf + buflen, sizeof(buf) - buflen, "MOPEN%d", OP(op) - MOPEN);
        p = NULL;
        break;
       case MCLOSE + 0:
@@ -5540,7 +5540,7 @@ regprop(char_u *op)
       case MCLOSE + 7:
       case MCLOSE + 8:
       case MCLOSE + 9:
-       buflen += sprintf(buf + buflen, "MCLOSE%d", OP(op) - MCLOSE);
+       buflen += vim_snprintf(buf + buflen, sizeof(buf) - buflen, "MCLOSE%d", OP(op) - MCLOSE);
        p = NULL;
        break;
       case BACKREF + 1:
@@ -5552,7 +5552,7 @@ regprop(char_u *op)
       case BACKREF + 7:
       case BACKREF + 8:
       case BACKREF + 9:
-       buflen += sprintf(buf + buflen, "BACKREF%d", OP(op) - BACKREF);
+       buflen += vim_snprintf(buf + buflen, sizeof(buf) - buflen, "BACKREF%d", OP(op) - BACKREF);
        p = NULL;
        break;
       case NOPEN:
@@ -5571,7 +5571,7 @@ regprop(char_u *op)
       case ZOPEN + 7:
       case ZOPEN + 8:
       case ZOPEN + 9:
-       buflen += sprintf(buf + buflen, "ZOPEN%d", OP(op) - ZOPEN);
+       buflen += vim_snprintf(buf + buflen, sizeof(buf) - buflen, "ZOPEN%d", OP(op) - ZOPEN);
        p = NULL;
        break;
       case ZCLOSE + 1:
@@ -5583,7 +5583,7 @@ regprop(char_u *op)
       case ZCLOSE + 7:
       case ZCLOSE + 8:
       case ZCLOSE + 9:
-       buflen += sprintf(buf + buflen, "ZCLOSE%d", OP(op) - ZCLOSE);
+       buflen += vim_snprintf(buf + buflen, sizeof(buf) - buflen, "ZCLOSE%d", OP(op) - ZCLOSE);
        p = NULL;
        break;
       case ZREF + 1:
@@ -5595,7 +5595,7 @@ regprop(char_u *op)
       case ZREF + 7:
       case ZREF + 8:
       case ZREF + 9:
-       buflen += sprintf(buf + buflen, "ZREF%d", OP(op) - ZREF);
+       buflen += vim_snprintf(buf + buflen, sizeof(buf) - buflen, "ZREF%d", OP(op) - ZREF);
        p = NULL;
        break;
 # endif
@@ -5636,7 +5636,7 @@ regprop(char_u *op)
       case BRACE_COMPLEX + 7:
       case BRACE_COMPLEX + 8:
       case BRACE_COMPLEX + 9:
-       buflen += sprintf(buf + buflen, "BRACE_COMPLEX%d", OP(op) - BRACE_COMPLEX);
+       buflen += vim_snprintf(buf + buflen, sizeof(buf) - buflen, "BRACE_COMPLEX%d", OP(op) - BRACE_COMPLEX);
        p = NULL;
        break;
       case MULTIBYTECODE:
@@ -5646,12 +5646,12 @@ regprop(char_u *op)
        p = "NEWL";
        break;
       default:
-       buflen += sprintf(buf + buflen, "corrupt %d", OP(op));
+       buflen += vim_snprintf(buf + buflen, sizeof(buf) - buflen, "corrupt %d", OP(op));
        p = NULL;
        break;
     }
     if (p != NULL)
-       STRCPY(buf + buflen, p);
+       vim_strncpy((char_u *)buf + buflen, (char_u *)p, sizeof(buf) - buflen - 1);
     return (char_u *)buf;
 }
 #endif     // DEBUG

diff --git a/src/spell.c b/src/spell.c
index 78fd78a78d0cdb2bd0c3980f11167e3aad65b166..01eb57e3a9e112aadf556d034064f8907ace8c78 100644 (file)
--- a/src/spell.c
+++ b/src/spell.c
@@ -2965,7 +2965,7 @@ ex_spellrepall(exarg_T *eap UNUSED)
     }
     size_t     repl_from_len = STRLEN(repl_from);
     size_t     repl_to_len = STRLEN(repl_to);
-    int                addlen = (int)(repl_to_len - repl_from_len);
+    long       addlen = (long)repl_to_len - (long)repl_from_len;
 
     frompat = alloc(repl_from_len + 7);
     if (frompat == NULL)
@@ -2999,7 +2999,7 @@ ex_spellrepall(exarg_T *eap UNUSED)
 #if defined(FEAT_PROP_POPUP)
            if (curbuf->b_has_textprop && addlen != 0)
                adjust_prop_columns(curwin->w_cursor.lnum,
-                                curwin->w_cursor.col, addlen, APC_SUBSTITUTE);
+                                curwin->w_cursor.col, (int)addlen, APC_SUBSTITUTE);
 #endif
 
            if (curwin->w_cursor.lnum != prev_lnum)

diff --git a/src/spellfile.c b/src/spellfile.c
index 7c105cf79c3ff061d69904b1a1a4c5fc8f0ee930..c570920795032f759a2e98ddafcd2162bd200eea 100644 (file)
--- a/src/spellfile.c
+++ b/src/spellfile.c
@@ -1589,7 +1589,7 @@ spell_read_tree(
     len = get4c(fd);
     if (len < 0)
        return SP_TRUNCERROR;
-    if (len >= LONG_MAX / (long)sizeof(int))
+    if ((size_t)len > SIZE_MAX / sizeof(int))
        // Invalid length, multiply with sizeof(int) would overflow.
        return SP_FORMERROR;
     if (len <= 0)

diff --git a/src/version.c b/src/version.c
index 0c3123cd955b6bc77c4de9b02516a8a5c5c4b2b6..9e288f00023764f8b1bd12d259ee6f9f7eb67e88 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -734,6 +734,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    315,
 /**/
     314,
 /**/