helpers.c: Fix invalid state of failed conversion result for object_to_vim() (#5282)

If a conversion for a container fails in object_to_vim(), the memory for
the container in the returned/converted value is freed, but the returned
value keeps a pointer to the freed memory. Calling later clear_tv() on
this value leads to an invalid memory access.

Set v_type to VAR_UNKNOWN in the converted value on failure, so that
clear_tv() has no effect.

1 files changed, 28 insertions, 17 deletions

diff --git a/src/nvim/api/private/helpers.c b/src/nvim/api/private/helpers.c
index 7df2079d93..3735139a07 100644
--- a/src/nvim/api/private/helpers.c
+++ b/src/nvim/api/private/helpers.c
@@ -584,10 +584,16 @@ String cstr_as_string(char *str) FUNC_ATTR_PURE
   return (String) {.data = str, .size = strlen(str)};
 }
 
+/// Converts from type Object to a VimL value.
+///
+/// @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.
 bool object_to_vim(Object obj, typval_T *tv, Error *err)
 {
   tv->v_type = VAR_UNKNOWN;
-  tv->v_lock = 0;
+  tv->v_lock = VAR_UNLOCKED;
 
   switch (obj.type) {
     case kObjectTypeNil:
@@ -628,9 +634,8 @@ bool object_to_vim(Object obj, typval_T *tv, Error *err)
       }
       break;
 
-    case kObjectTypeArray:
-      tv->v_type = VAR_LIST;
-      tv->vval.v_list = list_alloc();
+    case kObjectTypeArray: {
+      list_T *list = list_alloc();
 
       for (uint32_t i = 0; i < obj.data.array.size; i++) {
         Object item = obj.data.array.items[i];
@@ -639,45 +644,51 @@ bool object_to_vim(Object obj, typval_T *tv, Error *err)
         if (!object_to_vim(item, &li->li_tv, err)) {
           // cleanup
           listitem_free(li);
-          list_free(tv->vval.v_list, true);
+          list_free(list, true);
           return false;
         }
 
-        list_append(tv->vval.v_list, li);
+        list_append(list, li);
       }
-      tv->vval.v_list->lv_refcount++;
+      list->lv_refcount++;
+
+      tv->v_type = VAR_LIST;
+      tv->vval.v_list = list;
       break;
+    }
 
-    case kObjectTypeDictionary:
-      tv->v_type = VAR_DICT;
-      tv->vval.v_dict = dict_alloc();
+    case kObjectTypeDictionary: {
+      dict_T *dict = dict_alloc();
 
       for (uint32_t i = 0; i < obj.data.dictionary.size; i++) {
         KeyValuePair item = obj.data.dictionary.items[i];
         String key = item.key;
 
         if (key.size == 0) {
-          api_set_error(err,
-                        Validation,
+          api_set_error(err, Validation,
                         _("Empty dictionary keys aren't allowed"));
           // cleanup
-          dict_free(tv->vval.v_dict, true);
+          dict_free(dict, true);
           return false;
         }
 
-        dictitem_T *di = dictitem_alloc((uint8_t *) key.data);
+        dictitem_T *di = dictitem_alloc((uint8_t *)key.data);
 
         if (!object_to_vim(item.value, &di->di_tv, err)) {
           // cleanup
           dictitem_free(di);
-          dict_free(tv->vval.v_dict, true);
+          dict_free(dict, true);
           return false;
         }
 
-        dict_add(tv->vval.v_dict, di);
+        dict_add(dict, di);
       }
-      tv->vval.v_dict->dv_refcount++;
+      dict->dv_refcount++;
+
+      tv->v_type = VAR_DICT;
+      tv->vval.v_dict = dict;
       break;
+    }
     default:
       abort();
   }