packages/vim
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity
vim/backport-CVE-2024-22667.patch
wangjiang ee33628b4c fix CVE-2024-22667 
(cherry picked from commit 895a09597e0085952f227551dd7a6891c49c5727)
2024-07-09 11:40:05 +08:00

371 lines
12 KiB
Diff
Raw Blame History

From b39b240c386a5a29241415541f1c99e2e6b8ce47 Mon Sep 17 00:00:00 2001
From: Christian Brabandt <cb@256bit.org>
Date: Wed, 29 Nov 2023 11:34:05 +0100
Subject: [PATCH] patch 9.0.2142: [security]: stack-buffer-overflow in option
callback functions
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!
Signed-off-by: Christian Brabandt <cb@256bit.org>
---
src/map.c | 2 +-
src/option.c | 14 ++++---
src/option.h | 2 +
src/optionstr.c | 59 +++++++++++++++++----------
src/proto/optionstr.pro | 4 +-
src/structs.h | 2 +
6 files changed, 52 insertions(+), 31 deletions(-)
diff --git a/src/map.c b/src/map.c
index 5988445bd1588..98785e722ce53 100644
--- a/src/map.c
+++ b/src/map.c
@@ -3114,7 +3114,7 @@ did_set_langmap(optset_T *args UNUSED)
{
if (p[0] != ',')
{
- sprintf(args->os_errbuf,
+ snprintf(args->os_errbuf, args->os_errbuflen,
_(e_langmap_extra_characters_after_semicolon_str),
p);
return args->os_errbuf;
diff --git a/src/option.c b/src/option.c
index d5d20d7674aa6..572788559cdfd 100644
--- a/src/option.c
+++ b/src/option.c
@@ -1932,6 +1932,7 @@ do_set_option_string(
int cp_val,
char_u *varp_arg,
char *errbuf,
+ int errbuflen,
int *value_checked,
char **errmsg)
{
@@ -2030,7 +2031,7 @@ do_set_option_string(
// be triggered that can cause havoc.
*errmsg = did_set_string_option(
opt_idx, (char_u **)varp, oldval, newval, errbuf,
- opt_flags, op, value_checked);
+ errbuflen, opt_flags, op, value_checked);
secure = secure_saved;
}
@@ -2287,7 +2288,7 @@ do_set_option_value(
{
// string option
if (do_set_option_string(opt_idx, opt_flags, &arg, nextchar, op,
- flags, cp_val, varp, errbuf,
+ flags, cp_val, varp, errbuf, errbuflen,
&value_checked, &errmsg) == FAIL)
{
if (errmsg != NULL)
@@ -2579,12 +2580,12 @@ do_set(
{
int stopopteval = FALSE;
char *errmsg = NULL;
- char errbuf[80];
+ char errbuf[ERR_BUFLEN];
char_u *startarg = arg;
errmsg = do_set_option(opt_flags, &arg, arg_start, &startarg,
&did_show, &stopopteval, errbuf,
- sizeof(errbuf));
+ ERR_BUFLEN);
if (stopopteval)
break;
@@ -5347,7 +5348,8 @@ set_option_value(
int opt_idx;
char_u *varp;
long_u flags;
- static char errbuf[80];
+ static char errbuf[ERR_BUFLEN];
+ int errbuflen = ERR_BUFLEN;
opt_idx = findoption(name);
if (opt_idx < 0)
@@ -5390,7 +5392,7 @@ set_option_value(
}
#endif
if (flags & P_STRING)
- return set_string_option(opt_idx, string, opt_flags, errbuf);
+ return set_string_option(opt_idx, string, opt_flags, errbuf, errbuflen);
varp = get_varp_scope(&(options[opt_idx]), opt_flags);
if (varp != NULL) // hidden option is not changed
diff --git a/src/option.h b/src/option.h
index 2d1ca2cdeaf66..646056bf111b4 100644
--- a/src/option.h
+++ b/src/option.h
@@ -1321,4 +1321,6 @@ enum
// Value for b_p_ul indicating the global value must be used.
#define NO_LOCAL_UNDOLEVEL (-123456)
+#define ERR_BUFLEN 80
+
#endif // _OPTION_H_
diff --git a/src/optionstr.c b/src/optionstr.c
index b7cdcc4511c7c..84c77cb0a0e25 100644
--- a/src/optionstr.c
+++ b/src/optionstr.c
@@ -229,11 +229,12 @@ trigger_optionset_string(
#endif
static char *
-illegal_char(char *errbuf, int c)
+illegal_char(char *errbuf, int errbuflen, int c)
{
if (errbuf == NULL)
return "";
- sprintf((char *)errbuf, _(e_illegal_character_str), (char *)transchar(c));
+ snprintf((char *)errbuf, errbuflen, _(e_illegal_character_str),
+ (char *)transchar(c));
return errbuf;
}
@@ -525,7 +526,8 @@ set_string_option(
int opt_idx,
char_u *value,
int opt_flags, // OPT_LOCAL and/or OPT_GLOBAL
- char *errbuf)
+ char *errbuf,
+ int errbuflen)
{
char_u *s;
char_u **varp;
@@ -579,7 +581,7 @@ set_string_option(
}
#endif
if ((errmsg = did_set_string_option(opt_idx, varp, oldval, value, errbuf,
- opt_flags, OP_NONE, &value_checked)) == NULL)
+ errbuflen, opt_flags, OP_NONE, &value_checked)) == NULL)
did_set_option(opt_idx, opt_flags, TRUE, value_checked);
#if defined(FEAT_EVAL)
@@ -615,7 +617,8 @@ valid_filetype(char_u *val)
check_stl_option(char_u *s)
{
int groupdepth = 0;
- static char errbuf[80];
+ static char errbuf[ERR_BUFLEN];
+ int errbuflen = ERR_BUFLEN;
while (*s)
{
@@ -656,7 +659,7 @@ check_stl_option(char_u *s)
}
if (vim_strchr(STL_ALL, *s) == NULL)
{
- return illegal_char(errbuf, *s);
+ return illegal_char(errbuf, errbuflen, *s);
}
if (*s == '{')
{
@@ -664,7 +667,7 @@ check_stl_option(char_u *s)
if (reevaluate && *++s == '}')
// "}" is not allowed immediately after "%{%"
- return illegal_char(errbuf, '}');
+ return illegal_char(errbuf, errbuflen, '}');
while ((*s != '}' || (reevaluate && s[-1] != '%')) && *s)
s++;
if (*s != '}')
@@ -719,13 +722,17 @@ did_set_opt_strings(char_u *val, char **values, int list)
* An option which is a list of flags is set. Valid values are in 'flags'.
*/
static char *
-did_set_option_listflag(char_u *val, char_u *flags, char *errbuf)
+did_set_option_listflag(
+ char_u *val,
+ char_u *flags,
+ char *errbuf,
+ int errbuflen)
{
char_u *s;
for (s = val; *s; ++s)
if (vim_strchr(flags, *s) == NULL)
- return illegal_char(errbuf, *s);
+ return illegal_char(errbuf, errbuflen, *s);
return NULL;
}
@@ -1461,7 +1468,7 @@ did_set_comments(optset_T *args)
if (vim_strchr((char_u *)COM_ALL, *s) == NULL
&& !VIM_ISDIGIT(*s) && *s != '-')
{
- errmsg = illegal_char(args->os_errbuf, *s);
+ errmsg = illegal_char(args->os_errbuf, args->os_errbuflen, *s);
break;
}
++s;
@@ -1517,7 +1524,7 @@ did_set_complete(optset_T *args)
if (!*s)
break;
if (vim_strchr((char_u *)".wbuksid]tU", *s) == NULL)
- return illegal_char(args->os_errbuf, *s);
+ return illegal_char(args->os_errbuf, args->os_errbuflen, *s);
if (*++s != NUL && *s != ',' && *s != ' ')
{
if (s[-1] == 'k' || s[-1] == 's')
@@ -1534,7 +1541,7 @@ did_set_complete(optset_T *args)
{
if (args->os_errbuf != NULL)
{
- sprintf((char *)args->os_errbuf,
+ snprintf((char *)args->os_errbuf, args->os_errbuflen,
_(e_illegal_character_after_chr), *--s);
return args->os_errbuf;
}
@@ -1634,7 +1641,8 @@ did_set_concealcursor(optset_T *args)
{
char_u **varp = (char_u **)args->os_varp;
- return did_set_option_listflag(*varp, (char_u *)COCU_ALL, args->os_errbuf);
+ return did_set_option_listflag(*varp, (char_u *)COCU_ALL, args->os_errbuf,
+ args->os_errbuflen);
}
int
@@ -1652,7 +1660,8 @@ did_set_cpoptions(optset_T *args)
{
char_u **varp = (char_u **)args->os_varp;
- return did_set_option_listflag(*varp, (char_u *)CPO_ALL, args->os_errbuf);
+ return did_set_option_listflag(*varp, (char_u *)CPO_ALL, args->os_errbuf,
+ args->os_errbuflen);
}
int
@@ -2281,7 +2290,8 @@ did_set_formatoptions(optset_T *args)
{
char_u **varp = (char_u **)args->os_varp;
- return did_set_option_listflag(*varp, (char_u *)FO_ALL, args->os_errbuf);
+ return did_set_option_listflag(*varp, (char_u *)FO_ALL, args->os_errbuf,
+ args->os_errbuflen);
}
int
@@ -2422,7 +2432,8 @@ did_set_guioptions(optset_T *args)
char_u **varp = (char_u **)args->os_varp;
char *errmsg;
- errmsg = did_set_option_listflag(*varp, (char_u *)GO_ALL, args->os_errbuf);
+ errmsg = did_set_option_listflag(*varp, (char_u *)GO_ALL, args->os_errbuf,
+ args->os_errbuflen);
if (errmsg != NULL)
return errmsg;
@@ -2926,8 +2937,8 @@ did_set_mouse(optset_T *args)
{
char_u **varp = (char_u **)args->os_varp;
- return did_set_option_listflag(*varp, (char_u *)MOUSE_ALL,
- args->os_errbuf);
+ return did_set_option_listflag(*varp, (char_u *)MOUSE_ALL, args->os_errbuf,
+ args->os_errbuflen);
}
int
@@ -3364,7 +3375,8 @@ did_set_shortmess(optset_T *args)
{
char_u **varp = (char_u **)args->os_varp;
- return did_set_option_listflag(*varp, (char_u *)SHM_ALL, args->os_errbuf);
+ return did_set_option_listflag(*varp, (char_u *)SHM_ALL, args->os_errbuf,
+ args->os_errbuflen);
}
int
@@ -4030,7 +4042,7 @@ did_set_viminfo(optset_T *args)
// Check it's a valid character
if (vim_strchr((char_u *)"!\"%'/:<@cfhnrs", *s) == NULL)
{
- errmsg = illegal_char(args->os_errbuf, *s);
+ errmsg = illegal_char(args->os_errbuf, args->os_errbuflen, *s);
break;
}
if (*s == 'n') // name is always last one
@@ -4057,7 +4069,7 @@ did_set_viminfo(optset_T *args)
{
if (args->os_errbuf != NULL)
{
- sprintf(args->os_errbuf,
+ snprintf(args->os_errbuf, args->os_errbuflen,
_(e_missing_number_after_angle_str_angle),
transchar_byte(*(s - 1)));
errmsg = args->os_errbuf;
@@ -4140,7 +4152,8 @@ did_set_whichwrap(optset_T *args)
// Add ',' to the list flags because 'whichwrap' is a flag
// list that is comma-separated.
- return did_set_option_listflag(*varp, (char_u *)(WW_ALL ","), args->os_errbuf);
+ return did_set_option_listflag(*varp, (char_u *)(WW_ALL ","),
+ args->os_errbuf, args->os_errbuflen);
}
int
@@ -4341,6 +4354,7 @@ did_set_string_option(
char_u *oldval, // previous value of the option
char_u *value, // new value of the option
char *errbuf, // buffer for errors, or NULL
+ int errbuflen, // length of error buffer
int opt_flags, // OPT_LOCAL and/or OPT_GLOBAL
set_op_T op, // OP_ADDING/OP_PREPENDING/OP_REMOVING
int *value_checked) // value was checked to be safe, no
@@ -4385,6 +4399,7 @@ did_set_string_option(
args.os_oldval.string = oldval;
args.os_newval.string = value;
args.os_errbuf = errbuf;
+ args.os_errbuflen = errbuflen;
// Invoke the option specific callback function to validate and apply
// the new option value.
errmsg = did_set_cb(&args);
diff --git a/src/proto/optionstr.pro b/src/proto/optionstr.pro
index 22601ba996b8f..4ce93212dd948 100644
--- a/src/proto/optionstr.pro
+++ b/src/proto/optionstr.pro
@@ -8,7 +8,7 @@ void check_string_option(char_u **pp);
void set_string_option_direct(char_u *name, int opt_idx, char_u *val, int opt_flags, int set_sid);
void set_string_option_direct_in_win(win_T *wp, char_u *name, int opt_idx, char_u *val, int opt_flags, int set_sid);
void set_string_option_direct_in_buf(buf_T *buf, char_u *name, int opt_idx, char_u *val, int opt_flags, int set_sid);
-char *set_string_option(int opt_idx, char_u *value, int opt_flags, char *errbuf);
+char *set_string_option(int opt_idx, char_u *value, int opt_flags, char *errbuf, int errbuflen);
char *did_set_ambiwidth(optset_T *args);
char *did_set_background(optset_T *args);
char *did_set_backspace(optset_T *args);
@@ -121,7 +121,7 @@ char *did_set_wildmode(optset_T *args);
char *did_set_wildoptions(optset_T *args);
char *did_set_winaltkeys(optset_T *args);
char *did_set_wincolor(optset_T *args);
-char *did_set_string_option(int opt_idx, char_u **varp, char_u *oldval, char_u *value, char *errbuf, int opt_flags, set_op_T op, int *value_checked);
+char *did_set_string_option(int opt_idx, char_u **varp, char_u *oldval, char_u *value, char *errbuf, int errbuflen, int opt_flags, set_op_T op, int *value_checked);
int expand_set_ambiwidth(optexpand_T *args, int *numMatches, char_u ***matches);
int expand_set_background(optexpand_T *args, int *numMatches, char_u ***matches);
int expand_set_backspace(optexpand_T *args, int *numMatches, char_u ***matches);
diff --git a/src/structs.h b/src/structs.h
index 4e081b860ef45..6d9dcbb2ab17d 100644
--- a/src/structs.h
+++ b/src/structs.h
@@ -4968,6 +4968,8 @@ typedef struct
// is parameterized, then the "os_errbuf" buffer is used to store the error
// message (when it is not NULL).
char *os_errbuf;
+ // length of the error buffer
+ int os_errbuflen;
} optset_T;
/*
Reference in New Issue View Git Blame Copy Permalink