CVE-2018-20786: NULL Pointer Dereference (CWE-476)
value: libvterm through 0+bzr726, as used in Vim and other products, mishandles certain out-of-memory conditions, leading to a denial of service (application crash), related to screen.c, state.c, and vterm.c.
CWE description: The product dereferences a pointer that it expects to be valid but is NULL.
CVSS v3 base score: 7.5
Published: 2019-02-24T14:29Z
Repository: https://github.com/vim/vim
Language: C
Files changed: src/libvterm/src/state.c, src/libvterm/src/termscreen.c, src/libvterm/src/vterm.c, src/terminal.c, src/version.c
Commit date: 2018-12-24 21:38:45 +0100
Commit message: patch 8.1.0633: crash when out of memory while opening a terminal window

Problem:    Crash when out of memory while opening a terminal window.
Solution:   Handle out-of-memory more gracefully.

Diff: diff --git a/src/libvterm/src/state.c b/src/libvterm/src/state.c
index d977ecac6..8b02093bb 100644
--- a/src/libvterm/src/state.c
+++ b/src/libvterm/src/state.c
@@ -51,10 +51,12 @@ static void erase(VTermState *state, VTermRect rect, int selective)
 
 static VTermState *vterm_state_new(VTerm *vt)
 {
   VTermState *state = vterm_allocator_malloc(vt, sizeof(VTermState));
 
+  if (state == NULL)
+    return NULL;
   state->vt = vt;
 
   state->rows = vt->rows;
   state->cols = vt->cols;
 
@@ -1691,17 +1693,23 @@ static const VTermParserCallbacks parser_callbacks = {
   on_osc, /* osc */
   on_dcs, /* dcs */
   on_resize /* resize */
 };
 
+/*
+ * Return the existing state or create a new one.
+ * Returns NULL when out of memory.
+ */
 VTermState *vterm_obtain_state(VTerm *vt)
 {
   VTermState *state;
   if(vt->state)
     return vt->state;
 
   state = vterm_state_new(vt);
+  if (state == NULL)
+    return NULL;
   vt->state = state;
 
   state->combine_chars_size = 16;
   state->combine_chars = vterm_allocator_malloc(state->vt, state->combine_chars_size * sizeof(state->combine_chars[0]));
 
diff --git a/src/libvterm/src/termscreen.c b/src/libvterm/src/termscreen.c
index cfae088ba..0cd31cedf 100644
--- a/src/libvterm/src/termscreen.c
+++ b/src/libvterm/src/termscreen.c
@@ -1,7 +1,8 @@
 #include "vterm_internal.h"
 
+/* vim: set sw=2 : */
 #include <stdio.h>
 #include <string.h>
 
 #include "rect.h"
 #include "utf8.h"
@@ -93,12 +94,11 @@ static ScreenCell *realloc_buffer(VTermScreen *screen, ScreenCell *buffer, int n
         new_cell->pen = screen->pen;
       }
     }
   }
 
-  if(buffer)
-    vterm_allocator_free(screen->vt, buffer);
+  vterm_allocator_free(screen->vt, buffer);
 
   return new_buffer;
 }
 
 static void damagerect(VTermScreen *screen, VTermRect rect)
@@ -516,12 +516,11 @@ static int resize(int new_rows, int new_cols, VTermPos *delta, void *user)
   screen->buffer = is_altscreen ? screen->buffers[1] : screen->buffers[0];
 
   screen->rows = new_rows;
   screen->cols = new_cols;
 
-  if(screen->sb_buffer)
-    vterm_allocator_free(screen->vt, screen->sb_buffer);
+  vterm_allocator_free(screen->vt, screen->sb_buffer);
 
   screen->sb_buffer = vterm_allocator_malloc(screen->vt, sizeof(VTermScreenCell) * new_cols);
 
   if(new_cols > old_cols) {
     VTermRect rect;
@@ -617,20 +616,25 @@ static VTermStateCallbacks state_cbs = {
   &bell, /* bell */
   &resize, /* resize */
   &setlineinfo /* setlineinfo */
 };
 
+/*
+ * Allocate a new screen and return it.
+ * Return NULL when out of memory.
+ */
 static VTermScreen *screen_new(VTerm *vt)
 {
   VTermState *state = vterm_obtain_state(vt);
   VTermScreen *screen;
   int rows, cols;
 
-  if(!state)
+  if (state == NULL)
     return NULL;
-
   screen = vterm_allocator_malloc(vt, sizeof(VTermScreen));
+  if (screen == NULL)
+    return NULL;
 
   vterm_get_size(vt, &rows, &cols);
 
   screen->vt = vt;
   screen->state = state;
@@ -644,28 +648,28 @@ static VTermScreen *screen_new(VTerm *vt)
 
   screen->callbacks = NULL;
   screen->cbdata    = NULL;
 
   screen->buffers[0] = realloc_buffer(screen, NULL, rows, cols);
-
   screen->buffer = screen->buffers[0];
-
   screen->sb_buffer = vterm_allocator_malloc(screen->vt, sizeof(VTermScreenCell) * cols);
+  if (screen->buffer == NULL || screen->sb_buffer == NULL)
+  {
+    vterm_screen_free(screen);
+    return NULL;
+  }
 
   vterm_state_set_callbacks(screen->state, &state_cbs, screen);
 
   return screen;
 }
 
 INTERNAL void vterm_screen_free(VTermScreen *screen)
 {
   vterm_allocator_free(screen->vt, screen->buffers[0]);
-  if(screen->buffers[1])
-    vterm_allocator_free(screen->vt, screen->buffers[1]);
-
+  vterm_allocator_free(screen->vt, screen->buffers[1]);
   vterm_allocator_free(screen->vt, screen->sb_buffer);
-
   vterm_allocator_free(screen->vt, screen);
 }
 
 void vterm_screen_reset(VTermScreen *screen, int hard)
 {
diff --git a/src/libvterm/src/vterm.c b/src/libvterm/src/vterm.c
index 9025da444..5e4722ce3 100644
--- a/src/libvterm/src/vterm.c
+++ b/src/libvterm/src/vterm.c
@@ -1,7 +1,8 @@
 #define DEFINE_INLINES
 
+/* vim: set sw=2 : */
 #include "vterm_internal.h"
 
 #include <stdio.h>
 #include <stdlib.h>
 #include <stdarg.h>
@@ -39,10 +40,12 @@ VTerm *vterm_new(int rows, int cols)
 VTerm *vterm_new_with_allocator(int rows, int cols, VTermAllocatorFunctions *funcs, void *allocdata)
 {
   /* Need to bootstrap using the allocator function directly */
   VTerm *vt = (*funcs->malloc)(sizeof(VTerm), allocdata);
 
+  if (vt == NULL)
+    return NULL;
   vt->allocator = funcs;
   vt->allocdata = allocdata;
 
   vt->rows = rows;
   vt->cols = cols;
@@ -53,14 +56,25 @@ VTerm *vterm_new_with_allocator(int rows, int cols, VTermAllocatorFunctions *fun
   vt->parser.cbdata    = NULL;
 
   vt->parser.strbuffer_len = 500; /* should be able to hold an OSC string */
   vt->parser.strbuffer_cur = 0;
   vt->parser.strbuffer = vterm_allocator_malloc(vt, vt->parser.strbuffer_len);
+  if (vt->parser.strbuffer == NULL)
+  {
+    vterm_allocator_free(vt, vt);
+    return NULL;
+  }
 
   vt->outbuffer_len = 200;
   vt->outbuffer_cur = 0;
   vt->outbuffer = vterm_allocator_malloc(vt, vt->outbuffer_len);
+  if (vt->outbuffer == NULL)
+  {
+    vterm_allocator_free(vt, vt->parser.strbuffer);
+    vterm_allocator_free(vt, vt);
+    return NULL;
+  }
 
   return vt;
 }
 
 void vterm_free(VTerm *vt)
@@ -80,13 +94,17 @@ void vterm_free(VTerm *vt)
 INTERNAL void *vterm_allocator_malloc(VTerm *vt, size_t size)
 {
   return (*vt->allocator->malloc)(size, vt->allocdata);
 }
 
+/*
+ * Free "ptr" unless it is NULL.
+ */
 INTERNAL void vterm_allocator_free(VTerm *vt, void *ptr)
 {
-  (*vt->allocator->free)(ptr, vt->allocdata);
+  if (ptr)
+    (*vt->allocator->free)(ptr, vt->allocdata);
 }
 
 void vterm_get_size(const VTerm *vt, int *rowsp, int *colsp)
 {
   if(rowsp)
diff --git a/src/terminal.c b/src/terminal.c
index 06d470c56..1875cc3e3 100644
--- a/src/terminal.c
+++ b/src/terminal.c
@@ -3428,10 +3428,11 @@ init_default_colors(term_T *term)
     static void
 set_vterm_palette(VTerm *vterm, long_u *rgb)
 {
     int		index = 0;
     VTermState	*state = vterm_obtain_state(vterm);
+
     for (; index < 16; index++)
     {
 	VTermColor	color;
 	color.red = (unsigned)(rgb[index] >> 16);
 	color.green = (unsigned)(rgb[index] >> 8) & 255;
@@ -3701,30 +3702,42 @@ static VTermAllocatorFunctions vterm_allocator = {
   &vterm_memfree
 };
 
 /*
  * Create a new vterm and initialize it.
+ * Return FAIL when out of memory.
  */
-    static void
+    static int
 create_vterm(term_T *term, int rows, int cols)
 {
     VTerm	    *vterm;
     VTermScreen	    *screen;
     VTermState	    *state;
     VTermValue	    value;
 
     vterm = vterm_new_with_allocator(rows, cols, &vterm_allocator, NULL);
     term->tl_vterm = vterm;
+    if (vterm == NULL)
+	return FAIL;
+
+    // Allocate screen and state here, so we can bail out if that fails.
+    state = vterm_obtain_state(vterm);
     screen = vterm_obtain_screen(vterm);
+    if (state == NULL || screen == NULL)
+    {
+	vterm_free(vterm);
+	return FAIL;
+    }
+
     vterm_screen_set_callbacks(screen, &screen_callbacks, term);
     /* TODO: depends on 'encoding'. */
     vterm_set_utf8(vterm, 1);
 
     init_default_colors(term);
 
     vterm_state_set_default_colors(
-	    vterm_obtain_state(vterm),
+	    state,
 	    &term->tl_default_color.fg,
 	    &term->tl_default_color.bg);
 
     if (t_colors >= 16)
 	vterm_state_set_bold_highbright(vterm_obtain_state(vterm), 1);
@@ -3744,13 +3757,14 @@ create_vterm(term_T *term, int rows, int cols)
     else
 	value.boolean = 1;
 #else
     value.boolean = 0;
 #endif
-    state = vterm_obtain_state(vterm);
     vterm_state_set_termprop(state, VTERM_PROP_CURSORBLINK, &value);
     vterm_state_set_unrecognised_fallbacks(state, &parser_fallbacks, term);
+
+    return OK;
 }
 
 /*
  * Return the text to show for the buffer name and status.
  */
@@ -5627,11 +5641,12 @@ term_and_job_init(
     winpty_spawn_config_free(spawn_config);
     vim_free(cmd_wchar);
     vim_free(cwd_wchar);
     vim_free(env_wchar);
 
-    create_vterm(term, term->tl_rows, term->tl_cols);
+    if (create_vterm(term, term->tl_rows, term->tl_cols) == FAIL)
+	goto failed;
 
 #if defined(FEAT_GUI) || defined(FEAT_TERMGUICOLORS)
     if (opt->jo_set2 & JO2_ANSI_COLORS)
 	set_vterm_palette(term->tl_vterm, opt->jo_ansi_colors);
     else
@@ -5708,11 +5723,12 @@ create_pty_only(term_T *term, jobopt_T *options)
     HANDLE	    hPipeIn = INVALID_HANDLE_VALUE;
     HANDLE	    hPipeOut = INVALID_HANDLE_VALUE;
     char	    in_name[80], out_name[80];
     channel_T	    *channel = NULL;
 
-    create_vterm(term, term->tl_rows, term->tl_cols);
+    if (create_vterm(term, term->tl_rows, term->tl_cols) == FAIL)
+	return FAIL;
 
     vim_snprintf(in_name, sizeof(in_name), "\\\\.\\pipe\\vim-%d-in-%d",
 	    GetCurrentProcessId(),
 	    curbuf->b_fnum);
     hPipeIn = CreateNamedPipe(in_name, PIPE_ACCESS_OUTBOUND,
@@ -5820,11 +5836,12 @@ term_and_job_init(
 	typval_T    *argvar,
 	char	    **argv,
 	jobopt_T    *opt,
 	jobopt_T    *orig_opt UNUSED)
 {
-    create_vterm(term, term->tl_rows, term->tl_cols);
+    if (create_vterm(term, term->tl_rows, term->tl_cols) == FAIL)
+	return FAIL;
 
 #if defined(FEAT_GUI) || defined(FEAT_TERMGUICOLORS)
     if (opt->jo_set2 & JO2_ANSI_COLORS)
 	set_vterm_palette(term->tl_vterm, opt->jo_ansi_colors);
     else
@@ -5842,11 +5859,12 @@ term_and_job_init(
 }
 
     static int
 create_pty_only(term_T *term, jobopt_T *opt)
 {
-    create_vterm(term, term->tl_rows, term->tl_cols);
+    if (create_vterm(term, term->tl_rows, term->tl_cols) == FAIL)
+	return FAIL;
 
     term->tl_job = job_alloc();
     if (term->tl_job == NULL)
 	return FAIL;
     ++term->tl_job->jv_refcount;
diff --git a/src/version.c b/src/version.c
index 2452ae046..6956225c3 100644
--- a/src/version.c
+++ b/src/version.c
@@ -797,10 +797,12 @@ static char *(features[]) =
 	NULL
 };
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    633,
 /**/
     632,
 /**/
     631,
 /**/

Vulnerable code:   if(buffer)
    vterm_allocator_free(screen->vt, buffer);
  if(screen->sb_buffer)
    vterm_allocator_free(screen->vt, screen->sb_buffer);
  if(!state)



  if(screen->buffers[1])
    vterm_allocator_free(screen->vt, screen->buffers[1]);


  (*vt->allocator->free)(ptr, vt->allocdata);
    static void
	    vterm_obtain_state(vterm),
    state = vterm_obtain_state(vterm);
    create_vterm(term, term->tl_rows, term->tl_cols);
    create_vterm(term, term->tl_rows, term->tl_cols);
    create_vterm(term, term->tl_rows, term->tl_cols);
    create_vterm(term, term->tl_rows, term->tl_cols);
Fixed code:   if (state == NULL)
    return NULL;
/*
 * Return the existing state or create a new one.
 * Returns NULL when out of memory.
 */
  if (state == NULL)
    return NULL;
/* vim: set sw=2 : */
  vterm_allocator_free(screen->vt, buffer);
  vterm_allocator_free(screen->vt, screen->sb_buffer);
/*
 * Allocate a new screen and return it.
 * Return NULL when out of memory.
 */
  if (state == NULL)
  if (screen == NULL)
    return NULL;
  if (screen->buffer == NULL || screen->sb_buffer == NULL)
  {
    vterm_screen_free(screen);
    return NULL;
  }
  vterm_allocator_free(screen->vt, screen->buffers[1]);
/* vim: set sw=2 : */
  if (vt == NULL)
    return NULL;
  if (vt->parser.strbuffer == NULL)
  {
    vterm_allocator_free(vt, vt);
    return NULL;
  }
  if (vt->outbuffer == NULL)
  {
    vterm_allocator_free(vt, vt->parser.strbuffer);
    vterm_allocator_free(vt, vt);
    return NULL;
  }
/*
 * Free "ptr" unless it is NULL.
 */
  if (ptr)
    (*vt->allocator->free)(ptr, vt->allocdata);

 * Return FAIL when out of memory.
    static int
    if (vterm == NULL)
	return FAIL;

    // Allocate screen and state here, so we can bail out if that fails.
    state = vterm_obtain_state(vterm);
    if (state == NULL || screen == NULL)
    {
	vterm_free(vterm);
	return FAIL;
    }

	    state,

    return OK;
    if (create_vterm(term, term->tl_rows, term->tl_cols) == FAIL)
	goto failed;
    if (create_vterm(term, term->tl_rows, term->tl_cols) == FAIL)
	return FAIL;
    if (create_vterm(term, term->tl_rows, term->tl_cols) == FAIL)
	return FAIL;
    if (create_vterm(term, term->tl_rows, term->tl_cols) == FAIL)
	return FAIL;
/**/
    633,