Problem: spell: memory leak in spell_read_dic() (after 9.2.0524).
Solution: Free "pc" before breaking out of the loop (zeertzjq).
closes: #20309
Signed-off-by: zeertzjq <zeertzjq@outlook.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: spell: a word in a .dic file with many postponed prefix or
compound flags overflows the fixed-size store_afflist[MAXWLEN]
buffer in get_pfxlist() and get_compflags().
Solution: Add bounds checks (Yasuhiro Matsumoto).
closes: #20286
Signed-off-by: Yasuhiro Matsumoto <mattn.jp@gmail.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: [security]: memory safety issues in spellfile.c
(tacdm)
Solution: Add recursion limit to read_tree_node(), add length limit
check in tree_count_words(), use alloc_clear() in
spell_read_tree().
Github Security Advisory:
https://github.com/vim/vim/security/advisories/GHSA-3h95-3962-mmvf
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: read_compound() in spellfile.c computes the size of the regex
pattern buffer using signed-int arithmetic on the attacker
controlled SN_COMPOUND sectionlen. With sectionlen=0x40000008
and UTF-8 encoding active the multiplication wraps to 27 while
the per-byte loop writes up to ~1B bytes, overflowing the heap.
Reachable when loading a crafted .spl file (e.g. via 'set spell'
after a modeline sets 'spelllang'). The cp/ap/crp allocations
have the same int + 1 overflow class (Daniel Cervera)
Solution: Use type size_t as buffer size and reject values larger than
COMPOUND_MAX_LEN (100000). Apply the same size_t treatment to
the cp/ap/crp allocations.
Github Advisory:
https://github.com/vim/vim/security/advisories/GHSA-q4jv-r9gj-6cwv
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: spell_read_aff() uses sprintf() into a fixed-size stack buffer
without bounds checking. store_aff_word() uses STRCAT() to
append attacker-controlled strings into newword[MAXWLEN] without
checking remaining space. Both are reachable via :mkspell with
crafted .aff/.dic files (xinyi234)
Solution: Replace sprintf() with vim_snprintf() in spell_read_aff().
Replace STRCAT() with STRNCAT() with explicit remaining-space
calculation in store_aff_word().
closes: #19944
Signed-off-by: Christian Brabandt <cb@256bit.org>
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>
Problem: using copy_option_part() can be improved
Solution: Refactor and use the return value of copy_option_part() to
avoid strlen() calls (John Marriott).
In addition, this commit includes the following changes:
memline.c:
- In recover_names():
- Replace calls to vim_strsave() with vim_strnsave() for the literal
strings
- Use a string_T to store local variable dir_name.
bufwrite.c:
- In buf_write()
- move variable wp to where it is used.
help.c:
- In fix_help_buffer():
- replace call to add_pathsep() with after_pathsep()
optionstr.c:
- In export_myvimdir():
- use a string_T to store local variable buf
- replace call to add_pathsep() with after_pathsep()
scriptfile.c:
- In do_in_path():
- use a string_T to store local variable buf
- measure the lengths of prefix and name once before the while loop
- replace call to add_pathsep() with after_pathsep()
- move some variables closer to where they are used
spellfile.c:
- In init_spellfile():
- use a string_T to store local variable buf
closes: #19725
Signed-off-by: John Marriott <basilisk@internode.on.net>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: Generating prototype files does not work on all platforms
Solution: Rework prototypes generation using python instead of cproto,
enable it in CI to test it for each PR (Hirohito Higashi).
closes: #18045
Signed-off-by: Hirohito Higashi <h.east.727@gmail.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: time.h include is available on all platforms
Solution: Remove ifdef guards and simply include it in all source files,
get rid of double include in spellfile.c (Damien Lejay).
closes: #18075
Signed-off-by: Damien Lejay <damien@lejay.be>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: Coverity warns about a few potential issues
Solution: Fix those issues (see details below)
1) Fix overflow warning in highlight.c
This happens because we are comparing int with long
and assign a potential long value to an int, which
could cause an overflow. So add some casts to ensure
the value fits into an int.
2) Fix Overflow warning in shift_line().
This happens because we are performing a division/modulo
operation of a long type by an int type and assign the result
to an int, which could then overflow. So before performing
the operation, trim the long to value to at most max int value,
so that it can't overflow.
3) Fix overflow warning in syn_list_cluster in syntax.c
This is essential the same issue as 1)
4) not checking the return value of vim_mkdir() in spellfile.c
Creating the spell directory could fail. Handle this case
and return early in this case.
5) qsort() may deref a NULL pointer when fuzzy match does not
return a result. Fix this by checking that the accessed growarray
fuzzy_indices actually contains data. If not we can silently skip
the qsort() and related logic.
closes: #15284
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: More code can use ml_get_buf_len() instead of STRLEN().
Solution: Change more STRLEN() calls to ml_get_buf_len(). Also do not
set ml_line_textlen in ml_replace_len() if "has_props" is set,
because "len_arg" also includes the size of text properties in
that case. (zeertzjq)
closes: #14183
Signed-off-by: zeertzjq <zeertzjq@outlook.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: is*() and to*() function may be unsafe
Solution: Add SAFE_* macros and start using those instead
(Keith Thompson)
Use SAFE_() macros for is*() and to*() functions
The standard is*() and to*() functions declared in <ctype.h> have
undefined behavior for negative arguments other than EOF. If plain char
is signed, passing an unchecked value from argv for from user input
to one of these functions has undefined behavior.
Solution: Add SAFE_*() macros that cast the argument to unsigned char.
Most implementations behave sanely for negative arguments, and most
character values in practice are non-negative, but it's still best
to avoid undefined behavior.
The change from #13347 has been omitted, as this has already been
separately fixed in commit ac709e2fc0
(v9.0.2054)
fixes: #13332closes: #13347
Signed-off-by: Keith Thompson <Keith.S.Thompson@gmail.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: FOR_ALL_ macros are defined in an unexpected file.
Solution: Move FOR_ALL_ macros to macros.h. Add FOR_ALL_HASHTAB_ITEMS.
(Yegappan Lakshmanan, closes#12109)
Problem: Invalid memory access when adding word with a control character to
the internal spell word list.
Solution: Disallow adding a word with control characters or a trailing
slash.