Skip to content

Commit 8d23fcb

Browse files
mattnchrisbra
mattn
authored and
chrisbra
committed
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>
1 parent c3c3478 commit 8d23fcb

10 files changed

Lines changed: 35 additions & 20 deletions

File tree

‎src/alloc.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -746,6 +746,8 @@ ga_grow_inner(garray_T *gap, int n)
746746
if (n < gap->ga_len / 2)
747747
n = gap->ga_len / 2;
748748

749+
if (n > 0 && (size_t)(gap->ga_len + n) > SIZE_MAX / gap->ga_itemsize)
750+
return FAIL;
749751
new_len = (size_t)gap->ga_itemsize * (gap->ga_len + n);
750752
pp = vim_realloc(gap->ga_data, new_len);
751753
if (pp == NULL)

‎src/channel.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2290,6 +2290,8 @@ channel_consume(channel_T *channel, ch_part_T part, int len)
22902290
readq_T *node = head->rq_next;
22912291
char_u *buf = node->rq_buffer;
22922292

2293+
if (len < 0 || (long_u)len > node->rq_buflen)
2294+
return;
22932295
mch_memmove(buf, buf + len, node->rq_buflen - len);
22942296
node->rq_buflen -= len;
22952297
node->rq_buffer[node->rq_buflen] = NUL;

‎src/insexpand.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4538,6 +4538,8 @@ fuzzy_longest_match(void)
45384538
return;
45394539
}
45404540

4541+
if ((size_t)compl_num_bests > SIZE_MAX / sizeof(compl_T *))
4542+
return;
45414543
compl_best_matches = (compl_T **)alloc(compl_num_bests * sizeof(compl_T *));
45424544
if (compl_best_matches == NULL)
45434545
return;

‎src/libvterm/src/vterm.c‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -181,18 +181,17 @@ INTERNAL void vterm_push_output_bytes(VTerm *vt, const char *bytes, size_t len)
181181
INTERNAL void vterm_push_output_vsprintf(VTerm *vt, const char *format, va_list args)
182182
{
183183
size_t len;
184-
#ifndef VSNPRINTF
185-
// When vsnprintf() is not available (C90) fall back to vsprintf().
186-
char buffer[1024]; // 1Kbyte is enough for everybody, right?
187-
#endif
188-
189184
#ifdef VSNPRINTF
190185
len = VSNPRINTF(vt->tmpbuffer, vt->tmpbuffer_len, format, args);
191-
vterm_push_output_bytes(vt, vt->tmpbuffer, len);
192186
#else
193-
len = vsprintf(buffer, format, args);
194-
vterm_push_output_bytes(vt, buffer, len);
187+
// When vsnprintf() is not available (C90) fall back to vsprintf().
188+
// Use the heap-allocated tmpbuffer (default 4096 bytes) instead of a small
189+
// stack buffer to reduce the risk of overflow.
190+
len = vsprintf(vt->tmpbuffer, format, args);
191+
if (len >= vt->tmpbuffer_len)
192+
len = vt->tmpbuffer_len - 1;
195193
#endif
194+
vterm_push_output_bytes(vt, vt->tmpbuffer, len);
196195
}
197196

198197
INTERNAL void vterm_push_output_sprintf(VTerm *vt, const char *format, ...)

‎src/list.c‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,9 @@ list_alloc_with_items(int count)
118118
{
119119
list_T *l;
120120

121+
if (count > 0
122+
&& (size_t)count > (SIZE_MAX - sizeof(list_T)) / sizeof(listitem_T))
123+
return NULL;
121124
l = (list_T *)alloc_clear(sizeof(list_T) + count * sizeof(listitem_T));
122125
if (l == NULL)
123126
return NULL;

‎src/popupwin.c‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4103,6 +4103,11 @@ popup_update_mask(win_T *wp, int width, int height)
41034103
return; // cache is still valid
41044104

41054105
vim_free(wp->w_popup_mask_cells);
4106+
if (width > 0 && (size_t)height > SIZE_MAX / (size_t)width)
4107+
{
4108+
wp->w_popup_mask_cells = NULL;
4109+
return;
4110+
}
41064111
wp->w_popup_mask_cells = alloc_clear((size_t)width * height);
41074112
if (wp->w_popup_mask_cells == NULL)
41084113
return;

‎src/regexp_bt.c‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5525,7 +5525,7 @@ regprop(char_u *op)
55255525
case MOPEN + 7:
55265526
case MOPEN + 8:
55275527
case MOPEN + 9:
5528-
buflen += sprintf(buf + buflen, "MOPEN%d", OP(op) - MOPEN);
5528+
buflen += vim_snprintf(buf + buflen, sizeof(buf) - buflen, "MOPEN%d", OP(op) - MOPEN);
55295529
p = NULL;
55305530
break;
55315531
case MCLOSE + 0:
@@ -5540,7 +5540,7 @@ regprop(char_u *op)
55405540
case MCLOSE + 7:
55415541
case MCLOSE + 8:
55425542
case MCLOSE + 9:
5543-
buflen += sprintf(buf + buflen, "MCLOSE%d", OP(op) - MCLOSE);
5543+
buflen += vim_snprintf(buf + buflen, sizeof(buf) - buflen, "MCLOSE%d", OP(op) - MCLOSE);
55445544
p = NULL;
55455545
break;
55465546
case BACKREF + 1:
@@ -5552,7 +5552,7 @@ regprop(char_u *op)
55525552
case BACKREF + 7:
55535553
case BACKREF + 8:
55545554
case BACKREF + 9:
5555-
buflen += sprintf(buf + buflen, "BACKREF%d", OP(op) - BACKREF);
5555+
buflen += vim_snprintf(buf + buflen, sizeof(buf) - buflen, "BACKREF%d", OP(op) - BACKREF);
55565556
p = NULL;
55575557
break;
55585558
case NOPEN:
@@ -5571,7 +5571,7 @@ regprop(char_u *op)
55715571
case ZOPEN + 7:
55725572
case ZOPEN + 8:
55735573
case ZOPEN + 9:
5574-
buflen += sprintf(buf + buflen, "ZOPEN%d", OP(op) - ZOPEN);
5574+
buflen += vim_snprintf(buf + buflen, sizeof(buf) - buflen, "ZOPEN%d", OP(op) - ZOPEN);
55755575
p = NULL;
55765576
break;
55775577
case ZCLOSE + 1:
@@ -5583,7 +5583,7 @@ regprop(char_u *op)
55835583
case ZCLOSE + 7:
55845584
case ZCLOSE + 8:
55855585
case ZCLOSE + 9:
5586-
buflen += sprintf(buf + buflen, "ZCLOSE%d", OP(op) - ZCLOSE);
5586+
buflen += vim_snprintf(buf + buflen, sizeof(buf) - buflen, "ZCLOSE%d", OP(op) - ZCLOSE);
55875587
p = NULL;
55885588
break;
55895589
case ZREF + 1:
@@ -5595,7 +5595,7 @@ regprop(char_u *op)
55955595
case ZREF + 7:
55965596
case ZREF + 8:
55975597
case ZREF + 9:
5598-
buflen += sprintf(buf + buflen, "ZREF%d", OP(op) - ZREF);
5598+
buflen += vim_snprintf(buf + buflen, sizeof(buf) - buflen, "ZREF%d", OP(op) - ZREF);
55995599
p = NULL;
56005600
break;
56015601
# endif
@@ -5636,7 +5636,7 @@ regprop(char_u *op)
56365636
case BRACE_COMPLEX + 7:
56375637
case BRACE_COMPLEX + 8:
56385638
case BRACE_COMPLEX + 9:
5639-
buflen += sprintf(buf + buflen, "BRACE_COMPLEX%d", OP(op) - BRACE_COMPLEX);
5639+
buflen += vim_snprintf(buf + buflen, sizeof(buf) - buflen, "BRACE_COMPLEX%d", OP(op) - BRACE_COMPLEX);
56405640
p = NULL;
56415641
break;
56425642
case MULTIBYTECODE:
@@ -5646,12 +5646,12 @@ regprop(char_u *op)
56465646
p = "NEWL";
56475647
break;
56485648
default:
5649-
buflen += sprintf(buf + buflen, "corrupt %d", OP(op));
5649+
buflen += vim_snprintf(buf + buflen, sizeof(buf) - buflen, "corrupt %d", OP(op));
56505650
p = NULL;
56515651
break;
56525652
}
56535653
if (p != NULL)
5654-
STRCPY(buf + buflen, p);
5654+
vim_strncpy((char_u *)buf + buflen, (char_u *)p, sizeof(buf) - buflen - 1);
56555655
return (char_u *)buf;
56565656
}
56575657
#endif // DEBUG

‎src/spell.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2965,7 +2965,7 @@ ex_spellrepall(exarg_T *eap UNUSED)
29652965
}
29662966
size_t repl_from_len = STRLEN(repl_from);
29672967
size_t repl_to_len = STRLEN(repl_to);
2968-
int addlen = (int)(repl_to_len - repl_from_len);
2968+
long addlen = (long)repl_to_len - (long)repl_from_len;
29692969

29702970
frompat = alloc(repl_from_len + 7);
29712971
if (frompat == NULL)
@@ -2999,7 +2999,7 @@ ex_spellrepall(exarg_T *eap UNUSED)
29992999
#if defined(FEAT_PROP_POPUP)
30003000
if (curbuf->b_has_textprop && addlen != 0)
30013001
adjust_prop_columns(curwin->w_cursor.lnum,
3002-
curwin->w_cursor.col, addlen, APC_SUBSTITUTE);
3002+
curwin->w_cursor.col, (int)addlen, APC_SUBSTITUTE);
30033003
#endif
30043004

30053005
if (curwin->w_cursor.lnum != prev_lnum)

‎src/spellfile.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1589,7 +1589,7 @@ spell_read_tree(
15891589
len = get4c(fd);
15901590
if (len < 0)
15911591
return SP_TRUNCERROR;
1592-
if (len >= LONG_MAX / (long)sizeof(int))
1592+
if ((size_t)len > SIZE_MAX / sizeof(int))
15931593
// Invalid length, multiply with sizeof(int) would overflow.
15941594
return SP_FORMERROR;
15951595
if (len <= 0)

‎src/version.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -734,6 +734,8 @@ static char *(features[]) =
734734

735735
static int included_patches[] =
736736
{ /* Add new patch number below this line */
737+
/**/
738+
315,
737739
/**/
738740
314,
739741
/**/

0 commit comments

Comments
 (0)