patch 9.2.0844: [security]: use-after-free on json decode error

Problem:  [security]: use-after-free on json decode error
          (@tdjackey)
Solution: Report the position from the current reader
          (Matsumoto Yasuhiro)

json_decode_item() caches "p" into js_buf, but json_decode_string() can
refill via channel_fill(), which frees the old js_buf. When the string
parse then fails (e.g. an invalid \u escape), the shared error path passed
the now-dangling "p" to semsg(), a heap use-after-free read reachable
pre-auth through the socketserver.

Github Security Advisory:
https://github.com/vim/vim/security/advisories/GHSA-69ch-22ch-r887

Signed-off-by: Yasuhiro Matsumoto <mattn.jp@gmail.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>

diff --git a/src/json.c b/src/json.c
index b9cbc0d594a63e733b5cc06f9793dfa0b93f5e6e..fb7755d0fbee8cc9dfe87b8e6f18ce69805d8130 100644 (file)
--- a/src/json.c
+++ b/src/json.c
@@ -1431,7 +1431,9 @@ item_end:
        res->v_type = VAR_SPECIAL;
        res->vval.v_number = VVAL_NONE;
     }
-    semsg(_(e_json_decode_error_at_str), p);
+    // "p" may dangle into a buffer freed by a js_fill() refill in
+    // json_decode_string(); report the position from the current reader.
+    semsg(_(e_json_decode_error_at_str), reader->js_buf + reader->js_used);
 
 theend:
     for (i = 0; i < stack.ga_len; i++)

diff --git a/src/testdir/test_clientserver.vim b/src/testdir/test_clientserver.vim
index d5c243fda64ac6fcd39467d4c6eab0c85e5eaa68..8fa204ea5fc309c324ede4a3da5f240913382c25 100644 (file)
--- a/src/testdir/test_clientserver.vim
+++ b/src/testdir/test_clientserver.vim
@@ -427,6 +427,59 @@ func Test_client_server_socketserver_connection_flood()
   endtry
 endfunc
 
+" An invalid JSON message that spans two socketserver read buffers must not
+" crash the server: the parse buffer is refilled (and freed) mid-string, and
+" the error path must not report the position from the stale, freed cursor.
+func Test_client_server_socketserver_json_refill()
+  CheckFeature socketserver
+  CheckNotMSWindows
+
+  let g:test_is_flaky = 1
+  let cmd = GetVimCommand()
+  if cmd == ''
+    throw 'GetVimCommand() failed'
+  endif
+
+  let actual = cmd .. ' --clientserver socket --servername channel:2002'
+  let job = job_start(actual, {'stoponexit': 'kill', 'out_io': 'null'})
+  call WaitForAssert({-> assert_equal("run", job_status(job))})
+  call WaitForAssert({-> assert_match('channel:2002',
+        \ system(actual .. ' --remote-expr "v:servername"'))})
+
+  let ch = test_null_channel()
+  for _ in range(50)
+    let ch = ch_open('127.0.0.1:2002', {'mode': 'raw', 'waittime': 100})
+    if ch_status(ch) == 'open'
+      break
+    endif
+    sleep 100m
+  endfor
+  call assert_equal('open', ch_status(ch))
+
+  " The server reads at most MAXMSGSIZE (4096) bytes per read, so a single
+  " message longer than that is split across two read buffers. Keep the JSON
+  " string open past the split to force a refill (which frees the first
+  " buffer), then end in an invalid \u escape so the parse fails.
+  call ch_sendraw(ch, '{"k":"' .. repeat('A', 4200) .. '\uZZZZ')
+  sleep 500m
+
+  " The server must survive the invalid message and stay responsive.
+  call assert_equal("run", job_status(job))
+  call assert_match('channel:2002',
+        \ system(actual .. ' --remote-expr "v:servername"'))
+
+  call ch_close(ch)
+  call system(actual .. " --remote-expr 'execute(\"qa!\")'")
+  try
+    call WaitForAssert({-> assert_equal("dead", job_status(job))})
+  finally
+    if job_status(job) != 'dead'
+      call assert_report('Server did not exit')
+      call job_stop(job, 'kill')
+    endif
+  endtry
+endfunc
+
 " Test if --remote-wait works properly with multiple files
 func Test_client_server_multiple_remote_wait()
   CheckRunVimInTerminal

diff --git a/src/version.c b/src/version.c
index a9bcdc88ecdf90d3313aae0dd1cb1dddc6c1b354..4bd76c0f5001c308bc41d84793894990177528cf 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -758,6 +758,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    844,
 /**/
     843,
 /**/