From e9b9a86cd5a555943b87f0ba40c4527561c3c124 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Sat, 11 Nov 2023 10:21:14 +0800 Subject: [PATCH] fix(context): don't crash on invalid arg to nvim_get_context (#25977) Note: The crash happens in the second test case when using uninitialized memory, and therefore doesn't happen with ASAN. --- src/nvim/api/private/converter.c | 4 ++- src/nvim/api/vim.c | 6 ++--- src/nvim/channel.c | 2 ++ src/nvim/context.c | 25 +++++++++++-------- src/nvim/eval/funcs.c | 13 +++++++--- test/functional/api/vim_spec.lua | 7 ++++++ .../vimscript/ctx_functions_spec.lua | 6 +++++ 7 files changed, 45 insertions(+), 18 deletions(-) diff --git a/src/nvim/api/private/converter.c b/src/nvim/api/private/converter.c index 10152cb3c8..d9832abd76 100644 --- a/src/nvim/api/private/converter.c +++ b/src/nvim/api/private/converter.c @@ -261,7 +261,9 @@ Object vim_to_object(typval_T *obj) /// @param obj Object to convert from. /// @param tv Conversion result is placed here. On failure member v_type is /// set to VAR_UNKNOWN (no allocation was made for this variable). -/// returns true if conversion is successful, otherwise false. +/// @param err Error object. +/// +/// @returns true if conversion is successful, otherwise false. bool object_to_vim(Object obj, typval_T *tv, Error *err) { tv->v_type = VAR_UNKNOWN; diff --git a/src/nvim/api/vim.c b/src/nvim/api/vim.c index c3c619e206..8446f5b383 100644 --- a/src/nvim/api/vim.c +++ b/src/nvim/api/vim.c @@ -1402,7 +1402,7 @@ Dictionary nvim_get_context(Dict(context) *opts, Error *err) /// Sets the current editor state from the given |context| map. /// /// @param dict |Context| map. -Object nvim_load_context(Dictionary dict) +Object nvim_load_context(Dictionary dict, Error *err) FUNC_API_SINCE(6) { Context ctx = CONTEXT_INIT; @@ -1410,8 +1410,8 @@ Object nvim_load_context(Dictionary dict) int save_did_emsg = did_emsg; did_emsg = false; - ctx_from_dict(dict, &ctx); - if (!did_emsg) { + ctx_from_dict(dict, &ctx, err); + if (!ERROR_SET(err)) { ctx_restore(&ctx, kCtxAll); } diff --git a/src/nvim/channel.c b/src/nvim/channel.c index 38c40d2328..4c355dba3f 100644 --- a/src/nvim/channel.c +++ b/src/nvim/channel.c @@ -224,6 +224,7 @@ void channel_create_event(Channel *chan, const char *ext_source) // TODO(bfredl): do the conversion in one step. Also would be nice // to pretty print top level dict in defined order (void)object_to_vim(DICTIONARY_OBJ(info), &tv, NULL); + assert(tv.v_type == VAR_DICT); char *str = encode_tv2json(&tv, NULL); ILOG("new channel %" PRIu64 " (%s) : %s", chan->id, source, str); xfree(str); @@ -849,6 +850,7 @@ static void set_info_event(void **argv) Dictionary info = channel_info(chan->id); typval_T retval; (void)object_to_vim(DICTIONARY_OBJ(info), &retval, NULL); + assert(retval.v_type == VAR_DICT); tv_dict_add_dict(dict, S_LEN("info"), retval.vval.v_dict); tv_dict_set_keys_readonly(dict); diff --git a/src/nvim/context.c b/src/nvim/context.c index 9b1eee56c5..f7ed800aa7 100644 --- a/src/nvim/context.c +++ b/src/nvim/context.c @@ -321,24 +321,28 @@ static inline Array sbuf_to_array(msgpack_sbuffer sbuf) /// Convert readfile()-style array to msgpack_sbuffer. /// /// @param[in] array readfile()-style array to convert. +/// @param[out] err Error object. /// /// @return msgpack_sbuffer with conversion result. -static inline msgpack_sbuffer array_to_sbuf(Array array) +static inline msgpack_sbuffer array_to_sbuf(Array array, Error *err) + FUNC_ATTR_NONNULL_ALL { msgpack_sbuffer sbuf; msgpack_sbuffer_init(&sbuf); typval_T list_tv; - Error err = ERROR_INIT; - (void)object_to_vim(ARRAY_OBJ(array), &list_tv, &err); + if (!object_to_vim(ARRAY_OBJ(array), &list_tv, err)) { + return sbuf; + } + assert(list_tv.v_type == VAR_LIST); if (!encode_vim_list_to_buf(list_tv.vval.v_list, &sbuf.size, &sbuf.data)) { - emsg(_("E474: Failed to convert list to msgpack string buffer")); + api_set_error(err, kErrorTypeException, "%s", + "E474: Failed to convert list to msgpack string buffer"); } sbuf.alloc = sbuf.size; tv_clear(&list_tv); - api_clear_error(&err); return sbuf; } @@ -367,9 +371,10 @@ Dictionary ctx_to_dict(Context *ctx) /// /// @param[in] dict Context Dictionary representation. /// @param[out] ctx Context object to store conversion result into. +/// @param[out] err Error object. /// /// @return types of included context items. -int ctx_from_dict(Dictionary dict, Context *ctx) +int ctx_from_dict(Dictionary dict, Context *ctx, Error *err) FUNC_ATTR_NONNULL_ALL { assert(ctx != NULL); @@ -382,16 +387,16 @@ int ctx_from_dict(Dictionary dict, Context *ctx) } if (strequal(item.key.data, "regs")) { types |= kCtxRegs; - ctx->regs = array_to_sbuf(item.value.data.array); + ctx->regs = array_to_sbuf(item.value.data.array, err); } else if (strequal(item.key.data, "jumps")) { types |= kCtxJumps; - ctx->jumps = array_to_sbuf(item.value.data.array); + ctx->jumps = array_to_sbuf(item.value.data.array, err); } else if (strequal(item.key.data, "bufs")) { types |= kCtxBufs; - ctx->bufs = array_to_sbuf(item.value.data.array); + ctx->bufs = array_to_sbuf(item.value.data.array, err); } else if (strequal(item.key.data, "gvars")) { types |= kCtxGVars; - ctx->gvars = array_to_sbuf(item.value.data.array); + ctx->gvars = array_to_sbuf(item.value.data.array, err); } else if (strequal(item.key.data, "funcs")) { types |= kCtxFuncs; ctx->funcs = copy_object(item.value, NULL).data.array; diff --git a/src/nvim/eval/funcs.c b/src/nvim/eval/funcs.c index 2d8ad458f1..6efd9733e5 100644 --- a/src/nvim/eval/funcs.c +++ b/src/nvim/eval/funcs.c @@ -358,6 +358,7 @@ static void api_wrapper(typval_T *argvars, typval_T *rettv, EvalFuncData fptr) } if (!object_to_vim(result, rettv, &err)) { + assert(ERROR_SET(&err)); semsg(_("Error converting the call result: %s"), err.msg); } @@ -1024,7 +1025,7 @@ static void f_ctxget(typval_T *argvars, typval_T *rettv, EvalFuncData fptr) Dictionary ctx_dict = ctx_to_dict(ctx); Error err = ERROR_INIT; - object_to_vim(DICTIONARY_OBJ(ctx_dict), rettv, &err); + (void)object_to_vim(DICTIONARY_OBJ(ctx_dict), rettv, &err); api_free_dictionary(ctx_dict); api_clear_error(&err); } @@ -1090,14 +1091,16 @@ static void f_ctxset(typval_T *argvars, typval_T *rettv, EvalFuncData fptr) return; } - int save_did_emsg = did_emsg; + const int save_did_emsg = did_emsg; did_emsg = false; Dictionary dict = vim_to_object(&argvars[0]).data.dictionary; Context tmp = CONTEXT_INIT; - ctx_from_dict(dict, &tmp); + Error err = ERROR_INIT; + ctx_from_dict(dict, &tmp, &err); - if (did_emsg) { + if (ERROR_SET(&err)) { + semsg("%s", err.msg); ctx_free(&tmp); } else { ctx_free(ctx); @@ -1105,6 +1108,7 @@ static void f_ctxset(typval_T *argvars, typval_T *rettv, EvalFuncData fptr) } api_free_dictionary(dict); + api_clear_error(&err); did_emsg = save_did_emsg; } @@ -6753,6 +6757,7 @@ static void f_rpcrequest(typval_T *argvars, typval_T *rettv, EvalFuncData fptr) } if (!object_to_vim(result, rettv, &err)) { + assert(ERROR_SET(&err)); semsg(_("Error converting the call result: %s"), err.msg); } diff --git a/test/functional/api/vim_spec.lua b/test/functional/api/vim_spec.lua index c346dfbe6c..bc7e747916 100644 --- a/test/functional/api/vim_spec.lua +++ b/test/functional/api/vim_spec.lua @@ -1979,6 +1979,13 @@ describe('API', function() nvim('load_context', ctx) eq({1, 2 ,3}, eval('[g:one, g:Two, g:THREE]')) end) + + it('errors when context dictionary is invalid', function() + eq('E474: Failed to convert list to msgpack string buffer', + pcall_err(nvim, 'load_context', { regs = { {} } })) + eq("Empty dictionary keys aren't allowed", + pcall_err(nvim, 'load_context', { regs = { { [''] = '' } } })) + end) end) describe('nvim_replace_termcodes', function() diff --git a/test/functional/vimscript/ctx_functions_spec.lua b/test/functional/vimscript/ctx_functions_spec.lua index 5ee84a6d13..66a8ec3550 100644 --- a/test/functional/vimscript/ctx_functions_spec.lua +++ b/test/functional/vimscript/ctx_functions_spec.lua @@ -375,6 +375,12 @@ describe('context functions', function() eq(outofbounds, pcall_err(call, 'ctxset', {dummy = 1}, 0)) end) + it('errors when context dictionary is invalid', function() + call('ctxpush') + eq('Vim:E474: Failed to convert list to msgpack string buffer', + pcall_err(call, 'ctxset', { regs = { {} } })) + end) + it('sets context dictionary at index in context stack', function() nvim('set_var', 'one', 1) nvim('set_var', 'Two', 2)