From 0309d3fbf0edc5ac958964f85dff76719340c4c7 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Mon, 3 Feb 2025 11:11:46 +0800 Subject: vim-patch:8.2.0825: def_function() may return pointer that was freed Problem: def_function() may return pointer that was freed. Solution: Set "fp" to NULL after freeing it. https://github.com/vim/vim/commit/a14e6975478adeddcc2161edc1ec611016aa89f3 Co-authored-by: Bram Moolenaar --- src/nvim/eval/userfunc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/nvim/eval/userfunc.c b/src/nvim/eval/userfunc.c index 2e549fcf37..da91de4650 100644 --- a/src/nvim/eval/userfunc.c +++ b/src/nvim/eval/userfunc.c @@ -2867,6 +2867,7 @@ void ex_function(exarg_T *eap) if (tv_dict_add(fudi.fd_dict, fudi.fd_di) == FAIL) { xfree(fudi.fd_di); xfree(fp); + fp = NULL; goto erret; } } else { @@ -2887,6 +2888,7 @@ void ex_function(exarg_T *eap) hi->hi_key = UF2HIKEY(fp); } else if (hash_add(&func_hashtab, UF2HIKEY(fp)) == FAIL) { xfree(fp); + fp = NULL; goto erret; } fp->uf_refcount = 1; -- cgit From cd42740245b5dd25ef9c7e116656d6da630f5db0 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Mon, 3 Feb 2025 10:15:09 +0800 Subject: vim-patch:8.2.1445: Vim9: function expanded name is cleared when sourcing again Problem: Vim9: function expanded name is cleared when sourcing a script again. Solution: Only clear the expanded name when deleting the function. (closes vim/vim#6707) https://github.com/vim/vim/commit/c4ce36d48698669f81ec90f7c9dc9ab8c362e538 Co-authored-by: Bram Moolenaar --- src/nvim/eval/userfunc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/nvim/eval/userfunc.c b/src/nvim/eval/userfunc.c index da91de4650..5156e431db 100644 --- a/src/nvim/eval/userfunc.c +++ b/src/nvim/eval/userfunc.c @@ -891,7 +891,6 @@ static void func_clear_items(ufunc_T *fp) ga_clear_strings(&(fp->uf_args)); ga_clear_strings(&(fp->uf_def_args)); ga_clear_strings(&(fp->uf_lines)); - XFREE_CLEAR(fp->uf_name_exp); if (fp->uf_flags & FC_LUAREF) { api_free_luaref(fp->uf_luaref); @@ -930,6 +929,8 @@ static void func_free(ufunc_T *fp) if ((fp->uf_flags & (FC_DELETED | FC_REMOVED)) == 0) { func_remove(fp); } + + XFREE_CLEAR(fp->uf_name_exp); xfree(fp); } -- cgit From 638c6b406bc41d4fed5ef282bae526888de8229a Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Mon, 3 Feb 2025 10:21:57 +0800 Subject: vim-patch:8.2.2505: Vim9: crash after defining function with invalid return type Problem: Vim9: crash after defining function with invalid return type. Solution: Clear function growarrays. Fix memory leak. https://github.com/vim/vim/commit/31842cd0772b557eb9584a13740430db29de8a51 Cherry-pick free_fp from patch 8.2.3812. Co-authored-by: Bram Moolenaar --- src/nvim/eval/userfunc.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/nvim/eval/userfunc.c b/src/nvim/eval/userfunc.c index 5156e431db..3538493159 100644 --- a/src/nvim/eval/userfunc.c +++ b/src/nvim/eval/userfunc.c @@ -2515,7 +2515,8 @@ void ex_function(exarg_T *eap) garray_T newlines; int varargs = false; int flags = 0; - ufunc_T *fp; + ufunc_T *fp = NULL; + bool free_fp = false; bool overwrite = false; funcdict_T fudi; static int func_nr = 0; // number for nameless function @@ -2888,8 +2889,7 @@ void ex_function(exarg_T *eap) hashitem_T *hi = hash_find(&func_hashtab, name); hi->hi_key = UF2HIKEY(fp); } else if (hash_add(&func_hashtab, UF2HIKEY(fp)) == FAIL) { - xfree(fp); - fp = NULL; + free_fp = true; goto erret; } fp->uf_refcount = 1; @@ -2920,8 +2920,16 @@ void ex_function(exarg_T *eap) erret: ga_clear_strings(&newargs); ga_clear_strings(&default_args); + if (fp != NULL) { + ga_init(&fp->uf_args, (int)sizeof(char *), 1); + ga_init(&fp->uf_def_args, (int)sizeof(char *), 1); + } errret_2: ga_clear_strings(&newlines); + if (free_fp) { + xfree(fp); + fp = NULL; + } ret_free: xfree(line_to_free); xfree(fudi.fd_newkey); -- cgit From 82b029cbb00dbe9649824f7795c177974955d683 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Mon, 3 Feb 2025 10:45:50 +0800 Subject: vim-patch:9.0.1142: crash and/or memory leak when redefining function Problem: Crash and/or memory leak when redefining function after error. Solution: Clear pointer after making a copy. Clear arrays on failure. (closes vim/vim#11774) https://github.com/vim/vim/commit/f057171d8b562c72334fd7c15c89ff787358ce3a Co-authored-by: Bram Moolenaar --- src/nvim/eval/userfunc.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/nvim/eval/userfunc.c b/src/nvim/eval/userfunc.c index 3538493159..6eaccbae62 100644 --- a/src/nvim/eval/userfunc.c +++ b/src/nvim/eval/userfunc.c @@ -2918,13 +2918,14 @@ void ex_function(exarg_T *eap) goto ret_free; erret: - ga_clear_strings(&newargs); - ga_clear_strings(&default_args); if (fp != NULL) { + // these were set to "newargs" and "default_args", which are cleared below ga_init(&fp->uf_args, (int)sizeof(char *), 1); ga_init(&fp->uf_def_args, (int)sizeof(char *), 1); } errret_2: + ga_clear_strings(&newargs); + ga_clear_strings(&default_args); ga_clear_strings(&newlines); if (free_fp) { xfree(fp); -- cgit From b853ef770a25fcd91def6c5016d65c336897c2cc Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Sun, 2 Feb 2025 08:23:48 +0800 Subject: vim-patch:9.1.1063: too many strlen() calls in userfunc.c Problem: too many strlen() calls in userfunc.c Solution: refactor userfunc.c and remove calls to strlen(), drop set_ufunc_name() and roll it into alloc_ufunc(), check for out-of-memory condition in trans_function_name_ext() (John Marriott) closes: vim/vim#16537 https://github.com/vim/vim/commit/b32800f7c51c866dc0e87244eb4902540982309d Add missing change to call_user_func() from patch 8.1.1007. Consistently use PRIdSCID instead of PRId64 for script IDs. Co-authored-by: John Marriott --- src/nvim/eval/typval.c | 2 +- src/nvim/eval/typval_defs.h | 7 +- src/nvim/eval/userfunc.c | 184 +++++++++++++++++++++++++++----------------- src/nvim/ex_docmd.c | 3 +- src/nvim/keycodes.c | 2 +- 5 files changed, 122 insertions(+), 76 deletions(-) diff --git a/src/nvim/eval/typval.c b/src/nvim/eval/typval.c index 48b2e82c0a..f9cf245e50 100644 --- a/src/nvim/eval/typval.c +++ b/src/nvim/eval/typval.c @@ -2660,7 +2660,7 @@ int tv_dict_add_func(dict_T *const d, const char *const key, const size_t key_le dictitem_T *const item = tv_dict_item_alloc_len(key, key_len); item->di_tv.v_type = VAR_FUNC; - item->di_tv.vval.v_string = xstrdup(fp->uf_name); + item->di_tv.vval.v_string = xmemdupz(fp->uf_name, fp->uf_namelen); if (tv_dict_add(d, item) == FAIL) { tv_dict_item_free(item); return FAIL; diff --git a/src/nvim/eval/typval_defs.h b/src/nvim/eval/typval_defs.h index 1ec7631c4b..90b0d4e4a9 100644 --- a/src/nvim/eval/typval_defs.h +++ b/src/nvim/eval/typval_defs.h @@ -357,9 +357,10 @@ struct ufunc { funccall_T *uf_scoped; ///< l: local variables for closure char *uf_name_exp; ///< if "uf_name[]" starts with SNR the name with ///< "" as a string, otherwise NULL - char uf_name[]; ///< Name of function (actual size equals name); - ///< can start with 123_ - ///< ( is K_SPECIAL KS_EXTRA KE_SNR) + size_t uf_namelen; ///< Length of uf_name (excluding the NUL) + char uf_name[]; ///< Name of function (actual size equals name); + ///< can start with 123_ + ///< ( is K_SPECIAL KS_EXTRA KE_SNR) }; struct partial_S { diff --git a/src/nvim/eval/userfunc.c b/src/nvim/eval/userfunc.c index 6eaccbae62..6e7d921068 100644 --- a/src/nvim/eval/userfunc.c +++ b/src/nvim/eval/userfunc.c @@ -264,25 +264,47 @@ static void register_closure(ufunc_T *fp) [current_funccal->fc_ufuncs.ga_len++] = fp; } +static char lambda_name[8 + NUMBUFLEN]; +static size_t lambda_namelen = 0; + /// @return a name for a lambda. Returned in static memory. char *get_lambda_name(void) { - static char name[30]; static int lambda_no = 0; - snprintf(name, sizeof(name), "%d", ++lambda_no); - return name; + int n = snprintf(lambda_name, sizeof(lambda_name), "%d", ++lambda_no); + if (n < 1) { + lambda_namelen = 0; + } else if (n >= (int)sizeof(lambda_name)) { + lambda_namelen = sizeof(lambda_name) - 1; + } else { + lambda_namelen = (size_t)n; + } + + return lambda_name; +} + +/// Get the length of the last lambda name. +size_t get_lambda_name_len(void) +{ + return lambda_namelen; } -static void set_ufunc_name(ufunc_T *fp, char *name) +/// Allocate a "ufunc_T" for a function called "name". +static ufunc_T *alloc_ufunc(const char *name, size_t namelen) { + size_t len = offsetof(ufunc_T, uf_name) + namelen + 1; + ufunc_T *fp = xcalloc(1, len); STRCPY(fp->uf_name, name); + fp->uf_namelen = namelen; if ((uint8_t)name[0] == K_SPECIAL) { - fp->uf_name_exp = xmalloc(strlen(name) + 3); - STRCPY(fp->uf_name_exp, ""); - strcat(fp->uf_name_exp, fp->uf_name + 3); + len = namelen + 3; + fp->uf_name_exp = xmalloc(len); + snprintf(fp->uf_name_exp, len, "%s", fp->uf_name + 3); } + + return fp; } /// Parse a lambda expression and get a Funcref from "*arg". @@ -350,8 +372,9 @@ int get_lambda_tv(char **arg, typval_T *rettv, evalarg_T *evalarg) garray_T newlines; char *name = get_lambda_name(); + size_t namelen = get_lambda_name_len(); - fp = xcalloc(1, offsetof(ufunc_T, uf_name) + strlen(name) + 1); + fp = alloc_ufunc(name, namelen); pt = xcalloc(1, sizeof(partial_T)); ga_init(&newlines, (int)sizeof(char *), 1); @@ -369,7 +392,6 @@ int get_lambda_tv(char **arg, typval_T *rettv, evalarg_T *evalarg) } fp->uf_refcount = 1; - set_ufunc_name(fp, name); hash_add(&func_hashtab, UF2HIKEY(fp)); fp->uf_args = newargs; ga_init(&fp->uf_def_args, (int)sizeof(char *), 1); @@ -409,7 +431,10 @@ int get_lambda_tv(char **arg, typval_T *rettv, evalarg_T *evalarg) errret: ga_clear_strings(&newargs); - xfree(fp); + if (fp != NULL) { + xfree(fp->uf_name_exp); + xfree(fp); + } xfree(pt); if (evalarg != NULL && evalarg->eval_tofree == NULL) { evalarg->eval_tofree = tofree; @@ -627,33 +652,36 @@ static char *fname_trans_sid(const char *const name, char *const fname_buf, char int *const error) FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT { - const int llen = eval_fname_script(name); - if (llen == 0) { + const char *script_name = name + eval_fname_script(name); + if (script_name == name) { return (char *)name; // no prefix } fname_buf[0] = (char)K_SPECIAL; fname_buf[1] = (char)KS_EXTRA; fname_buf[2] = KE_SNR; - int i = 3; - if (eval_fname_sid(name)) { // "" or "s:" + size_t fname_buflen = 3; + if (!eval_fname_sid(name)) { // "" or "s:" + fname_buf[fname_buflen] = NUL; + } else { if (current_sctx.sc_sid <= 0) { *error = FCERR_SCRIPT; } else { - snprintf(fname_buf + i, (size_t)(FLEN_FIXED + 1 - i), "%" PRId64 "_", - (int64_t)current_sctx.sc_sid); - i = (int)strlen(fname_buf); + fname_buflen += (size_t)snprintf(fname_buf + fname_buflen, + FLEN_FIXED + 1 - fname_buflen, + "%" PRIdSCID "_", + current_sctx.sc_sid); } } + size_t fnamelen = fname_buflen + strlen(script_name); char *fname; - if ((size_t)i + strlen(name + llen) < FLEN_FIXED) { - STRCPY(fname_buf + i, name + llen); + if (fnamelen < FLEN_FIXED) { + STRCPY(fname_buf + fname_buflen, script_name); fname = fname_buf; } else { - fname = xmalloc((size_t)i + strlen(name + llen) + 1); + fname = xmalloc(fnamelen + 1); *tofree = fname; - memmove(fname, fname_buf, (size_t)i); - STRCPY(fname + i, name + llen); + snprintf(fname, fnamelen + 1, "%s%s", fname_buf, script_name); } return fname; } @@ -711,20 +739,20 @@ ufunc_T *find_func(const char *name) /// Copy the function name of "fp" to buffer "buf". /// "buf" must be able to hold the function name plus three bytes. /// Takes care of script-local function names. -static void cat_func_name(char *buf, size_t buflen, ufunc_T *fp) +static int cat_func_name(char *buf, size_t bufsize, ufunc_T *fp) { int len = -1; - size_t uflen = strlen(fp->uf_name); + size_t uflen = fp->uf_namelen; assert(uflen > 0); if ((uint8_t)fp->uf_name[0] == K_SPECIAL && uflen > 3) { - len = snprintf(buf, buflen, "%s", fp->uf_name + 3); + len = snprintf(buf, bufsize, "%s", fp->uf_name + 3); } else { - len = snprintf(buf, buflen, "%s", fp->uf_name); + len = snprintf(buf, bufsize, "%s", fp->uf_name); } - (void)len; // Avoid unused warning on release builds assert(len > 0); + return (len >= (int)bufsize) ? (int)bufsize - 1 : len; } /// Add a number variable "name" to dict "dp" with value "nr". @@ -984,6 +1012,7 @@ void call_user_func(ufunc_T *fp, int argcount, typval_T *argvars, typval_T *rett bool islambda = false; char numbuf[NUMBUFLEN]; char *name; + size_t namelen; typval_T *tv_to_free[MAX_FUNC_ARGS]; int tv_to_free_len = 0; proftime_T wait_start; @@ -1105,23 +1134,25 @@ void call_user_func(ufunc_T *fp, int argcount, typval_T *argvars, typval_T *rett break; } } + + namelen = strlen(name); } else { if ((fp->uf_flags & FC_NOARGS) != 0) { // Bail out if no a: arguments used (in lambda). break; } // "..." argument a:1, a:2, etc. - snprintf(numbuf, sizeof(numbuf), "%d", ai + 1); + namelen = (size_t)snprintf(numbuf, sizeof(numbuf), "%d", ai + 1); name = numbuf; } - if (fixvar_idx < FIXVAR_CNT && strlen(name) <= VAR_SHORT_LEN) { + if (fixvar_idx < FIXVAR_CNT && namelen <= VAR_SHORT_LEN) { v = (dictitem_T *)&fc->fc_fixvar[fixvar_idx++]; v->di_flags = DI_FLAGS_RO | DI_FLAGS_FIX; + STRCPY(v->di_key, name); } else { - v = xmalloc(sizeof(dictitem_T) + strlen(name)); - v->di_flags = DI_FLAGS_RO | DI_FLAGS_FIX | DI_FLAGS_ALLOC; + v = tv_dict_item_alloc_len(name, namelen); + v->di_flags |= DI_FLAGS_RO | DI_FLAGS_FIX; } - STRCPY(v->di_key, name); // Note: the values are copied directly to avoid alloc/free. // "argvars" must have VAR_FIXED for v_lock. @@ -2096,7 +2127,7 @@ char *trans_function_name(char **pp, bool skip, int flags, funcdict_T *fdp, part len = (int)(end - lv.ll_name); } - size_t sid_buf_len = 0; + size_t sid_buflen = 0; char sid_buf[20]; // Copy the function name to allocated memory. @@ -2106,15 +2137,16 @@ char *trans_function_name(char **pp, bool skip, int flags, funcdict_T *fdp, part lead = 0; // do nothing } else if (lead > 0) { lead = 3; - if ((lv.ll_exp_name != NULL && eval_fname_sid(lv.ll_exp_name)) || eval_fname_sid(*pp)) { + if ((lv.ll_exp_name != NULL && eval_fname_sid(lv.ll_exp_name)) + || eval_fname_sid(*pp)) { // It's "s:" or "". if (current_sctx.sc_sid <= 0) { emsg(_(e_usingsid)); goto theend; } - sid_buf_len = - (size_t)snprintf(sid_buf, sizeof(sid_buf), "%" PRIdSCID "_", current_sctx.sc_sid); - lead += (int)sid_buf_len; + sid_buflen = (size_t)snprintf(sid_buf, sizeof(sid_buf), "%" PRIdSCID "_", + current_sctx.sc_sid); + lead += (int)sid_buflen; } } else if (!(flags & TFN_INT) && builtin_function(lv.ll_name, (int)lv.ll_name_len)) { semsg(_("E128: Function name must start with a capital or \"s:\": %s"), @@ -2136,8 +2168,8 @@ char *trans_function_name(char **pp, bool skip, int flags, funcdict_T *fdp, part name[0] = (char)K_SPECIAL; name[1] = (char)KS_EXTRA; name[2] = KE_SNR; - if (sid_buf_len > 0) { // If it's "" - memcpy(name + 3, sid_buf, sid_buf_len); + if (sid_buflen > 0) { // If it's "" + memcpy(name + 3, sid_buf, sid_buflen); } } memmove(name + lead, lv.ll_name, (size_t)len); @@ -2173,12 +2205,12 @@ char *get_scriptlocal_funcname(char *funcname) char sid_buf[25]; // Expand s: and prefix into nr_ - snprintf(sid_buf, sizeof(sid_buf), "%" PRId64 "_", - (int64_t)current_sctx.sc_sid); + size_t sid_buflen = (size_t)snprintf(sid_buf, sizeof(sid_buf), "%" PRIdSCID "_", + current_sctx.sc_sid); const int off = *funcname == 's' ? 2 : 5; - char *newname = xmalloc(strlen(sid_buf) + strlen(funcname + off) + 1); - STRCPY(newname, sid_buf); - strcat(newname, funcname + off); + size_t newnamesize = sid_buflen + strlen(funcname + off) + 1; + char *newname = xmalloc(newnamesize); + snprintf(newname, newnamesize, "%s%s", sid_buf, funcname + off); return newname; } @@ -2250,6 +2282,7 @@ static int get_function_body(exarg_T *eap, garray_T *newlines, char *line_arg_in int ret = FAIL; bool is_heredoc = false; char *heredoc_trimmed = NULL; + size_t heredoc_trimmedlen = 0; bool do_concat = true; while (true) { @@ -2313,19 +2346,18 @@ static int get_function_body(exarg_T *eap, garray_T *newlines, char *line_arg_in // * ":let {var-name} =<< [trim] {marker}" and "{marker}" if (heredoc_trimmed == NULL || (is_heredoc && skipwhite(theline) == theline) - || strncmp(theline, heredoc_trimmed, - strlen(heredoc_trimmed)) == 0) { + || strncmp(theline, heredoc_trimmed, heredoc_trimmedlen) == 0) { if (heredoc_trimmed == NULL) { p = theline; } else if (is_heredoc) { - p = skipwhite(theline) == theline - ? theline : theline + strlen(heredoc_trimmed); + p = skipwhite(theline) == theline ? theline : theline + heredoc_trimmedlen; } else { - p = theline + strlen(heredoc_trimmed); + p = theline + heredoc_trimmedlen; } if (strcmp(p, skip_until) == 0) { XFREE_CLEAR(skip_until); XFREE_CLEAR(heredoc_trimmed); + heredoc_trimmedlen = 0; do_concat = true; is_heredoc = false; } @@ -2402,7 +2434,7 @@ static int get_function_body(exarg_T *eap, garray_T *newlines, char *line_arg_in && (!ASCII_ISALPHA(p[1]) || (p[1] == 'n' && (!ASCII_ISALPHA(p[2]) || (p[2] == 's')))))) { - skip_until = xstrdup("."); + skip_until = xmemdupz(".", 1); } // heredoc: Check for ":python <di_tv.v_type == VAR_FUNC) { @@ -2815,7 +2850,7 @@ void ex_function(exarg_T *eap) } } } else { - char numbuf[20]; + char numbuf[NUMBUFLEN]; fp = NULL; if (fudi.fd_newkey == NULL && !eap->forceit) { @@ -2835,8 +2870,8 @@ void ex_function(exarg_T *eap) // Give the function a sequential number. Can only be used with a // Funcref! xfree(name); - snprintf(numbuf, sizeof(numbuf), "%d", ++func_nr); - name = xstrdup(numbuf); + namelen = (size_t)snprintf(numbuf, sizeof(numbuf), "%d", ++func_nr); + name = xmemdupz(numbuf, namelen); } if (fp == NULL) { @@ -2860,7 +2895,10 @@ void ex_function(exarg_T *eap) } } - fp = xcalloc(1, offsetof(ufunc_T, uf_name) + strlen(name) + 1); + if (namelen == 0) { + namelen = strlen(name); + } + fp = alloc_ufunc(name, namelen); if (fudi.fd_dict != NULL) { if (fudi.fd_di == NULL) { @@ -2877,14 +2915,13 @@ void ex_function(exarg_T *eap) tv_clear(&fudi.fd_di->di_tv); } fudi.fd_di->di_tv.v_type = VAR_FUNC; - fudi.fd_di->di_tv.vval.v_string = xstrdup(name); + fudi.fd_di->di_tv.vval.v_string = xmemdupz(name, namelen); // behave like "dict" was used flags |= FC_DICT; } // insert the new function in the function list - set_ufunc_name(fp, name); if (overwrite) { hashitem_T *hi = hash_find(&func_hashtab, name); hi->hi_key = UF2HIKEY(fp); @@ -2927,6 +2964,9 @@ errret_2: ga_clear_strings(&newargs); ga_clear_strings(&default_args); ga_clear_strings(&newlines); + if (fp != NULL) { + XFREE_CLEAR(fp->uf_name_exp); + } if (free_fp) { xfree(fp); fp = NULL; @@ -3022,15 +3062,16 @@ char *get_user_func_name(expand_T *xp, int idx) return ""; // don't show dict and lambda functions } - if (strlen(fp->uf_name) + 4 >= IOSIZE) { + if (fp->uf_namelen + 4 >= IOSIZE) { return fp->uf_name; // Prevent overflow. } - cat_func_name(IObuff, IOSIZE, fp); + int len = cat_func_name(IObuff, IOSIZE, fp); if (xp->xp_context != EXPAND_USER_FUNC) { - xstrlcat(IObuff, "(", IOSIZE); + xstrlcpy(IObuff + len, "(", IOSIZE - (size_t)len); if (!fp->uf_varargs && GA_EMPTY(&fp->uf_args)) { - xstrlcat(IObuff, ")", IOSIZE); + len++; + xstrlcpy(IObuff + len, ")", IOSIZE - (size_t)len); } } return IObuff; @@ -3631,22 +3672,27 @@ bool do_return(exarg_T *eap, bool reanimate, bool is_cmd, void *rettv) char *get_return_cmd(void *rettv) { char *s = NULL; - char *tofree = NULL; + size_t slen = 0; if (rettv != NULL) { + char *tofree = NULL; tofree = s = encode_tv2echo((typval_T *)rettv, NULL); + xfree(tofree); } if (s == NULL) { s = ""; + } else { + slen = strlen(s); } xstrlcpy(IObuff, ":return ", IOSIZE); xstrlcpy(IObuff + 8, s, IOSIZE - 8); - if (strlen(s) + 8 >= IOSIZE) { + size_t IObufflen = 8 + slen; + if (slen + 8 >= IOSIZE) { STRCPY(IObuff + IOSIZE - 4, "..."); + IObufflen += 3; } - xfree(tofree); - return xstrdup(IObuff); + return xstrnsave(IObuff, IObufflen); } /// Get next function line. @@ -4089,7 +4135,8 @@ bool set_ref_in_func(char *name, ufunc_T *fp_in, int copyID) char *register_luafunc(LuaRef ref) { char *name = get_lambda_name(); - ufunc_T *fp = xcalloc(1, offsetof(ufunc_T, uf_name) + strlen(name) + 1); + size_t namelen = get_lambda_name_len(); + ufunc_T *fp = alloc_ufunc(name, namelen); fp->uf_refcount = 1; fp->uf_varargs = true; @@ -4098,7 +4145,6 @@ char *register_luafunc(LuaRef ref) fp->uf_script_ctx = current_sctx; fp->uf_luaref = ref; - STRCPY(fp->uf_name, name); hash_add(&func_hashtab, UF2HIKEY(fp)); // coverity[leaked_storage] diff --git a/src/nvim/ex_docmd.c b/src/nvim/ex_docmd.c index 137226e2ad..ceb17a995d 100644 --- a/src/nvim/ex_docmd.c +++ b/src/nvim/ex_docmd.c @@ -7400,8 +7400,7 @@ char *eval_vars(char *src, const char *srcstart, size_t *usedlen, linenr_T *lnum *errormsg = _(e_usingsid); return NULL; } - snprintf(strbuf, sizeof(strbuf), "%" PRIdSCID "_", - current_sctx.sc_sid); + snprintf(strbuf, sizeof(strbuf), "%" PRIdSCID "_", current_sctx.sc_sid); result = strbuf; break; diff --git a/src/nvim/keycodes.c b/src/nvim/keycodes.c index f7215d3d12..a9f8c9222a 100644 --- a/src/nvim/keycodes.c +++ b/src/nvim/keycodes.c @@ -917,7 +917,7 @@ char *replace_termcodes(const char *const from, const size_t from_len, char **co result[dlen++] = (char)K_SPECIAL; result[dlen++] = (char)KS_EXTRA; result[dlen++] = KE_SNR; - snprintf(result + dlen, buf_len - dlen, "%" PRId64, (int64_t)sid); + snprintf(result + dlen, buf_len - dlen, "%" PRIdSCID, sid); dlen += strlen(result + dlen); result[dlen++] = '_'; continue; -- cgit From 82ac8294c22a15899548a1cb408ca114a598f434 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Sun, 2 Feb 2025 16:00:04 +0800 Subject: vim-patch:9.1.1066: heap-use-after-free and stack-use-after-scope with :14verbose Problem: heap-use-after-free and stack-use-after-scope with :14verbose when using :return and :try (after 9.1.1063). Solution: Move back the vim_free(tofree) and the scope of numbuf[]. (zeertzjq) closes: vim/vim#16563 https://github.com/vim/vim/commit/2101230f4013860dbafcb0cab3f4e6bc92fb6f35 --- src/nvim/eval/userfunc.c | 8 ++++---- test/old/testdir/test_user_func.vim | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/nvim/eval/userfunc.c b/src/nvim/eval/userfunc.c index 6e7d921068..402798cafa 100644 --- a/src/nvim/eval/userfunc.c +++ b/src/nvim/eval/userfunc.c @@ -3672,12 +3672,11 @@ bool do_return(exarg_T *eap, bool reanimate, bool is_cmd, void *rettv) char *get_return_cmd(void *rettv) { char *s = NULL; + char *tofree = NULL; size_t slen = 0; if (rettv != NULL) { - char *tofree = NULL; tofree = s = encode_tv2echo((typval_T *)rettv, NULL); - xfree(tofree); } if (s == NULL) { s = ""; @@ -3688,10 +3687,11 @@ char *get_return_cmd(void *rettv) xstrlcpy(IObuff, ":return ", IOSIZE); xstrlcpy(IObuff + 8, s, IOSIZE - 8); size_t IObufflen = 8 + slen; - if (slen + 8 >= IOSIZE) { + if (IObufflen >= IOSIZE) { STRCPY(IObuff + IOSIZE - 4, "..."); - IObufflen += 3; + IObufflen = IOSIZE - 1; } + xfree(tofree); return xstrnsave(IObuff, IObufflen); } diff --git a/test/old/testdir/test_user_func.vim b/test/old/testdir/test_user_func.vim index b509b03778..da6a6d8dc4 100644 --- a/test/old/testdir/test_user_func.vim +++ b/test/old/testdir/test_user_func.vim @@ -910,4 +910,36 @@ func Test_func_curly_brace_invalid_name() delfunc Fail endfunc +func Test_func_return_in_try_verbose() + func TryReturnList() + try + return [1, 2, 3] + endtry + endfunc + func TryReturnNumber() + try + return 123 + endtry + endfunc + func TryReturnOverlongString() + try + return repeat('a', 9999) + endtry + endfunc + + " This should not cause heap-use-after-free + call assert_match('\n:return \[1, 2, 3\] made pending\n', + \ execute('14verbose call TryReturnList()')) + " This should not cause stack-use-after-scope + call assert_match('\n:return 123 made pending\n', + \ execute('14verbose call TryReturnNumber()')) + " An overlong string is truncated + call assert_match('\n:return a\{100,}\.\.\.', + \ execute('14verbose call TryReturnOverlongString()')) + + delfunc TryReturnList + delfunc TryReturnNumber + delfunc TryReturnOverlongString +endfunc + " vim: shiftwidth=2 sts=2 expandtab -- cgit From db7db783a2d634d5589ebe12605e3989cb30650c Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Mon, 3 Feb 2025 10:49:06 +0800 Subject: vim-patch:9.1.1071: args missing after failing to redefine a function Problem: Arguments of a function are missing after failing to redefine it (after 8.2.2505), and heap-use-after-free with script-local function (after 9.1.1063). Solution: Don't clear arguments or free uf_name_exp when failing to redefine an existing function (zeertzjq) closes: vim/vim#16567 https://github.com/vim/vim/commit/04d2a3fdc051d6a419dc0ea4de7a9640cefccd31 --- src/nvim/eval/userfunc.c | 11 +++++----- test/old/testdir/test_user_func.vim | 40 +++++++++++++++++++++++++++++++++++-- 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/src/nvim/eval/userfunc.c b/src/nvim/eval/userfunc.c index 402798cafa..8022b37f6b 100644 --- a/src/nvim/eval/userfunc.c +++ b/src/nvim/eval/userfunc.c @@ -2825,11 +2825,11 @@ void ex_function(exarg_T *eap) && (fp->uf_script_ctx.sc_sid != current_sctx.sc_sid || fp->uf_script_ctx.sc_seq == current_sctx.sc_seq)) { emsg_funcname(e_funcexts, name); - goto erret; + goto errret_keep; } if (fp->uf_calls > 0) { emsg_funcname(N_("E127: Cannot redefine function %s: It is in use"), name); - goto erret; + goto errret_keep; } if (fp->uf_refcount > 1) { // This function is referenced somewhere, don't redefine it but @@ -2961,9 +2961,6 @@ erret: ga_init(&fp->uf_def_args, (int)sizeof(char *), 1); } errret_2: - ga_clear_strings(&newargs); - ga_clear_strings(&default_args); - ga_clear_strings(&newlines); if (fp != NULL) { XFREE_CLEAR(fp->uf_name_exp); } @@ -2971,6 +2968,10 @@ errret_2: xfree(fp); fp = NULL; } +errret_keep: + ga_clear_strings(&newargs); + ga_clear_strings(&default_args); + ga_clear_strings(&newlines); ret_free: xfree(line_to_free); xfree(fudi.fd_newkey); diff --git a/test/old/testdir/test_user_func.vim b/test/old/testdir/test_user_func.vim index da6a6d8dc4..b1543c8f24 100644 --- a/test/old/testdir/test_user_func.vim +++ b/test/old/testdir/test_user_func.vim @@ -421,12 +421,48 @@ func Test_func_def_error() call assert_fails('exe l', 'E717:') " Define an autoload function with an incorrect file name - call writefile(['func foo#Bar()', 'return 1', 'endfunc'], 'Xscript') + call writefile(['func foo#Bar()', 'return 1', 'endfunc'], 'Xscript', 'D') call assert_fails('source Xscript', 'E746:') - call delete('Xscript') " Try to list functions using an invalid search pattern call assert_fails('function /\%(/', 'E53:') + + " Use a script-local function to cover uf_name_exp. + func s:TestRedefine(arg1 = 1, arg2 = 10) + let caught_E122 = 0 + try + func s:TestRedefine(arg1 = 1, arg2 = 10) + endfunc + catch /E122:/ + let caught_E122 = 1 + endtry + call assert_equal(1, caught_E122) + + let caught_E127 = 0 + try + func! s:TestRedefine(arg1 = 1, arg2 = 10) + endfunc + catch /E127:/ + let caught_E127 = 1 + endtry + call assert_equal(1, caught_E127) + + " The failures above shouldn't cause heap-use-after-free here. + return [a:arg1 + a:arg2, expand('')] + endfunc + + let stacks = [] + " Call the function twice. + " Failing to redefine a function shouldn't clear its argument list. + for i in range(2) + let [val, stack] = s:TestRedefine(1000) + call assert_equal(1010, val) + call assert_match(expand('') .. 'TestRedefine\[20\]$', stack) + call add(stacks, stack) + endfor + call assert_equal(stacks[0], stacks[1]) + + delfunc s:TestRedefine endfunc " Test for deleting a function -- cgit