Problem: Moving tabpages on :drop may cause an endless loop
Solution: Disallow moving tabpages on :drop when cleaning up the arglist
first
Moving tabpages during drop command may cause an endless loop
When executing a :tab drop command, Vim will close all windows not in
the argument list. This triggers various autocommands. If a user has
created an 'au Tabenter * :tabmove -' autocommand, this can cause Vim to
end up in an endless loop, when trying to iterate over all tabs (which
would trigger the tabmove autocommand, which will change the tpnext
pointer, etc).
So instead of blocking all autocommands before we actually try to edit
the given file, lets simply disallow to move tabpages around. Otherwise,
we may change the expected number of events triggered during a :drop
command, which users may rely on (there is actually a test, that expects
various TabLeave/TabEnter autocommands) and would therefore be a
backwards incompatible change.
Don't make this an error, as this could trigger several times during the
drop command, but silently ignore the :tabmove command in this case (and
it should in fact finally trigger successfully when loading the given
file in a new tab). So let's just be quiet here instead.
fixes: vim/vim#13676closes: vim/vim#13686df12e39b8b
Co-authored-by: Christian Brabandt <cb@256bit.org>
Problem: We have `P_(BOOL|NUM|STRING)` macros to represent an option's type, which is redundant because `OptValType` can already do that. The current implementation of option type flags is also too limited to allow adding multitype options in the future.
Solution: Remove `P_(BOOL|NUM|STRING)` and replace it with a new `type_flags` attribute in `vimoption_T`. Also do some groundwork for adding multitype options in the future.
Side-effects: Attempting to set an invalid keycode option (e.g. `set t_foo=123`) no longer gives an error.
Problem: The options[] array is not sorted alphabetically.
Solution: Sort it alphabetically. Add a test. Avoid unnecessary loop
iterations in findoption().
closes: vim/vim#13648
Cherry-pick Test_set_one_column() change from patch 8.2.0432.
f48558e10a
Problem: Confusing error for missing key.
Solution: Use the actualy key for the error. (closesvim/vim#9241)
5c1ec439f0
Co-authored-by: Bram Moolenaar <Bram@vim.org>
Problem: 'breakindent' is not drawn after diff filler lines.
Solution: Correct check for whether 'breakindent' should be drawn.
closes: vim/vim#13624588f20dece
Cherry-pick Test_diff_with_syntax() change from patch 9.0.1257.
Problem: Vim does not detect pacman.log file
Solution: Detect pacmanlogs and add syntax highlighting
pacman.log is a filetype common to Arch Liux and related distributions.
Add some simple syntax highlighting for the pacmanlog filetype.
closes: vim/vim#136181e5d66408e
Co-authored-by: Ronan Pigott <ronan@rjp.ie>
Problem: Wrong scrolling in Insert mode with 'smoothscroll' at the
bottom of the window.
Solution: Don't use set_topline() when 'smoothscroll' is set.
fixes: vim/vim#13612closes: vim/vim#136135b4d1fcbf0
Problem: Default color scheme is suboptimal.
Solution: Start using new color scheme. Introduce new `vim` color scheme
for opt-in backward compatibility.
------
Main design ideas
- Be "Neovim branded".
- Be minimal for 256 colors with a bit more shades for true colors.
- Be accessible through high enough contrast ratios.
- Be suitable for dark and light backgrounds via exchange of dark and
light palettes.
------
Palettes
- Have dark and light variants. Implemented through exporeted
`NvimDark*` and `NvimLight*` hex colors.
- Palettes have 4 shades of grey for UI elements and 6 colors (red,
yellow, green, cyan, blue, magenta).
- Actual values are computed procedurally in Oklch color space based on
a handful of hyperparameters.
- Each color has a 256 colors variant with perceptually closest color.
------
Highlight groups
Use:
- Grey shades for general UI according to their design.
- Bold text for keywords (`Statement` highlight group). This is an
important choice to increase accessibility for people with color
deficiencies, as it doesn't rely on actual color.
- Green for strings, `DiffAdd` (as background), `DiagnosticOk`, and some
minor text UI elements.
- Cyan as main syntax color, i.e. for function usage (`Function`
highlight group), `DiffText`, `DiagnosticInfo`, and some minor text UI
elements.
- Red to generally mean high user attention, i.e. errors; in particular
for `ErrorMsg`, `DiffDelete`, `DiagnosticError`.
- Yellow very sparingly only with true colors to mean mild user
attention, i.e. warnings. That is, `DiagnosticWarn` and `WarningMsg`.
- Blue very sparingly only with true colors as `DiagnosticHint` and some
additional important syntax group (like `Identifier`).
- Magenta very carefully (if at all).
------
Notes
- To make tests work without relatively larege updates, each one is
prepended with an equivalent of the call `:colorscheme vim`.
Plus some tests which spawn new Neovim instances also now use 'vim'
color scheme.
In some cases tests are updated to fit new default color scheme.
Problem: [security]: buffer-overflow in ex_substitute
Solution: clear memory after allocating
When allocating the new_start pointer in ex_substitute() the memory
pointer points to some garbage that the following for loop in
ex_cmds.c:4743 confuses and causes it to accessing the new_start pointer
beyond it's size, leading to a buffer-overlow.
So fix this by using alloc_clear() instead of alloc(), which will
clear the memory by NUL and therefore cause the loop to terminate
correctly.
Reported by @henices, thanks!
closes: vim/vim#13596abfa13ebe9
Co-authored-by: Christian Brabandt <cb@256bit.org>
Problem: [security]: stack-buffer-overflow in option callback functions
Solution: pass size of errbuf down the call stack, use snprintf()
instead of sprintf()
We pass the error buffer down to the option callback functions, but in
some parts of the code, we simply use sprintf(buf) to write into the error
buffer, which can overflow.
So let's pass down the length of the error buffer and use sprintf(buf, size)
instead.
Reported by @henices, thanks!
b39b240c38
Co-authored-by: Christian Brabandt <cb@256bit.org>
Problem: [security]: buffer-overflow in suggest_trie_walk
Solution: Check n before using it as index into byts array
Basically, n as an index into the byts array, can point to beyond the byts
array. So let's double check, that n is within the expected range after
incrementing it from sp->ts_curi and bail out if it would be invalid.
Reported by @henices, thanks!
0fb375aae6
Co-authored-by: Christian Brabandt <cb@256bit.org>
Problem: [security]: use-after-free in win-enter
Solution: validate window pointer before calling win_enter()
win_goto() may stop visual mode, if it is active. However, this may in
turn trigger the ModeChanged autocommand, which could potentially free
the wp pointer which was valid before now became stale and points to now
freed memory.
So before calling win_enter(), let's verify one more time, that the
wp pointer still points to a valid window structure.
Reported by @henices, thanks!
eec0c2b3a4
Co-authored-by: Christian Brabandt <cb@256bit.org>
Problem: html.angular ft is problematic
Solution: partly revert v9.0.2137
The html.angular filetype causes issues and does not trigger FileType
autocommands for the html or angular filetypes.
So let's roll back that particular change and detect this only as html
file
related: https://github.com/vim/vim/pull/13594#issuecomment-1834465890closes: vim/vim#136044f3480c943
Co-authored-by: Christian Brabandt <cb@256bit.org>
Problem: ml_get error when scrolling after delete
Solution: mark topline to be validated in main_loop
if it is larger than current buffers line
count
reset_lnums() is called after e.g. TextChanged autocommands and it may
accidentally cause curwin->w_topline to become invalid, e.g. if the
autocommand has deleted some lines.
So verify that curwin->w_topline points to a valid line and if not, mark
the window to have w_topline recalculated in main_loop() in
update_topline() after reset_lnums() returns.
fixes: vim/vim#13568fixes: vim/vim#13578c4ffeddfe5
The error doesn't happen in Nvim because Nvim triggers TextChanged after
calling update_topline().
Co-authored-by: Christian Brabandt <cb@256bit.org>
Problem: No test for mode() when executing Ex commands
Solution: Add some test cases and simplify several other test cases.
Also add a few more test cases for ModeChanged.
closes: vim/vim#13588fcaeb3d42b
Problem: not all nushell files detected
Solution: use *.nu to detect nushell files
closes: vim/vim#13586b9efc72c24
Co-authored-by: Daniel Buch Hansen <boogiewasthere@gmail.com>
Add syntax and filetype plugins for SWIG (Simplified Wrapper Interface
Generator) description files.
The default syntax for .i files highlights comments in a reverse
color scheme which doesn't look well. This syntax builds
on vim's c++ syntax by adding highlighting for common swig
directives and user defined directives. For an alternative
syntax, see vimscript vim/vim#1247 (which I found after writing this).
closes: vim/vim#135622e31065a65
Co-authored-by: Julien Marrec <julien.marrec@gmail.com>
Co-authored-by: Matěj Cepl <mcepl@cepl.eu>
09d4133 changed blocknr_T from long to int64_t, so pe_bnum is now always 64-bit. This was an incompatible change in the swapfile format for 32-bit systems, but there have been no complaints in the past 9 years so just adjust the test.
Problem: Unused assignments when checking the value of 'listchars'.
Solution: Loop only once when just checking the value. Add a test to
check that this change doesn't cause double-free.
closes: vim/vim#1355900624a2fa0
Problem: File info disappears immediately when 'cmdheight' has just
decreased due to switching tabpage and 'shortmess' doesn't
contain 'o' or 'O'.
Solution: Make sure msg_row isn't smaller than cmdline_row.
fixes: vim/vim#13560closes: vim/vim#1356140ed6711bd
Problem: [security]: use-after-free in ex_substitute
Solution: always allocate memory
closes: vim/vim#13552
A recursive :substitute command could cause a heap-use-after free in Vim
(CVE-2023-48706).
The whole reproducible test is a bit tricky, I can only reproduce this
reliably when no previous substitution command has been used yet
(which is the reason, the test needs to run as first one in the
test_substitute.vim file) and as a combination of the `:~` command
together with a :s command that contains the special substitution atom `~\=`
which will make use of a sub-replace special atom and calls a vim script
function.
There was a comment in the existing :s code, that already makes the
`sub` variable allocate memory so that a recursive :s call won't be able
to cause any issues here, so this was known as a potential problem
already. But for the current test-case that one does not work, because
the substitution does not start with `\=` but with `~\=` (and since
there does not yet exist a previous substitution atom, Vim will simply
increment the `sub` pointer (which then was not allocated dynamically)
and later one happily use a sub-replace special expression (which could
then free the `sub` var).
The following commit fixes this, by making the sub var always using
allocated memory, which also means we need to free the pointer whenever
we leave the function. Since sub is now always an allocated variable,
we also do no longer need the sub_copy variable anymore, since this one
was used to indicated when sub pointed to allocated memory (and had
therefore to be freed on exit) and when not.
Github Security Advisory:
https://github.com/vim/vim/security/advisories/GHSA-c8qm-x72m-q53q26c11c5688
Co-authored-by: Christian Brabandt <cb@256bit.org>
Problem: The legacy signlist data structures and associated functions are
redundant since the introduction of extmark signs.
Solution: Store signs defined through the legacy commands in a hashmap, placed
signs in the extmark tree. Replace signlist associated functions.
Usage of the legacy sign commands should yield no change in behavior with the
exception of:
- "orphaned signs" are now always removed when the line it is placed on is
deleted. This used to depend on the value of 'signcolumn'.
- It is no longer possible to place multiple signs with the same identifier
in a single group on multiple lines. This will now move the sign instead.
Moreover, both signs placed through the legacy sign commands and through
|nvim_buf_set_extmark()|:
- Will show up in both |sign-place| and |nvim_buf_get_extmarks()|.
- Are displayed by increasing sign identifier, left to right.
Extmark signs used to be ordered decreasingly as opposed to legacy signs.
Problem: [security]: Use-after-free in win_close()
Solution: Check window is valid, before accessing it
If the current window structure is no longer valid (because a previous
autocommand has already freed this window), fail and return before
attempting to set win->w_closing variable.
Add a test to trigger ASAN in CI
25aabc2b8e
Co-authored-by: Christian Brabandt <cb@256bit.org>
Problem: buffer-overflow in trunc_string()
Solution: Add NULL at end of buffer
Currently trunc_string() assumes that when the string is too long,
buf[e-1] will always be writeable. But that assumption may not always be
true. The condition currently looks like this
else if (e + 3 < buflen)
[...]
else
{
// can't fit in the "...", just truncate it
buf[e - 1] = NUL;
}
but this means, we may run into the last else clause with e still being
larger than buflen. So a buffer overflow occurs.
So instead of using `buf[e - 1]`, let's just always
truncate at `buf[buflen - 1]` which should always be writable.
3bd7fa12e1
vim-patch:9.0.2004: Missing test file
Problem: Missing test file
Solution: git-add the file to the repo
closes: vim/vim#13305d4afbdd071
Co-authored-by: Christian Brabandt <cb@256bit.org>
Problem: Trailing white space in tests
Solution: Delete it
This causes test_codestyle to fail, so we need to remove it again.
Hopefully that makes the CI green again.
Note: I will start using annotated tags from now on.
da200c2f78
Co-authored-by: Christian Brabandt <cb@256bit.org>
Problem: tests running sh have problems
Solution: Check that dash is installed
closes: vim/vim#130401690ec64ff
Co-authored-by: Philip H <47042125+pheiduck@users.noreply.github.com>
Co-authored-by: Christian Brabandt <cb@256bit.org>
Problem: heap-buffer-overflow in vim_regsub_both
Solution: Disallow exchanging windows when textlock is active
f6d28fe2c9
Cherry-pick test_crash.vim change from patch 9.0.1876.
Co-authored-by: Christian Brabandt <cb@256bit.org>
Problem: crash with bt_quickfix1_poc when cleaning up
and EXITFREE is defined
Solution: Test if buffer is valid in a window, else close
window directly, don't try to access buffer properties
While at it, increase the crash timeout slightly, so that CI has a
chance to finish processing the test_crash() test.
623ba31821
Co-authored-by: Christian Brabandt <cb@256bit.org>
Problem: heap-use-after-free in bt_normal()
Solution: check that buffer is still valid
6e60cf444a
Test change only.
Co-authored-by: Christian Brabandt <cb@256bit.org>
Problem: heap use after free in ins_compl_get_exp()
Solution: validate buffer before accessing it
ee9166eb3b
Co-authored-by: Christian Brabandt <cb@256bit.org>
Problem: heap-use-after-free in is_qf_win()
Solution: Check buffer is valid before accessing it
fc68299d43
Co-authored-by: Christian Brabandt <cb@256bit.org>
Problem: test_crash1() fails on CI
Solution: don't run Screendump test, verify that it doesn't crash
by running it through a shell command line, testing
the exit value and concatenating success cmd using '&&'
db510ca805
Co-authored-by: Christian Brabandt <cb@256bit.org>
Problem: [security]: FPE in adjust_plines_for_skipcol
Solution: don't divide by zero, return zero
Prevent a floating point exception when calculating w_skipcol (which can
happen with a small window when the number option is set and cpo+=n).
Add a test to verify
cb0b99f067
Co-authored-by: Christian Brabandt <cb@256bit.org>
Problem: [security]: overflow in get_number
Solution: Return 0 when the count gets too large
[security]: overflow in get_number
When using the z= command, we may overflow the count with values larger
than MAX_INT. So verify that we do not overflow and in case when an
overflow is detected, simply return 0
73b2d3790c
Co-authored-by: Christian Brabandt <cb@256bit.org>
Problem: [security]: overflow in ex address parsing
Solution: Verify that lnum is positive, before substracting from
LONG_MAX
[security]: overflow in ex address parsing
When parsing relative ex addresses one may unintentionally cause an
overflow (because LONG_MAX - lnum will overflow for negative addresses).
So verify that lnum is actually positive before doing the overflow
check.
060623e4a3
Co-authored-by: Christian Brabandt <cb@256bit.org>